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 <[email protected]> >> --- >> 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 >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
