On Tue, Jan 16, 2024 at 09:56:32AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> 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 <p...@chromium.org>
> Reviewed-by: Andrzej Hajda <andrzej.ha...@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
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 <p...@chromium.org>

> ---
>  .../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
> 

Reply via email to