diff mbox series

[v3,12/16] drm/i915: Simplify intel_initial_plane_config() calling convention

Message ID 20240116075636.6121-13-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: (stolen) memory region related fixes | expand

Commit Message

Ville Syrjälä Jan. 16, 2024, 7:56 a.m. UTC
From: Ville Syrjälä <ville.syrjala@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 <pazz@chromium.org>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../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(-)

Comments

kernel test robot Jan. 28, 2024, 4:18 a.m. UTC | #1
Hi Ville,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.8-rc1 next-20240125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ville-Syrjala/drm-i915-Use-struct-resource-for-memory-region-IO-as-well/20240125-222947
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
patch link:    https://lore.kernel.org/r/20240116075636.6121-13-ville.syrjala%40linux.intel.com
patch subject: [PATCH v3 12/16] drm/i915: Simplify intel_initial_plane_config() calling convention
config: alpha-randconfig-r121-20240127 (https://download.01.org/0day-ci/archive/20240128/202401281233.cX62UpWx-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240128/202401281233.cX62UpWx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401281233.cX62UpWx-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/xe/display/xe_plane_initial.c:270:6: sparse: sparse: symbol 'intel_crtc_initial_plane_config' was not declared. Should it be static?

vim +/intel_crtc_initial_plane_config +270 drivers/gpu/drm/xe/display/xe_plane_initial.c

44e694958b9539 Maarten Lankhorst 2023-08-17  269  
44e694958b9539 Maarten Lankhorst 2023-08-17 @270  void intel_crtc_initial_plane_config(struct intel_crtc *crtc)
Paz Zcharya Jan. 30, 2024, 11:24 p.m. UTC | #2
On Tue, Jan 16, 2024 at 09:56:32AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@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 <pazz@chromium.org>
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@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 <pazz@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
>
Jani Nikula Feb. 2, 2024, 3:14 p.m. UTC | #3
On Tue, 16 Jan 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@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 <pazz@chromium.org>
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  .../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
Ville Syrjälä Feb. 2, 2024, 4:12 p.m. UTC | #4
On Fri, Feb 02, 2024 at 05:14:37PM +0200, Jani Nikula wrote:
> On Tue, 16 Jan 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@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 <pazz@chromium.org>
> > Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  .../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.

Can't see anything off in the CI results. Are we not at
least build testing xe in i915 CI?

> 
> 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
Jani Nikula Feb. 2, 2024, 4:15 p.m. UTC | #5
On Fri, 02 Feb 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Feb 02, 2024 at 05:14:37PM +0200, Jani Nikula wrote:
>> On Tue, 16 Jan 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> 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.
>
> Can't see anything off in the CI results. Are we not at
> least build testing xe in i915 CI?

We're not. It's a work in progress.

You can Cc: intel-xe mailing list to get both build and testing on
Xe. (Also a work in progress to use the same config and builds for
both. Now they're completely detached.)


BR,
Jani.
kernel test robot Feb. 2, 2024, 11:58 p.m. UTC | #6
Hi Ville,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.8-rc2 next-20240202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ville-Syrjala/drm-i915-Use-struct-resource-for-memory-region-IO-as-well/20240125-222947
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
patch link:    https://lore.kernel.org/r/20240116075636.6121-13-ville.syrjala%40linux.intel.com
patch subject: [PATCH v3 12/16] drm/i915: Simplify intel_initial_plane_config() calling convention
config: x86_64-randconfig-076-20240202 (https://download.01.org/0day-ci/archive/20240203/202402030704.nGzOFDCt-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240203/202402030704.nGzOFDCt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402030704.nGzOFDCt-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/xe/display/xe_plane_initial.c:270:6: error: no previous prototype for function 'intel_crtc_initial_plane_config' [-Werror,-Wmissing-prototypes]
     270 | void intel_crtc_initial_plane_config(struct intel_crtc *crtc)
         |      ^
   drivers/gpu/drm/xe/display/xe_plane_initial.c:270:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     270 | void intel_crtc_initial_plane_config(struct intel_crtc *crtc)
         | ^
         | static 
   1 error generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for DRM_I915_DEBUG_GEM
   Depends on [n]: HAS_IOMEM [=y] && DRM_I915 [=m] && EXPERT [=y] && DRM_I915_WERROR [=n]
   Selected by [m]:
   - DRM_I915_DEBUG [=y] && HAS_IOMEM [=y] && DRM_I915 [=m] && EXPERT [=y] && !COMPILE_TEST [=n]


vim +/intel_crtc_initial_plane_config +270 drivers/gpu/drm/xe/display/xe_plane_initial.c

44e694958b9539 Maarten Lankhorst 2023-08-17  269  
44e694958b9539 Maarten Lankhorst 2023-08-17 @270  void intel_crtc_initial_plane_config(struct intel_crtc *crtc)
diff mbox series

Patch

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