diff mbox series

[04/11] drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v3.

Message ID 20191114160522.9699-4-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [01/11] HAX to make DSC work on the icelake test system | expand

Commit Message

Maarten Lankhorst Nov. 14, 2019, 4:05 p.m. UTC
Small changes to intel_dp_mode_valid(), allow listing modes that
can only be supported in the bigjoiner configuration, which is
not supported yet.

eDP does not support bigjoiner, so do not expose bigjoiner only
modes on the eDP port.

Changes since v1:
- Disallow bigjoiner on eDP.
Changes since v2:
- Rename intel_dp_downstream_max_dotclock to intel_dp_max_dotclock,
  and split off the downstream and source checking to its own function.
  (Ville)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 117 ++++++++++++++++++------
 1 file changed, 89 insertions(+), 28 deletions(-)

Comments

Ville Syrjälä Nov. 28, 2019, 7:04 p.m. UTC | #1
On Thu, Nov 14, 2019 at 05:05:15PM +0100, Maarten Lankhorst wrote:
> Small changes to intel_dp_mode_valid(), allow listing modes that
> can only be supported in the bigjoiner configuration, which is
> not supported yet.
> 
> eDP does not support bigjoiner, so do not expose bigjoiner only
> modes on the eDP port.
> 
> Changes since v1:
> - Disallow bigjoiner on eDP.
> Changes since v2:
> - Rename intel_dp_downstream_max_dotclock to intel_dp_max_dotclock,
>   and split off the downstream and source checking to its own function.
>   (Ville)
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 117 ++++++++++++++++++------
>  1 file changed, 89 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 3123958e2081..9b7df8e85ea2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -243,25 +243,37 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
>  	return max_link_clock * max_lanes;
>  }
>  
> -static int
> -intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp)
> +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct intel_encoder *encoder = &intel_dig_port->base;
> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	int max_dotclk = dev_priv->max_dotclk_freq;
> -	int ds_max_dotclk;
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +
> +	if (allow_bigjoiner && INTEL_GEN(i915) >= 11 && !intel_dp_is_edp(intel_dp))

Should the edp check actually be check for the edp transcoder
(ie. port A) on icl?

> +		return 2 * i915->max_dotclk_freq;
> +
> +	return i915->max_dotclk_freq;
> +}
>  
> +static int downstream_max_dotclock(struct intel_dp *intel_dp)
> +{
>  	int type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
>  
>  	if (type != DP_DS_PORT_TYPE_VGA)
> -		return max_dotclk;
> +		return 0;
>  
> -	ds_max_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
> -						    intel_dp->downstream_ports);
> +	return drm_dp_downstream_max_clock(intel_dp->dpcd,
> +					   intel_dp->downstream_ports);
> +}
> +
> +static int
> +intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> +{
> +	int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
> +	int ds_max_dotclk = downstream_max_dotclock(intel_dp);
>  
>  	if (ds_max_dotclk != 0)
> -		max_dotclk = min(max_dotclk, ds_max_dotclk);
> +		return min(max_dotclk, ds_max_dotclk);
>  
>  	return max_dotclk;
>  }
> @@ -506,7 +518,8 @@ small_joiner_ram_size_bits(struct drm_i915_private *i915)
>  
>  static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>  				       u32 link_clock, u32 lane_count,
> -				       u32 mode_clock, u32 mode_hdisplay)
> +				       u32 mode_clock, u32 mode_hdisplay,
> +				       bool bigjoiner)
>  {
>  	u32 bits_per_pixel, max_bpp_small_joiner_ram;
>  	int i;
> @@ -524,6 +537,10 @@ static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>  	/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
>  	max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) /
>  		mode_hdisplay;
> +
> +	if (bigjoiner)
> +		max_bpp_small_joiner_ram *= 2;
> +
>  	DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
>  
>  	/*
> @@ -532,6 +549,15 @@ static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>  	 */
>  	bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
>  
> +	if (bigjoiner) {
> +		u32 max_bpp_bigjoiner =
> +			i915->max_cdclk_freq * 48 /
> +			intel_dp_mode_to_fec_clock(mode_clock);
> +
> +		DRM_DEBUG_KMS("Max big joiner bpp: %u\n", max_bpp_bigjoiner);
> +		bits_per_pixel = min(bits_per_pixel, max_bpp_bigjoiner);
> +	}
> +
>  	/* Error out if the max bpp is less than smallest allowed valid bpp */
>  	if (bits_per_pixel < valid_dsc_bpp[0]) {
>  		DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
> @@ -554,7 +580,8 @@ static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>  }
>  
>  static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> -				       int mode_clock, int mode_hdisplay)
> +				       int mode_clock, int mode_hdisplay,
> +				       bool bigjoiner)
>  {
>  	u8 min_slice_count, i;
>  	int max_slice_width;
> @@ -579,12 +606,20 @@ static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
>  
>  	/* Find the closest match to the valid slice count values */
>  	for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
> -		if (valid_dsc_slicecount[i] >
> -		    drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> -						    false))
> +		u8 test_slice_count = bigjoiner ?

