> > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
> > b/drivers/gpu/drm/i915/display/intel_acpi.c
> > index 9df78e7caa2b..912fe5c2ffe5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> > @@ -349,8 +349,13 @@ void intel_acpi_video_register(struct
> drm_i915_private *i915)
> > */
> > drm_connector_list_iter_begin(&i915->drm, &conn_iter);
> > drm_for_each_connector_iter(connector, &conn_iter) {
> > - struct intel_panel *panel = &to_intel_connector(connector)-
> >panel;
> > + struct intel_panel *panel;
> > + struct intel_connector *intel_connector =
> > + to_intel_connector(connector);
> >
> > + if (!intel_connector)
> > + continue;
> > + panel = &intel_connector->panel;
> > if (panel->backlight.funcs && !panel->backlight.device) {
> > acpi_video_register_backlight();
> > break;
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> > b/drivers/gpu/drm/i915/display/intel_display.h
> > index a1ed9c82e2ed..102bf7d47ccc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -52,6 +52,7 @@ struct intel_crtc_state; struct intel_digital_port;
> > struct intel_dp; struct intel_encoder;
> > +struct intel_connector;
> > struct intel_initial_plane_config;
> > struct intel_load_detect_pipe;
> > struct intel_plane;
> > @@ -469,16 +470,12 @@ enum hpd_pin {
> > for_each_if(intel_encoder_can_psr(intel_encoder))
> >
> > #define for_each_intel_connector_iter(intel_connector, iter) \
> > - while ((intel_connector =
> to_intel_connector(drm_connector_list_iter_next(iter))))
> > + while ((intel_connector = intel_connector_list_iter_next(iter)))
> >
> > #define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \
> > 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))
> > -
>
> Getting rid of this macro should be a separate change. As the first patch,
> could've been merged already.
>
Hi Jani,
This change has no dependency on the last patch as the only place this is called (intel_encoder_find_connector)
Is also being fixed in this patch itself so creating a new patch for this may not be necessary
Regards,
Suraj Kandpal
> > #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_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 633cacd79074..a2d294929a64 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1498,12 +1498,14 @@ struct cxsr_latency { #define
> > to_intel_atomic_state(x) container_of(x, struct intel_atomic_state,
> > base) #define to_intel_crtc(x) container_of(x, struct intel_crtc,
> > base) #define to_intel_crtc_state(x) container_of(x, struct
> > intel_crtc_state, uapi) -#define to_intel_connector(x) container_of(x,
> > struct intel_connector, base)
> > +#define to_intel_wb_connector(x) container_of(x, struct
> > +intel_wb_connector, base)
>
> Note that all of the macros here are sorted alphabetically.
>
> The wb/wd difference is a pretty bad eyesore. Maybe we should use one or
> the other, not mix them. (Except if we go with writeback, leave the register
> macros WD because that's what they are.)
>
> > #define to_intel_encoder(x) container_of(x, struct intel_encoder,
> > base) #define to_intel_framebuffer(x) container_of(x, struct
> > intel_framebuffer, base) #define to_intel_plane(x) container_of(x,
> > struct intel_plane, base) #define to_intel_plane_state(x)
> > container_of(x, struct intel_plane_state, uapi) #define
> > intel_fb_obj(x) ((x) ? to_intel_bo((x)->obj[0]) : NULL)
> > +#define to_intel_connector(x) (((x->connector_type ==
> DRM_MODE_CONNECTOR_WRITEBACK)) ? \
> > + NULL : container_of(x, struct
> intel_connector, base))
>
> Would need to have (x)->connector_type, with parenthesis.
>
> The problem with this is that we currently have 131 uses of
> to_intel_connector() and practically all of them expect to get non-NULL
> result.
>
> You're going to get 131 static analyzer reports that you don't check for
> NULL. You can't check for NULL in most places, because they're in the
> middle of local parameter initialization.
>
> >
> > struct intel_hdmi {
> > i915_reg_t hdmi_reg;
> > @@ -2069,4 +2071,26 @@ to_intel_frontbuffer(struct drm_framebuffer
> *fb)
> > return fb ? to_intel_framebuffer(fb)->frontbuffer : NULL; }
> >
> > +static inline struct intel_connector *
> > +intel_connector_list_iter_next(struct drm_connector_list_iter *iter)
> > +{
> > + struct drm_connector *connector;
> > + bool flag = true;
> > + /*
> > + * Skipping connector that are Writeback connector as they will
> > + * not be embedded in intel connector
> > + */
> > + while (flag) {
> > + connector = drm_connector_list_iter_next(iter);
> > + if (connector && !to_intel_connector(connector))
> > + continue;
> > +
> > + flag = false;
> > +
> > + if (connector)
> > + return to_intel_connector(connector);
> > +
> > + }
> > + return NULL;
> > +}
>
> This gets pretty ugly. I've got an idea, I'll send patches later today.
>
> Code is worth a thousand words, it's easier to explain that way.
>
> BR,
> Jani.
>
> > #endif /* __INTEL_DISPLAY_TYPES_H__ */ diff --git
> > a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > index cbfabd58b75a..e1a90331c230 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) {
> > + if (&encoder->base == connector->base.encoder) {
> > + found_connector = true;
> > + break;
> > + }
> > + }
> > + drm_connector_list_iter_end(&conn_iter);
> >
> > + if (found_connector)
> > + return connector;
> > return NULL;
> > }
>
> --
> Jani Nikula, Intel Open Source Graphics Center
