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@xxxxxxxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