The test_ prefix is throwing me off. Just slice_count?

> +			2 * valid_dsc_slicecount[i] :
> +			valid_dsc_slicecount[i];
> +
> +		if (test_slice_count >
> +		    drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd, false))
>  			break;
> -		if (min_slice_count  <= valid_dsc_slicecount[i])
> -			return valid_dsc_slicecount[i];
> +
> +		/* big joiner needs small joiner to be enabled */
> +		if (bigjoiner && test_slice_count < 4)
> +			continue;
> +
> +		if (min_slice_count <= test_slice_count)

slice_count > min_slice_count? Would feel a bit more naturalto me.

> +			return test_slice_count;
>  	}
>  
>  	DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
> @@ -623,11 +658,15 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  	int max_dotclk;
>  	u16 dsc_max_output_bpp = 0;
>  	u8 dsc_slice_count = 0;
> +	bool dsc = false, bigjoiner = false;
>  
>  	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return MODE_NO_DBLESCAN;
>  
> -	max_dotclk = intel_dp_downstream_max_dotclock(intel_dp);
> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> +		return MODE_H_ILLEGAL;
> +
> +	max_dotclk = intel_dp_max_dotclock(intel_dp, false);
>  
>  	if (intel_dp_is_edp(intel_dp) && fixed_mode) {
>  		if (mode->hdisplay > fixed_mode->hdisplay)
> @@ -639,6 +678,21 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  		target_clock = fixed_mode->clock;
>  	}
>  
> +	if (mode->clock < 10000)
> +		return MODE_CLOCK_LOW;
> +
> +	if (target_clock > max_dotclk) {
> +		if (intel_dp_is_edp(intel_dp))
> +			return MODE_CLOCK_HIGH;
> +
> +		max_dotclk = intel_dp_max_dotclock(intel_dp, true);
> +
> +		if (target_clock > max_dotclk)
> +			return MODE_CLOCK_HIGH;
> +
> +		bigjoiner = true;
> +	}
> +
>  	max_link_clock = intel_dp_max_link_rate(intel_dp);
>  	max_lanes = intel_dp_max_lane_count(intel_dp);
>  
> @@ -666,23 +720,28 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  							    max_link_clock,
>  							    max_lanes,
>  							    target_clock,
> -							    mode->hdisplay) >> 4;
> +							    mode->hdisplay,
> +							    bigjoiner) >> 4;
>  			dsc_slice_count =
>  				intel_dp_dsc_get_slice_count(intel_dp,
>  							     target_clock,
> -							     mode->hdisplay);
> +							     mode->hdisplay,
> +							     bigjoiner);
>  		}
> +
> +		dsc = dsc_max_output_bpp && dsc_slice_count;
>  	}
>  
> -	if ((mode_rate > max_rate && !(dsc_max_output_bpp && dsc_slice_count)) ||
> -	    target_clock > max_dotclk)
> +	/* big joiner configuration needs DSC */
> +	if (bigjoiner && !dsc) {
> +		DRM_DEBUG_KMS("Link clock needs bigjoiner, but DSC or FEC not available\n");

I don't think we spew debugs from mode_valid() elsewhere.

>  		return MODE_CLOCK_HIGH;
> +	}
>  
> -	if (mode->clock < 10000)
> -		return MODE_CLOCK_LOW;
> -
> -	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> -		return MODE_H_ILLEGAL;
> +	if (mode_rate > max_rate && !dsc) {

Not a huge fan of this dsc boolean. Would feel more natural if the dsc
thing would compute a new mode_rate with max achievable compression,
or something along those lines. But this could be cleaned up later.

> +		DRM_DEBUG_KMS("Cannot drive without DSC\n");
> +		return MODE_CLOCK_HIGH;
> +	}
>  
>  	return intel_mode_valid_max_plane_size(dev_priv, mode);
>  }
> @@ -2104,11 +2163,13 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  						    pipe_config->port_clock,
>  						    pipe_config->lane_count,
>  						    adjusted_mode->crtc_clock,
> -						    adjusted_mode->crtc_hdisplay);
> +						    adjusted_mode->crtc_hdisplay,
> +						    false);
>  		dsc_dp_slice_count =
>  			intel_dp_dsc_get_slice_count(intel_dp,
>  						     adjusted_mode->crtc_clock,
> -						     adjusted_mode->crtc_hdisplay);
> +						     adjusted_mode->crtc_hdisplay,
> +						     false);
>  		if (!dsc_max_output_bpp || !dsc_dp_slice_count) {
>  			DRM_DEBUG_KMS("Compressed BPP/Slice Count not supported\n");
>  			return -EINVAL;
> -- 
> 2.24.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst Dec. 3, 2019, 9:18 a.m. UTC | #2
Op 28-11-2019 om 20:04 schreef Ville Syrjälä:
> On Thu, Nov 14, 2019 at 05:05:15PM +0100, Maarten Lankhorst wrote:
>> Small changes to intel_dp_mode_valid(), allow listing modes that
>> can only be supported in the bigjoiner configuration, which is
>> not supported yet.
>>
>> eDP does not support bigjoiner, so do not expose bigjoiner only
>> modes on the eDP port.
>>
>> Changes since v1:
>> - Disallow bigjoiner on eDP.
>> Changes since v2:
>> - Rename intel_dp_downstream_max_dotclock to intel_dp_max_dotclock,
>>   and split off the downstream and source checking to its own function.
>>   (Ville)
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp.c | 117 ++++++++++++++++++------
>>  1 file changed, 89 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 3123958e2081..9b7df8e85ea2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -243,25 +243,37 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
>>  	return max_link_clock * max_lanes;
>>  }
>>  
>> -static int
>> -intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp)
>> +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
>>  {
>>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>  	struct intel_encoder *encoder = &intel_dig_port->base;
>> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> -	int max_dotclk = dev_priv->max_dotclk_freq;
>> -	int ds_max_dotclk;
>> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> +
>> +	if (allow_bigjoiner && INTEL_GEN(i915) >= 11 && !intel_dp_is_edp(intel_dp))
> Should the edp check actually be check for the edp transcoder
> (ie. port A) on icl?
Isn't that equivalent to this check?
>
>> +		return 2 * i915->max_dotclk_freq;
>> +
>> +	return i915->max_dotclk_freq;
>> +}
>>  
>> +static int downstream_max_dotclock(struct intel_dp *intel_dp)
>> +{
>>  	int type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
>>  
>>  	if (type != DP_DS_PORT_TYPE_VGA)
>> -		return max_dotclk;
>> +		return 0;
>>  
>> -	ds_max_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
>> -						    intel_dp->downstream_ports);
>> +	return drm_dp_downstream_max_clock(intel_dp->dpcd,
>> +					   intel_dp->downstream_ports);
>> +}
>> +
>> +static int
>> +intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
>> +{
>> +	int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
>> +	int ds_max_dotclk = downstream_max_dotclock(intel_dp);
>>  
>>  	if (ds_max_dotclk != 0)
>> -		max_dotclk = min(max_dotclk, ds_max_dotclk);
>> +		return min(max_dotclk, ds_max_dotclk);
>>  
>>  	return max_dotclk;
>>  }
>> @@ -506,7 +518,8 @@ small_joiner_ram_size_bits(struct drm_i915_private *i915)
>>  
>>  static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>>  				       u32 link_clock, u32 lane_count,
>> -				       u32 mode_clock, u32 mode_hdisplay)
>> +				       u32 mode_clock, u32 mode_hdisplay,
>> +				       bool bigjoiner)
>>  {
>>  	u32 bits_per_pixel, max_bpp_small_joiner_ram;
>>  	int i;
>> @@ -524,6 +537,10 @@ static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>>  	/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
>>  	max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) /
>>  		mode_hdisplay;
>> +
>> +	if (bigjoiner)
>> +		max_bpp_small_joiner_ram *= 2;
>> +
>>  	DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
>>  
>>  	/*
>> @@ -532,6 +549,15 @@ static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>>  	 */
>>  	bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
>>  
>> +	if (bigjoiner) {
>> +		u32 max_bpp_bigjoiner =
>> +			i915->max_cdclk_freq * 48 /
>> +			intel_dp_mode_to_fec_clock(mode_clock);
>> +
>> +		DRM_DEBUG_KMS("Max big joiner bpp: %u\n", max_bpp_bigjoiner);
>> +		bits_per_pixel = min(bits_per_pixel, max_bpp_bigjoiner);
>> +	}
>> +
>>  	/* Error out if the max bpp is less than smallest allowed valid bpp */
>>  	if (bits_per_pixel < valid_dsc_bpp[0]) {
>>  		DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
>> @@ -554,7 +580,8 @@ static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>>  }
>>  
>>  static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
>> -				       int mode_clock, int mode_hdisplay)
>> +				       int mode_clock, int mode_hdisplay,
>> +				       bool bigjoiner)
>>  {
>>  	u8 min_slice_count, i;
>>  	int max_slice_width;
>> @@ -579,12 +606,20 @@ static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
>>  
>>  	/* Find the closest match to the valid slice count values */
>>  	for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
>> -		if (valid_dsc_slicecount[i] >
>> -		    drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
>> -						    false))
>> +		u8 test_slice_count = bigjoiner ?
> The test_ prefix is throwing me off. Just slice_count?
>
>> +			2 * valid_dsc_slicecount[i] :
>> +			valid_dsc_slicecount[i];
>> +
>> +		if (test_slice_count >
>> +		    drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd, false))
>>  			break;
>> -		if (min_slice_count  <= valid_dsc_slicecount[i])
>> -			return valid_dsc_slicecount[i];
>> +
>> +		/* big joiner needs small joiner to be enabled */
>> +		if (bigjoiner && test_slice_count < 4)
>> +			continue;
>> +
>> +		if (min_slice_count <= test_slice_count)
> slice_count > min_slice_count? Would feel a bit more naturalto me.
Yeah makes sense.
>> +			return test_slice_count;
>>  	}
>>  
>>  	DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
>> @@ -623,11 +658,15 @@ intel_dp_mode_valid(struct drm_connector *connector,
>>  	int max_dotclk;
>>  	u16 dsc_max_output_bpp = 0;
>>  	u8 dsc_slice_count = 0;
>> +	bool dsc = false, bigjoiner = false;
>>  
>>  	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
>>  		return MODE_NO_DBLESCAN;
>>  
>> -	max_dotclk = intel_dp_downstream_max_dotclock(intel_dp);
>> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
>> +		return MODE_H_ILLEGAL;
>> +
>> +	max_dotclk = intel_dp_max_dotclock(intel_dp, false);
>>  
>>  	if (intel_dp_is_edp(intel_dp) && fixed_mode) {
>>  		if (mode->hdisplay > fixed_mode->hdisplay)
>> @@ -639,6 +678,21 @@ intel_dp_mode_valid(struct drm_connector *connector,
>>  		target_clock = fixed_mode->clock;
>>  	}
>>  
>> +	if (mode->clock < 10000)
>> +		return MODE_CLOCK_LOW;
>> +
>> +	if (target_clock > max_dotclk) {
>> +		if (intel_dp_is_edp(intel_dp))
>> +			return MODE_CLOCK_HIGH;
>> +
>> +		max_dotclk = intel_dp_max_dotclock(intel_dp, true);
>> +
>> +		if (target_clock > max_dotclk)
>> +			return MODE_CLOCK_HIGH;
>> +
>> +		bigjoiner = true;
>> +	}
>> +
>>  	max_link_clock = intel_dp_max_link_rate(intel_dp);
>>  	max_lanes = intel_dp_max_lane_count(intel_dp);
>>  
>> @@ -666,23 +720,28 @@ intel_dp_mode_valid(struct drm_connector *connector,
>>  							    max_link_clock,
>>  							    max_lanes,
>>  							    target_clock,
>> -							    mode->hdisplay) >> 4;
>> +							    mode->hdisplay,
>> +							    bigjoiner) >> 4;
>>  			dsc_slice_count =
>>  				intel_dp_dsc_get_slice_count(intel_dp,
>>  							     target_clock,
>> -							     mode->hdisplay);
>> +							     mode->hdisplay,
>> +							     bigjoiner);
>>  		}
>> +
>> +		dsc = dsc_max_output_bpp && dsc_slice_count;
>>  	}
>>  
>> -	if ((mode_rate > max_rate && !(dsc_max_output_bpp && dsc_slice_count)) ||
>> -	    target_clock > max_dotclk)
>> +	/* big joiner configuration needs DSC */
>> +	if (bigjoiner && !dsc) {
>> +		DRM_DEBUG_KMS("Link clock needs bigjoiner, but DSC or FEC not available\n");
> I don't think we spew debugs from mode_valid() elsewhere.
Good for debugging, but yeah can kill debug messages if wanted.
>>  		return MODE_CLOCK_HIGH;
>> +	}
>>  
>> -	if (mode->clock < 10000)
>> -		return MODE_CLOCK_LOW;
>> -
>> -	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
>> -		return MODE_H_ILLEGAL;
>> +	if (mode_rate > max_rate && !dsc) {
> Not a huge fan of this dsc boolean. Would feel more natural if the dsc
> thing would compute a new mode_rate with max achievable compression,
> or something along those lines. But this could be cleaned up later.
Yeah.
>
>> +		DRM_DEBUG_KMS("Cannot drive without DSC\n");
>> +		return MODE_CLOCK_HIGH;
>> +	}
>>  
>>  	return intel_mode_valid_max_plane_size(dev_priv, mode);
>>  }
>> @@ -2104,11 +2163,13 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>>  						    pipe_config->port_clock,
>>  						    pipe_config->lane_count,
>>  						    adjusted_mode->crtc_clock,
>> -						    adjusted_mode->crtc_hdisplay);
>> +						    adjusted_mode->crtc_hdisplay,
>> +						    false);
>>  		dsc_dp_slice_count =
>>  			intel_dp_dsc_get_slice_count(intel_dp,
>>  						     adjusted_mode->crtc_clock,
>> -						     adjusted_mode->crtc_hdisplay);
>> +						     adjusted_mode->crtc_hdisplay,
>> +						     false);
>>  		if (!dsc_max_output_bpp || !dsc_dp_slice_count) {
>>  			DRM_DEBUG_KMS("Compressed BPP/Slice Count not supported\n");
>>  			return -EINVAL;
>> -- 
>> 2.24.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Dec. 3, 2019, 1:39 p.m. UTC | #3
On Tue, Dec 03, 2019 at 10:18:51AM +0100, Maarten Lankhorst wrote:
> Op 28-11-2019 om 20:04 schreef Ville Syrjälä:
> > On Thu, Nov 14, 2019 at 05:05:15PM +0100, Maarten Lankhorst wrote:
> >> Small changes to intel_dp_mode_valid(), allow listing modes that
> >> can only be supported in the bigjoiner configuration, which is
> >> not supported yet.
> >>
> >> eDP does not support bigjoiner, so do not expose bigjoiner only
> >> modes on the eDP port.
> >>
> >> Changes since v1:
> >> - Disallow bigjoiner on eDP.
> >> Changes since v2:
> >> - Rename intel_dp_downstream_max_dotclock to intel_dp_max_dotclock,
> >>   and split off the downstream and source checking to its own function.
> >>   (Ville)
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_dp.c | 117 ++++++++++++++++++------
> >>  1 file changed, 89 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index 3123958e2081..9b7df8e85ea2 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -243,25 +243,37 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> >>  	return max_link_clock * max_lanes;
> >>  }
> >>  
> >> -static int
> >> -intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp)
> >> +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> >>  {
> >>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>  	struct intel_encoder *encoder = &intel_dig_port->base;
> >> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >> -	int max_dotclk = dev_priv->max_dotclk_freq;
> >> -	int ds_max_dotclk;
> >> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >> +
> >> +	if (allow_bigjoiner && INTEL_GEN(i915) >= 11 && !intel_dp_is_edp(intel_dp))
> > Should the edp check actually be check for the edp transcoder
> > (ie. port A) on icl?
> Isn't that equivalent to this check?

You can have eDP on other ports (at least in theory).
Also on tgl+ there is no eDP transcoder anymore.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 3123958e2081..9b7df8e85ea2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -243,25 +243,37 @@  intel_dp_max_data_rate(int max_link_clock, int max_lanes)
 	return max_link_clock * max_lanes;
 }
 
