diff mbox

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

Message ID 1480398794-22741-1-git-send-email-nabendu.bikash.maiti@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nabendu Maiti Nov. 29, 2016, 5:53 a.m. UTC
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.

v3: Changed skl_init_scaler to intel_crtc_init_scalers

v2: Added Chris's comments.

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |  1 +
 drivers/gpu/drm/i915/intel_device_info.c |  3 +++
 drivers/gpu/drm/i915/intel_display.c     | 26 +++++++++++---------------
 3 files changed, 15 insertions(+), 15 deletions(-)

Comments

Nabendu Maiti Dec. 26, 2016, 11 a.m. UTC | #1
Hi,

Please rb this patch.

earlier patch rbed
https://patchwork.freedesktop.org/patch/123962/


On 11/29/2016 11:23 AM, 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.
>
> v3: Changed skl_init_scaler to intel_crtc_init_scalers
>
> v2: Added Chris's comments.
>
> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |  1 +
>  drivers/gpu/drm/i915/intel_device_info.c |  3 +++
>  drivers/gpu/drm/i915/intel_display.c     | 26 +++++++++++---------------
>  3 files changed, 15 insertions(+), 15 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..46de54a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -115,8 +115,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>  			    const struct intel_crtc_state *pipe_config);
>  static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
>  static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
> -static void skl_init_scalers(struct drm_i915_private *dev_priv,
> -			     struct intel_crtc *crtc,
> +static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>  			     struct intel_crtc_state *crtc_state);
>  static void skylake_pfit_enable(struct intel_crtc *crtc);
>  static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
> @@ -10713,7 +10712,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>
>  	if (INTEL_GEN(dev_priv) >= 9) {
> -		skl_init_scalers(dev_priv, crtc, pipe_config);
> +		intel_crtc_init_scalers(crtc, pipe_config);
>
>  		pipe_config->scaler_state.scaler_id = -1;
>  		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> @@ -15258,14 +15257,18 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	return ERR_PTR(ret);
>  }
>
> -static void skl_init_scalers(struct drm_i915_private *dev_priv,
> -			     struct intel_crtc *crtc,
> +static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>  			     struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	int i;
>
> +	crtc->num_scalers = dev_priv->info.num_scalers[crtc->pipe];
> +	if (!crtc->num_scalers)
> +		return;
> +
>  	for (i = 0; i < crtc->num_scalers; i++) {
>  		struct intel_scaler *scaler = &scaler_state->scalers[i];
>
> @@ -15297,16 +15300,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 +15341,9 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>
>  	intel_crtc->wm.cxsr_allowed = true;
>
> +	/* initialize shared scalers */
> +	intel_crtc_init_scalers(intel_crtc, crtc_state);
> +
>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>  	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = intel_crtc;
>
Ander Conselvan de Oliveira Jan. 2, 2017, 1 p.m. UTC | #2
On Tue, 2016-11-29 at 11:23 +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.
> 
> v3: Changed skl_init_scaler to intel_crtc_init_scalers
> 
> v2: Added Chris's comments.
> 
> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |  1 +
>  drivers/gpu/drm/i915/intel_device_info.c |  3 +++
>  drivers/gpu/drm/i915/intel_display.c     | 26 +++++++++++---------------
>  3 files changed, 15 insertions(+), 15 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..46de54a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -115,8 +115,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>  			    const struct intel_crtc_state *pipe_config);
>  static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
>  static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
> -static void skl_init_scalers(struct drm_i915_private *dev_priv,
> -			     struct intel_crtc *crtc,
> +static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>  			     struct intel_crtc_state *crtc_state);

This line is now misaligned.

>  static void skylake_pfit_enable(struct intel_crtc *crtc);
>  static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
> @@ -10713,7 +10712,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>  
>  	if (INTEL_GEN(dev_priv) >= 9) {
> -		skl_init_scalers(dev_priv, crtc, pipe_config);
> +		intel_crtc_init_scalers(crtc, pipe_config);
>  
>  		pipe_config->scaler_state.scaler_id = -1;
>  		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> @@ -15258,14 +15257,18 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	return ERR_PTR(ret);
>  }
>  
> -static void skl_init_scalers(struct drm_i915_private *dev_priv,
> -			     struct intel_crtc *crtc,
> +static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>  			     struct intel_crtc_state *crtc_state)

Here too.

With that fixed,

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

>  {
>  	struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	int i;
>  
> +	crtc->num_scalers = dev_priv->info.num_scalers[crtc->pipe];
> +	if (!crtc->num_scalers)
> +		return;
> +
>  	for (i = 0; i < crtc->num_scalers; i++) {
>  		struct intel_scaler *scaler = &scaler_state->scalers[i];
>  
> @@ -15297,16 +15300,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 +15341,9 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  
>  	intel_crtc->wm.cxsr_allowed = true;
>  
> +	/* initialize shared scalers */
> +	intel_crtc_init_scalers(intel_crtc, crtc_state);
> +
>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>  	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = intel_crtc;
Ander Conselvan de Oliveira Jan. 2, 2017, 1:04 p.m. UTC | #3
On Mon, 2017-01-02 at 15:00 +0200, Ander Conselvan De Oliveira wrote:
> On Tue, 2016-11-29 at 11:23 +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.
> > 
> > v3: Changed skl_init_scaler to intel_crtc_init_scalers
> > 
> > v2: Added Chris's comments.
> > 
> > Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h          |  1 +
> >  drivers/gpu/drm/i915/intel_device_info.c |  3 +++
> >  drivers/gpu/drm/i915/intel_display.c     | 26 +++++++++++---------------
> >  3 files changed, 15 insertions(+), 15 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..46de54a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -115,8 +115,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
> >  			    const struct intel_crtc_state *pipe_config);
> >  static void intel_begin_crtc_commit(struct drm_crtc *, struct
> > drm_crtc_state *);
> >  static void intel_finish_crtc_commit(struct drm_crtc *, struct
> > drm_crtc_state *);
> > -static void skl_init_scalers(struct drm_i915_private *dev_priv,
> > -			     struct intel_crtc *crtc,
> > +static void intel_crtc_init_scalers(struct intel_crtc *crtc,
> >  			     struct intel_crtc_state *crtc_state);
> This line is now misaligned.
> 
> > 
> >  static void skylake_pfit_enable(struct intel_crtc *crtc);
> >  static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
> > @@ -10713,7 +10712,7 @@ static bool haswell_get_pipe_config(struct
> > intel_crtc *crtc,
> >  		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
> >  
> >  	if (INTEL_GEN(dev_priv) >= 9) {
> > -		skl_init_scalers(dev_priv, crtc, pipe_config);
> > +		intel_crtc_init_scalers(crtc, pipe_config);
> >  
> >  		pipe_config->scaler_state.scaler_id = -1;
> >  		pipe_config->scaler_state.scaler_users &= ~(1 <<
> > SKL_CRTC_INDEX);
> > @@ -15258,14 +15257,18 @@ intel_cursor_plane_create(struct drm_i915_private
> > *dev_priv, enum pipe pipe)
> >  	return ERR_PTR(ret);
> >  }
> >  
> > -static void skl_init_scalers(struct drm_i915_private *dev_priv,
> > -			     struct intel_crtc *crtc,
> > +static void intel_crtc_init_scalers(struct intel_crtc *crtc,
> >  			     struct intel_crtc_state *crtc_state)
> Here too.
> 
> With that fixed,
> 
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

Pushed.

I fixed the coding style issues while pushing, but please observe these in the
future.

Ander

> 
> > 
> >  {
> >  	struct intel_crtc_scaler_state *scaler_state =
> >  		&crtc_state->scaler_state;
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	int i;
> >  
> > +	crtc->num_scalers = dev_priv->info.num_scalers[crtc->pipe];
> > +	if (!crtc->num_scalers)
> > +		return;
> > +
> >  	for (i = 0; i < crtc->num_scalers; i++) {
> >  		struct intel_scaler *scaler = &scaler_state->scalers[i];
> >  
> > @@ -15297,16 +15300,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 +15341,9 @@ static int intel_crtc_init(struct drm_i915_private
> > *dev_priv, enum pipe pipe)
> >  
> >  	intel_crtc->wm.cxsr_allowed = true;
> >  
> > +	/* initialize shared scalers */
> > +	intel_crtc_init_scalers(intel_crtc, crtc_state);
> > +
> >  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
> >  	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
> >  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = intel_crtc;
Nabendu Maiti Jan. 2, 2017, 1:21 p.m. UTC | #4
Sure, Thank you Ander.

On 1/2/2017 6:34 PM, Ander Conselvan De Oliveira wrote:
> On Mon, 2017-01-02 at 15:00 +0200, Ander Conselvan De Oliveira wrote:
>> On Tue, 2016-11-29 at 11:23 +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.
>>>
>>> v3: Changed skl_init_scaler to intel_crtc_init_scalers
>>>
>>> v2: Added Chris's comments.
>>>
>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_drv.h          |  1 +
>>>  drivers/gpu/drm/i915/intel_device_info.c |  3 +++
>>>  drivers/gpu/drm/i915/intel_display.c     | 26 +++++++++++---------------
>>>  3 files changed, 15 insertions(+), 15 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..46de54a 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -115,8 +115,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>>>  			    const struct intel_crtc_state *pipe_config);
>>>  static void intel_begin_crtc_commit(struct drm_crtc *, struct
>>> drm_crtc_state *);
>>>  static void intel_finish_crtc_commit(struct drm_crtc *, struct
>>> drm_crtc_state *);
>>> -static void skl_init_scalers(struct drm_i915_private *dev_priv,
>>> -			     struct intel_crtc *crtc,
>>> +static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>>>  			     struct intel_crtc_state *crtc_state);
>> This line is now misaligned.
>>
>>>
>>>  static void skylake_pfit_enable(struct intel_crtc *crtc);
>>>  static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
>>> @@ -10713,7 +10712,7 @@ static bool haswell_get_pipe_config(struct
>>> intel_crtc *crtc,
>>>  		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>>>
>>>  	if (INTEL_GEN(dev_priv) >= 9) {
>>> -		skl_init_scalers(dev_priv, crtc, pipe_config);
>>> +		intel_crtc_init_scalers(crtc, pipe_config);
>>>
>>>  		pipe_config->scaler_state.scaler_id = -1;
>>>  		pipe_config->scaler_state.scaler_users &= ~(1 <<
>>> SKL_CRTC_INDEX);
>>> @@ -15258,14 +15257,18 @@ intel_cursor_plane_create(struct drm_i915_private
>>> *dev_priv, enum pipe pipe)
>>>  	return ERR_PTR(ret);
>>>  }
>>>
>>> -static void skl_init_scalers(struct drm_i915_private *dev_priv,
>>> -			     struct intel_crtc *crtc,
>>> +static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>>>  			     struct intel_crtc_state *crtc_state)
>> Here too.
>>
>> With that fixed,
>>
>> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
>
> Pushed.
>
> I fixed the coding style issues while pushing, but please observe these in the
> future.
>
> Ander
>
>>
>>>
>>>  {
>>>  	struct intel_crtc_scaler_state *scaler_state =
>>>  		&crtc_state->scaler_state;
>>> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>>  	int i;
>>>
>>> +	crtc->num_scalers = dev_priv->info.num_scalers[crtc->pipe];
>>> +	if (!crtc->num_scalers)
>>> +		return;
>>> +
>>>  	for (i = 0; i < crtc->num_scalers; i++) {
>>>  		struct intel_scaler *scaler = &scaler_state->scalers[i];
>>>
>>> @@ -15297,16 +15300,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 +15341,9 @@ static int intel_crtc_init(struct drm_i915_private
>>> *dev_priv, enum pipe pipe)
>>>
>>>  	intel_crtc->wm.cxsr_allowed = true;
>>>
>>> +	/* initialize shared scalers */
>>> +	intel_crtc_init_scalers(intel_crtc, crtc_state);
>>> +
>>>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>>>  	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>>>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = intel_crtc;
Ander Conselvan de Oliveira Jan. 2, 2017, 1:45 p.m. UTC | #5
On Tue, 2016-11-29 at 11:23 +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.
> 
> v3: Changed skl_init_scaler to intel_crtc_init_scalers
> 
> v2: Added Chris's comments.
> 
> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |  1 +
>  drivers/gpu/drm/i915/intel_device_info.c |  3 +++
>  drivers/gpu/drm/i915/intel_display.c     | 26 +++++++++++---------------
>  3 files changed, 15 insertions(+), 15 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;

*double facepalm*

I missed this only initializes num_scalers for bxt, thus breaking skl and glk.

Ander

>  	} 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..46de54a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -115,8 +115,7 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
>  			    const struct intel_crtc_state *pipe_config);
>  static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state
> *);
>  static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state
> *);
> -static void skl_init_scalers(struct drm_i915_private *dev_priv,
> -			     struct intel_crtc *crtc,
> +static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>  			     struct intel_crtc_state *crtc_state);
>  static void skylake_pfit_enable(struct intel_crtc *crtc);
>  static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
> @@ -10713,7 +10712,7 @@ static bool haswell_get_pipe_config(struct intel_crtc
> *crtc,
>  		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>  
>  	if (INTEL_GEN(dev_priv) >= 9) {
> -		skl_init_scalers(dev_priv, crtc, pipe_config);
> +		intel_crtc_init_scalers(crtc, pipe_config);
>  
>  		pipe_config->scaler_state.scaler_id = -1;
>  		pipe_config->scaler_state.scaler_users &= ~(1 <<
> SKL_CRTC_INDEX);
> @@ -15258,14 +15257,18 @@ intel_cursor_plane_create(struct drm_i915_private
> *dev_priv, enum pipe pipe)
>  	return ERR_PTR(ret);
>  }
>  
> -static void skl_init_scalers(struct drm_i915_private *dev_priv,
> -			     struct intel_crtc *crtc,
> +static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>  			     struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	int i;
>  
> +	crtc->num_scalers = dev_priv->info.num_scalers[crtc->pipe];
> +	if (!crtc->num_scalers)
> +		return;
> +
>  	for (i = 0; i < crtc->num_scalers; i++) {
>  		struct intel_scaler *scaler = &scaler_state->scalers[i];
>  
> @@ -15297,16 +15300,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 +15341,9 @@ static int intel_crtc_init(struct drm_i915_private
> *dev_priv, enum pipe pipe)
>  
>  	intel_crtc->wm.cxsr_allowed = true;
>  
> +	/* initialize shared scalers */
> +	intel_crtc_init_scalers(intel_crtc, crtc_state);
> +
>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>  	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = intel_crtc;
diff mbox

Patch

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..46de54a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -115,8 +115,7 @@  static void chv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
-static void skl_init_scalers(struct drm_i915_private *dev_priv,
-			     struct intel_crtc *crtc,
+static void intel_crtc_init_scalers(struct intel_crtc *crtc,
 			     struct intel_crtc_state *crtc_state);
 static void skylake_pfit_enable(struct intel_crtc *crtc);
 static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
@@ -10713,7 +10712,7 @@  static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
-		skl_init_scalers(dev_priv, crtc, pipe_config);
+		intel_crtc_init_scalers(crtc, pipe_config);
 
 		pipe_config->scaler_state.scaler_id = -1;
 		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
@@ -15258,14 +15257,18 @@  intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	return ERR_PTR(ret);
 }
 
-static void skl_init_scalers(struct drm_i915_private *dev_priv,
-			     struct intel_crtc *crtc,
+static void intel_crtc_init_scalers(struct intel_crtc *crtc,
 			     struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int i;
 
+	crtc->num_scalers = dev_priv->info.num_scalers[crtc->pipe];
+	if (!crtc->num_scalers)
+		return;
+
 	for (i = 0; i < crtc->num_scalers; i++) {
 		struct intel_scaler *scaler = &scaler_state->scalers[i];
 
@@ -15297,16 +15300,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 +15341,9 @@  static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 	intel_crtc->wm.cxsr_allowed = true;
 
+	/* initialize shared scalers */
+	intel_crtc_init_scalers(intel_crtc, crtc_state);
+
 	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
 	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
 	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = intel_crtc;