On Tue, 16 Jan 2024, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> 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>
> ---
> .../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)
So this fails to build on CONFIG_DRM_XE=m, because it has its own
version of intel_plane_initial.c which has
intel_crtc_initial_plane_config(), but not intel_initial_plane_config().
You'll get this as the first indication:
CC [M] drivers/gpu/drm/xe/display/xe_plane_initial.o
../drivers/gpu/drm/xe/display/xe_plane_initial.c:270:6: error: no previous prototype for ‘intel_crtc_initial_plane_config’ [-Werror=missing-prototypes]
270 | void intel_crtc_initial_plane_config(struct intel_crtc *crtc)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
but if you bypass that, eventually:
MODPOST Module.symvers
ERROR: modpost: "intel_initial_plane_config" [drivers/gpu/drm/xe/xe.ko] undefined!
Needs to be fixed before merging.
BR,
Jani.
> {
> - 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
--
Jani Nikula, Intel