-static int
-intel_dp_downstream_max_dotclock(struct intel_dp *intel_dp)
+static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *encoder = &intel_dig_port->base;
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	int max_dotclk = dev_priv->max_dotclk_freq;
-	int ds_max_dotclk;
+	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+
+	if (allow_bigjoiner && INTEL_GEN(i915) >= 11 && !intel_dp_is_edp(intel_dp))
+		return 2 * i915->max_dotclk_freq;
+
+	return i915->max_dotclk_freq;
+}
 
+static int downstream_max_dotclock(struct intel_dp *intel_dp)
+{
 	int type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
 
 	if (type != DP_DS_PORT_TYPE_VGA)
-		return max_dotclk;
+		return 0;
 
-	ds_max_dotclk = drm_dp_downstream_max_clock(intel_dp->dpcd,
-						    intel_dp->downstream_ports);
+	return drm_dp_downstream_max_clock(intel_dp->dpcd,
+					   intel_dp->downstream_ports);
+}
+
+static int
+intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
+{
+	int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
+	int ds_max_dotclk = downstream_max_dotclock(intel_dp);
 
 	if (ds_max_dotclk != 0)
-		max_dotclk = min(max_dotclk, ds_max_dotclk);
+		return min(max_dotclk, ds_max_dotclk);
 
 	return max_dotclk;
 }
