On Tue, Jan 16, 2024 at 09:56:32AM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > There's no reason the caller of intel_initial_plane_config() should > have to loop over the CRTCs. Pull the loop into the function to > make life simpler for the caller. > > Cc: Paz Zcharya <pazz@xxxxxxxxxxxx> > Reviewed-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Hi Ville, Thank you so much for this incredible series. It solves the issue regarding MTL initial plane readout that Andrzej Hajda and I worked on in https://patchwork.freedesktop.org/patch/570811/?series=127130&rev=2 In addition, it solved the issue with the new GOP. I tested it on two different devices with Meteor Lake and it worked perfectly: no i915 errors, no flickers or observable issues. Tested-by: Paz Zcharya <pazz@xxxxxxxxxxxx> > --- > .../drm/i915/display/intel_display_driver.c | 7 +--- > .../drm/i915/display/intel_plane_initial.c | 40 +++++++++++-------- > .../drm/i915/display/intel_plane_initial.h | 4 +- > 3 files changed, 26 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c > index ecf9cb74734b..f3fe1743243b 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c > @@ -415,7 +415,6 @@ int intel_display_driver_probe_nogem(struct drm_i915_private *i915) > { > struct drm_device *dev = &i915->drm; > enum pipe pipe; > - struct intel_crtc *crtc; > int ret; > > if (!HAS_DISPLAY(i915)) > @@ -467,11 +466,7 @@ int intel_display_driver_probe_nogem(struct drm_i915_private *i915) > intel_acpi_assign_connector_fwnodes(i915); > drm_modeset_unlock_all(dev); > > - for_each_intel_crtc(dev, crtc) { > - if (!to_intel_crtc_state(crtc->base.state)->uapi.active) > - continue; > - intel_crtc_initial_plane_config(crtc); > - } > + intel_initial_plane_config(i915); > > /* > * Make sure hardware watermarks really match the state we read out. > diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c > index 78bff6181ceb..b7e12b60d68b 100644 > --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c > +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c > @@ -357,25 +357,31 @@ static void plane_config_fini(struct intel_initial_plane_config *plane_config) > i915_vma_put(plane_config->vma); > } > > -void intel_crtc_initial_plane_config(struct intel_crtc *crtc) > +void intel_initial_plane_config(struct drm_i915_private *i915) > { > - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > - struct intel_initial_plane_config plane_config = {}; > + struct intel_crtc *crtc; > > - /* > - * Note that reserving the BIOS fb up front prevents us > - * from stuffing other stolen allocations like the ring > - * on top. This prevents some ugliness at boot time, and > - * can even allow for smooth boot transitions if the BIOS > - * fb is large enough for the active pipe configuration. > - */ > - dev_priv->display.funcs.display->get_initial_plane_config(crtc, &plane_config); > + for_each_intel_crtc(&i915->drm, crtc) { > + struct intel_initial_plane_config plane_config = {}; > > - /* > - * If the fb is shared between multiple heads, we'll > - * just get the first one. > - */ > - intel_find_initial_plane_obj(crtc, &plane_config); > + if (!to_intel_crtc_state(crtc->base.state)->uapi.active) > + continue; > > - plane_config_fini(&plane_config); > + /* > + * Note that reserving the BIOS fb up front prevents us > + * from stuffing other stolen allocations like the ring > + * on top. This prevents some ugliness at boot time, and > + * can even allow for smooth boot transitions if the BIOS > + * fb is large enough for the active pipe configuration. > + */ > + i915->display.funcs.display->get_initial_plane_config(crtc, &plane_config); > + > + /* > + * If the fb is shared between multiple heads, we'll > + * just get the first one. > + */ > + intel_find_initial_plane_obj(crtc, &plane_config); > + > + plane_config_fini(&plane_config); > + } > } > diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.h b/drivers/gpu/drm/i915/display/intel_plane_initial.h > index c7e35ab3182b..64ab95239cd4 100644 > --- a/drivers/gpu/drm/i915/display/intel_plane_initial.h > +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.h > @@ -6,8 +6,8 @@ > #ifndef __INTEL_PLANE_INITIAL_H__ > #define __INTEL_PLANE_INITIAL_H__ > > -struct intel_crtc; > +struct drm_i915_private; > > -void intel_crtc_initial_plane_config(struct intel_crtc *crtc); > +void intel_initial_plane_config(struct drm_i915_private *i915); > > #endif > -- > 2.41.0 >