Re: [PATCH] drm/i915: Move number of scalers initialization to runtime init

[Chris Wilson <chris@chris-wilson.co.uk>]

On Fri, Nov 25, 2016 at 03:16:58PM +0530, Nabendu Maiti wrote:
> In future patches, we require greater flexibility in describing
> the number of scalers available on each CRTC. To ease that transition
> we move the current assignment to intel_device_info.
> 
> Scaler structure initialisation is done if scaler is available on the CRTC.
> Gen9 check is not required as on depending upon numbers of scalers we
> initialize scalers or return without doing anything in skl_init_scalers.
> 
> v2: Added Chris's commenents.
comments :)

> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> ---
>  drivers/gpu/drm/i915/i915_drv.h          |  1 +
>  drivers/gpu/drm/i915/intel_device_info.c |  3 +++
>  drivers/gpu/drm/i915/intel_display.c     | 18 ++++++++----------
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1ec9619..bb8c5f0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -758,6 +758,7 @@ struct intel_device_info {
>  	u16 device_id;
>  	u8 num_pipes;
>  	u8 num_sprites[I915_MAX_PIPES];
> +	u8 num_scalers[I915_MAX_PIPES];
>  	u8 gen;
>  	u16 gen_mask;
>  	u8 ring_mask; /* Rings supported by the HW */
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 185e3bb..ef26fa8 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -282,6 +282,9 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>  		info->num_sprites[PIPE_A] = 2;
>  		info->num_sprites[PIPE_B] = 2;
>  		info->num_sprites[PIPE_C] = 1;
> +		info->num_scalers[PIPE_A] = 2;
> +		info->num_scalers[PIPE_B] = 2;
> +		info->num_scalers[PIPE_C] = 1;
>  	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>  		for_each_pipe(dev_priv, pipe)
>  			info->num_sprites[pipe] = 2;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5d11002..2062170 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15266,6 +15266,11 @@ static void skl_init_scalers(struct drm_i915_private *dev_priv,
>  		&crtc_state->scaler_state;
>  	int i;
>  
> +	crtc->num_scalers = dev_priv->info.num_scalers[crtc->pipe];
> +

Blank here is overkill

> +	if (!crtc->num_scalers)
> +		return;
> +

crtc->num_scalers = <info>;
if (!ctrc->num_scalers)
	return;

is quite clean.

>  	for (i = 0; i < crtc->num_scalers; i++) {
>  		struct intel_scaler *scaler = &scaler_state->scalers[i];
>  
> @@ -15297,16 +15302,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	intel_crtc->base.state = &crtc_state->base;
>  	crtc_state->base.crtc = &intel_crtc->base;
>  
> -	/* initialize shared scalers */
> -	if (INTEL_GEN(dev_priv) >= 9) {
> -		if (pipe == PIPE_C)
> -			intel_crtc->num_scalers = 1;
> -		else
> -			intel_crtc->num_scalers = SKL_NUM_SCALERS;
> -
> -		skl_init_scalers(dev_priv, intel_crtc, crtc_state);
> -	}
> -
>  	primary = intel_primary_plane_create(dev_priv, pipe);
>  	if (IS_ERR(primary)) {
>  		ret = PTR_ERR(primary);
> @@ -15348,6 +15343,9 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  
>  	intel_crtc->wm.cxsr_allowed = true;
>  
> +	/* initialize shared scalers */
> +	skl_init_scalers(dev_priv, intel_crtc, crtc_state);

As this is now called by all, it really should be
intel_crtc_init_scalers() and we only need to pass intel_crtc at this
point.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx