On Thu, Oct 20, 2022 at 09:48:20AM +0000, Kandpal, Suraj wrote:
> > > 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
It's fine for now. Stopping the legacy pointer usage involved
a lot of work.
>
> >
> > 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
--
Ville Syrjälä
Intel