@@ -506,7 +518,8 @@  small_joiner_ram_size_bits(struct drm_i915_private *i915)
 
 static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
 				       u32 link_clock, u32 lane_count,
-				       u32 mode_clock, u32 mode_hdisplay)
+				       u32 mode_clock, u32 mode_hdisplay,
+				       bool bigjoiner)
 {
 	u32 bits_per_pixel, max_bpp_small_joiner_ram;
 	int i;
@@ -524,6 +537,10 @@  static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
 	/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
 	max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) /
 		mode_hdisplay;
+
+	if (bigjoiner)
+		max_bpp_small_joiner_ram *= 2;
+
 	DRM_DEBUG_KMS("Max small joiner bpp: %u\n", max_bpp_small_joiner_ram);
 
 	/*
@@ -532,6 +549,15 @@  static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
 	 */
 	bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
 
+	if (bigjoiner) {
+		u32 max_bpp_bigjoiner =
+			i915->max_cdclk_freq * 48 /
+			intel_dp_mode_to_fec_clock(mode_clock);
+
+		DRM_DEBUG_KMS("Max big joiner bpp: %u\n", max_bpp_bigjoiner);
+		bits_per_pixel = min(bits_per_pixel, max_bpp_bigjoiner);
+	}
+
 	/* Error out if the max bpp is less than smallest allowed valid bpp */
 	if (bits_per_pixel < valid_dsc_bpp[0]) {
 		DRM_DEBUG_KMS("Unsupported BPP %u, min %u\n",
@@ -554,7 +580,8 @@  static u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
 }
 
 static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
-				       int mode_clock, int mode_hdisplay)
+				       int mode_clock, int mode_hdisplay,
+				       bool bigjoiner)
 {
 	u8 min_slice_count, i;
 	int max_slice_width;
@@ -579,12 +606,20 @@  static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
 
 	/* Find the closest match to the valid slice count values */
 	for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
-		if (valid_dsc_slicecount[i] >
-		    drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
-						    false))
+		u8 test_slice_count = bigjoiner ?
+			2 * valid_dsc_slicecount[i] :
+			valid_dsc_slicecount[i];
+
+		if (test_slice_count >
+		    drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd, false))
 			break;
