[Intel-gfx] [PATCH] drm/i915: Clean up scaler setup, v2.

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Fri Sep 21 17:32:12 UTC 2018


Op 21-09-18 om 18:40 schreef Matt Roper:
> On Fri, Sep 21, 2018 at 04:44:37PM +0200, Maarten Lankhorst wrote:
>> On skylake we can switch to a high quality scaler mode when only 1 out
>> of 2 scalers are used, but on GLK and later bit 28 has a different
>> meaning. Don't set it, and make clear the distinction between
>> SKL and later PS values.
>>
>> Changes since v1:
>> - Add missing break statement.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> Reviewed-by: Matt Roper <matthew.d.roper at intel.com> #v1
> R-b for v2 as well.
>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h      |   7 +-
>>  drivers/gpu/drm/i915/intel_atomic.c  | 109 +++++++++++++++------------
>>  drivers/gpu/drm/i915/intel_display.c |   2 +-
>>  3 files changed, 67 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 4948b352bf4c..e7e6ca7f9665 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6850,11 +6850,12 @@ enum {
>>  #define _PS_2B_CTRL      0x68A80
>>  #define _PS_1C_CTRL      0x69180
>>  #define PS_SCALER_EN        (1 << 31)
>> -#define PS_SCALER_MODE_MASK (3 << 28)
>> -#define PS_SCALER_MODE_DYN  (0 << 28)
>> -#define PS_SCALER_MODE_HQ  (1 << 28)
>> +#define SKL_PS_SCALER_MODE_MASK (3 << 28)
>> +#define SKL_PS_SCALER_MODE_DYN  (0 << 28)
>> +#define SKL_PS_SCALER_MODE_HQ  (1 << 28)
>>  #define SKL_PS_SCALER_MODE_NV12 (2 << 28)
>>  #define PS_SCALER_MODE_PLANAR (1 << 29)
>> +#define PS_SCALER_MODE_PACKED (0 << 29)
>>  #define PS_PLANE_SEL_MASK  (7 << 25)
>>  #define PS_PLANE_SEL(plane) (((plane) + 1) << 25)
>>  #define PS_FILTER_MASK         (3 << 23)
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>> index b04952bacf77..c23a6ec40f60 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -203,6 +203,63 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
>>  	drm_atomic_helper_crtc_destroy_state(crtc, state);
>>  }
>>  
>> +static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_state,
>> +				      int num_scalers_need, struct intel_crtc *intel_crtc,
>> +				      const char *name, int idx,
>> +				      struct intel_plane_state *plane_state,
>> +				      int *scaler_id)
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>> +	int j;
>> +	u32 mode;
>> +
>> +	if (*scaler_id < 0) {
>> +		/* find a free scaler */
>> +		for (j = 0; j < intel_crtc->num_scalers; j++) {
>> +			if (scaler_state->scalers[j].in_use)
>> +				continue;
>> +
>> +			*scaler_id = j;
>> +			scaler_state->scalers[*scaler_id].in_use = 1;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (WARN(*scaler_id < 0, "Cannot find scaler for %s:%d\n", name, idx))
>> +		return;
>> +
>> +	/* set scaler mode */
>> +	if (plane_state && plane_state->base.fb &&
>> +	    plane_state->base.fb->format->is_yuv &&
>> +	    plane_state->base.fb->format->num_planes > 1) {
>> +		if (INTEL_GEN(dev_priv) == 9 &&
>> +		    !IS_GEMINILAKE(dev_priv))
>> +			mode = SKL_PS_SCALER_MODE_NV12;
>> +		else
>> +			mode = PS_SCALER_MODE_PLANAR;
>> +
>> +	} else if (INTEL_GEN(dev_priv) > 9 || IS_GEMINILAKE(dev_priv)) {
>> +		mode = PS_SCALER_MODE_PACKED;
>> +	} else if (num_scalers_need == 1 && intel_crtc->num_scalers > 1) {
>> +		/*
>> +		 * when only 1 scaler is in use on a pipe with 2 scalers
>> +		 * scaler 0 operates in high quality (HQ) mode.
>> +		 * In this case use scaler 0 to take advantage of HQ mode
>> +		 */
>> +		scaler_state->scalers[*scaler_id].in_use = 0;
>> +		*scaler_id = 0;
>> +		scaler_state->scalers[0].in_use = 1;
>> +		mode = SKL_PS_SCALER_MODE_HQ;
>> +	} else {
>> +		mode = SKL_PS_SCALER_MODE_DYN;
>> +	}
>> +
>> +	DRM_DEBUG_KMS("Attached scaler id %u.%u to %s:%d\n",
>> +		      intel_crtc->pipe, *scaler_id, name, idx);
>> +	scaler_state->scalers[*scaler_id].mode = mode;
>> +}
>> +
>> +
>>  /**
>>   * intel_atomic_setup_scalers() - setup scalers for crtc per staged requests
>>   * @dev_priv: i915 device
>> @@ -232,7 +289,7 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>>  	struct drm_atomic_state *drm_state = crtc_state->base.state;
>>  	struct intel_atomic_state *intel_state = to_intel_atomic_state(drm_state);
>>  	int num_scalers_need;
>> -	int i, j;
>> +	int i;
>>  
>>  	num_scalers_need = hweight32(scaler_state->scaler_users);
>>  
>> @@ -304,59 +361,17 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>>  			idx = plane->base.id;
>>  
>>  			/* plane on different crtc cannot be a scaler user of this crtc */
>> -			if (WARN_ON(intel_plane->pipe != intel_crtc->pipe)) {
>> +			if (WARN_ON(intel_plane->pipe != intel_crtc->pipe))
>>  				continue;
>> -			}
>>  
>>  			plane_state = intel_atomic_get_new_plane_state(intel_state,
>>  								       intel_plane);
>>  			scaler_id = &plane_state->scaler_id;
>>  		}
>>  
>> -		if (*scaler_id < 0) {
>> -			/* find a free scaler */
>> -			for (j = 0; j < intel_crtc->num_scalers; j++) {
>> -				if (!scaler_state->scalers[j].in_use) {
>> -					scaler_state->scalers[j].in_use = 1;
>> -					*scaler_id = j;
>> -					DRM_DEBUG_KMS("Attached scaler id %u.%u to %s:%d\n",
>> -						intel_crtc->pipe, *scaler_id, name, idx);
>> -					break;
>> -				}
>> -			}
>> -		}
>> -
>> -		if (WARN_ON(*scaler_id < 0)) {
>> -			DRM_DEBUG_KMS("Cannot find scaler for %s:%d\n", name, idx);
>> -			continue;
>> -		}
>> -
>> -		/* set scaler mode */
>> -		if ((INTEL_GEN(dev_priv) >= 9) &&
>> -		    plane_state && plane_state->base.fb &&
>> -		    plane_state->base.fb->format->format ==
>> -		    DRM_FORMAT_NV12) {
>> -			if (INTEL_GEN(dev_priv) == 9 &&
>> -			    !IS_GEMINILAKE(dev_priv) &&
>> -			    !IS_SKYLAKE(dev_priv))
>> -				scaler_state->scalers[*scaler_id].mode =
>> -					SKL_PS_SCALER_MODE_NV12;
>> -			else
>> -				scaler_state->scalers[*scaler_id].mode =
>> -					PS_SCALER_MODE_PLANAR;
>> -		} else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
>> -			/*
>> -			 * when only 1 scaler is in use on either pipe A or B,
>> -			 * scaler 0 operates in high quality (HQ) mode.
>> -			 * In this case use scaler 0 to take advantage of HQ mode
>> -			 */
>> -			*scaler_id = 0;
>> -			scaler_state->scalers[0].in_use = 1;
>> -			scaler_state->scalers[0].mode = PS_SCALER_MODE_HQ;
>> -			scaler_state->scalers[1].in_use = 0;
>> -		} else {
>> -			scaler_state->scalers[*scaler_id].mode = PS_SCALER_MODE_DYN;
>> -		}
>> +		intel_atomic_setup_scaler(scaler_state, num_scalers_need,
>> +					  intel_crtc, name, idx,
>> +					  plane_state, scaler_id);
>>  	}
>>  
>>  	return 0;
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 02374185b993..931898013506 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13963,7 +13963,7 @@ static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>>  		struct intel_scaler *scaler = &scaler_state->scalers[i];
>>  
>>  		scaler->in_use = 0;
>> -		scaler->mode = PS_SCALER_MODE_DYN;
>> +		scaler->mode = 0;
>>  	}
>>  
>>  	scaler_state->scaler_id = -1;
>> -- 
>> 2.18.0
>>
Thanks, pushed the series. :)

~Maarten



More information about the Intel-gfx mailing list