> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> > b/drivers/gpu/drm/i915/display/intel_display.h
> > index 2af4a1925063..c803330a276d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -469,10 +469,6 @@ enum hpd_pin {
> >     list_for_each_entry((intel_encoder), &(dev)-
> >mode_config.encoder_list, base.head) \
> >             for_each_if((intel_encoder)->base.crtc == (__crtc))
> >
> > -#define for_each_connector_on_encoder(dev, __encoder,
> intel_connector) \
> > -   list_for_each_entry((intel_connector), &(dev)-
> >mode_config.connector_list, base.head) \
> > -           for_each_if((intel_connector)->base.encoder ==
> (__encoder))
> > -
> >  #define for_each_old_intel_plane_in_state(__state, plane,
> old_plane_state, __i) \
> >     for ((__i) = 0; \
> >          (__i) < (__state)->base.dev->mode_config.num_total_plane && \
> > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > index cbfabd58b75a..31ce4ac4059c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > @@ -205,12 +205,22 @@ static bool intel_crtc_has_encoders(struct
> > intel_crtc *crtc)
> >
> >  static struct intel_connector *intel_encoder_find_connector(struct
> > intel_encoder *encoder)  {
> > -   struct drm_device *dev = encoder->base.dev;
> > +   struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >     struct intel_connector *connector;
> > +   struct drm_connector_list_iter conn_iter;
> > +   bool found_connector = false;
> >
> > -   for_each_connector_on_encoder(dev, &encoder->base, connector)
> > -           return connector;
> > +   drm_connector_list_iter_begin(&i915->drm, &conn_iter);
> > +           for_each_intel_connector_iter(connector, &conn_iter) {
> 
> Bogus indentation.
> 
Hi Ville,

Will remove the bogus indentation.

> > +                   if (&encoder->base == connector->base.encoder) {
> 
> Still using the legacy pointers so not great. But I suppse it's still better 
> than
> before because the horrors are now better contained in this file rather than
> in an official looking macro.

Any suggestion on how I could make this better or should I go ahead with this
approach for now

> 
> I do have some work in some old branch to get rid of the legacy pointer
> crap, but it'll probably need a rewrite by now...
> 
> > +                           found_connector = true;
> > +                           break;
> > +                   }
> > +   }
> > +   drm_connector_list_iter_end(&conn_iter);
> >
> > +   if (found_connector)
> 
> I would make that a pointer to the connector, then you can just return it
> directly. And as a bonus we don't use the loop iterator outside the loop
> anymore, which is a bit questionable in general.

Ah got it .

Regards,
Suraj Kandpal

> 
> > +           return connector;
> >     return NULL;
> >  }
> >
> > --
> > 2.25.1
> 
> --
> Ville Syrjälä
> Intel

Reply via email to