-		if (min_slice_count  <= valid_dsc_slicecount[i])
-			return valid_dsc_slicecount[i];
+
+		/* big joiner needs small joiner to be enabled */
+		if (bigjoiner && test_slice_count < 4)
+			continue;
+
+		if (min_slice_count <= test_slice_count)
+			return test_slice_count;
 	}
 
 	DRM_DEBUG_KMS("Unsupported Slice Count %d\n", min_slice_count);
@@ -623,11 +658,15 @@  intel_dp_mode_valid(struct drm_connector *connector,
 	int max_dotclk;
 	u16 dsc_max_output_bpp = 0;
 	u8 dsc_slice_count = 0;
+	bool dsc = false, bigjoiner = false;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return MODE_NO_DBLESCAN;
 
-	max_dotclk = intel_dp_downstream_max_dotclock(intel_dp);
+	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
+		return MODE_H_ILLEGAL;
+
+	max_dotclk = intel_dp_max_dotclock(intel_dp, false);
 
 	if (intel_dp_is_edp(intel_dp) && fixed_mode) {
 		if (mode->hdisplay > fixed_mode->hdisplay)
@@ -639,6 +678,21 @@  intel_dp_mode_valid(struct drm_connector *connector,
 		target_clock = fixed_mode->clock;
 	}
 
+	if (mode->clock < 10000)
+		return MODE_CLOCK_LOW;
+
+	if (target_clock > max_dotclk) {
+		if (intel_dp_is_edp(intel_dp))
+			return MODE_CLOCK_HIGH;
+
+		max_dotclk = intel_dp_max_dotclock(intel_dp, true);
+
+		if (target_clock > max_dotclk)
+			return MODE_CLOCK_HIGH;
+
+		bigjoiner = true;
+	}
+
 	max_link_clock = intel_dp_max_link_rate(intel_dp);
 	max_lanes = intel_dp_max_lane_count(intel_dp);
 
@@ -666,23 +720,28 @@  intel_dp_mode_valid(struct drm_connector *connector,
 							    max_link_clock,
 							    max_lanes,
 							    target_clock,
-							    mode->hdisplay) >> 4;
+							    mode->hdisplay,
+							    bigjoiner) >> 4;
 			dsc_slice_count =
 				intel_dp_dsc_get_slice_count(intel_dp,
 							     target_clock,
-							     mode->hdisplay);
+							     mode->hdisplay,
+							     bigjoiner);
 		}
+
+		dsc = dsc_max_output_bpp && dsc_slice_count;
 	}
 
-	if ((mode_rate > max_rate && !(dsc_max_output_bpp && dsc_slice_count)) ||
-	    target_clock > max_dotclk)
+	/* big joiner configuration needs DSC */
+	if (bigjoiner && !dsc) {
+		DRM_DEBUG_KMS("Link clock needs bigjoiner, but DSC or FEC not available\n");
 		return MODE_CLOCK_HIGH;
+	}
 
-	if (mode->clock < 10000)
-		return MODE_CLOCK_LOW;
-
-	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
-		return MODE_H_ILLEGAL;
+	if (mode_rate > max_rate && !dsc) {
+		DRM_DEBUG_KMS("Cannot drive without DSC\n");
+		return MODE_CLOCK_HIGH;
+	}
 
 	return intel_mode_valid_max_plane_size(dev_priv, mode);
 }
@@ -2104,11 +2163,13 @@  static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 						    pipe_config->port_clock,
 						    pipe_config->lane_count,
 						    adjusted_mode->crtc_clock,
-						    adjusted_mode->crtc_hdisplay);
+						    adjusted_mode->crtc_hdisplay,
+						    false);
 		dsc_dp_slice_count =
 			intel_dp_dsc_get_slice_count(intel_dp,
 						     adjusted_mode->crtc_clock,
-						     adjusted_mode->crtc_hdisplay);
+						     adjusted_mode->crtc_hdisplay,
+						     false);
 		if (!dsc_max_output_bpp || !dsc_dp_slice_count) {
 			DRM_DEBUG_KMS("Compressed BPP/Slice Count not supported\n");
 			return -EINVAL;