| Message ID | 20200715224222.7557-4-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6,01/11] HAX to make DSC work on the icelake test system | expand |
Op 16-07-2020 om 00:42 schreef Manasi Navare: > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > 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. > > v5: > * Increase max plane width to support 8K with bigjoiner (Maarten) > v4: > * Rebase (Manasi) > > 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) > v3: > * Rebase (Manasi) > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Looking good, but I think you should have someone else review it as well to be certain. :) > --- > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > 2 files changed, 91 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 78cbfefbfa62..3ecb642805a6 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > * too big for that. > */ > if (INTEL_GEN(dev_priv) >= 11) { > - plane_width_max = 5120; > + plane_width_max = 7680; > plane_height_max = 4320; > } else { > plane_width_max = 5120; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index d6295eb20b63..fbfea99fd804 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > - struct intel_encoder *encoder = &dig_port->base; > + 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; > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > + return 2 * dev_priv->max_dotclk_freq; > + > + return dev_priv->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; > } > @@ -527,7 +539,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; > @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > max_bpp_small_joiner_ram); > > @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > @@ -576,7 +602,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) > { > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > u8 min_slice_count, i; > @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > @@ -648,11 +683,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) > @@ -664,6 +703,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); > > @@ -691,23 +745,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); > } > @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > "Compressed BPP/Slice Count not supported\n");
On 16-07-2020 04:12, Manasi Navare wrote: > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > 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. > > v5: > * Increase max plane width to support 8K with bigjoiner (Maarten) > v4: > * Rebase (Manasi) > > 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) > v3: > * Rebase (Manasi) > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > 2 files changed, 91 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 78cbfefbfa62..3ecb642805a6 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > * too big for that. > */ > if (INTEL_GEN(dev_priv) >= 11) { > - plane_width_max = 5120; > + plane_width_max = 7680; Other encoder also use this function and big joiner on DP only need this change. Is it good idea to add encoder check? Maybe in a cover-letter can we add some description about big-joiner feature and current limitation. Overall changes looks good to me, for dsc related code better get a review from someone who has worked before. Regards, Animesh > plane_height_max = 4320; > } else { > plane_width_max = 5120; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index d6295eb20b63..fbfea99fd804 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > - struct intel_encoder *encoder = &dig_port->base; > + 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; > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > + return 2 * dev_priv->max_dotclk_freq; > + > + return dev_priv->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; > } > @@ -527,7 +539,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; > @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > max_bpp_small_joiner_ram); > > @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > @@ -576,7 +602,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) > { > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > u8 min_slice_count, i; > @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > @@ -648,11 +683,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) > @@ -664,6 +703,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); > > @@ -691,23 +745,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); > } > @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > "Compressed BPP/Slice Count not supported\n");
On Fri, Aug 21, 2020 at 03:11:45PM +0530, Manna, Animesh wrote: > > On 16-07-2020 04:12, Manasi Navare wrote: > >From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > >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. > > > >v5: > >* Increase max plane width to support 8K with bigjoiner (Maarten) > >v4: > >* Rebase (Manasi) > > > >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) > >v3: > >* Rebase (Manasi) > > > >Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > >Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > >--- > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > > 2 files changed, 91 insertions(+), 30 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > >index 78cbfefbfa62..3ecb642805a6 100644 > >--- a/drivers/gpu/drm/i915/display/intel_display.c > >+++ b/drivers/gpu/drm/i915/display/intel_display.c > >@@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > > * too big for that. > > */ > > if (INTEL_GEN(dev_priv) >= 11) { > >- plane_width_max = 5120; > >+ plane_width_max = 7680; > > > Other encoder also use this function and big joiner on DP only need this change. Is it good idea to add encoder check? Maybe in a cover-letter can we add some description about big-joiner > feature and current limitation. > Overall changes looks good to me, for dsc related code better get a review from someone who has worked before. Yes I think the plane width max also should only be changed to 7680 if big joiner is supported so this needs some tweak anyways. I will take a look into this. Also will sync with Maarten on what might be the best approach. Manasi > > Regards, > Animesh > > > plane_height_max = 4320; > > } else { > > plane_width_max = 5120; > >diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > >index d6295eb20b63..fbfea99fd804 100644 > >--- a/drivers/gpu/drm/i915/display/intel_dp.c > >+++ b/drivers/gpu/drm/i915/display/intel_dp.c > >@@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > >- struct intel_encoder *encoder = &dig_port->base; > >+ 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; > >+ if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > >+ return 2 * dev_priv->max_dotclk_freq; > >+ > >+ return dev_priv->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; > > } > >@@ -527,7 +539,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; > >@@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > > max_bpp_small_joiner_ram); > >@@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > >@@ -576,7 +602,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) > > { > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > u8 min_slice_count, i; > >@@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > >@@ -648,11 +683,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) > >@@ -664,6 +703,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); > >@@ -691,23 +745,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); > > } > >@@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > > "Compressed BPP/Slice Count not supported\n");
On Wed, Jul 15, 2020 at 03:42:15PM -0700, Manasi Navare wrote: > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > 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. > > v5: > * Increase max plane width to support 8K with bigjoiner (Maarten) > v4: > * Rebase (Manasi) > > 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) > v3: > * Rebase (Manasi) > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > 2 files changed, 91 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 78cbfefbfa62..3ecb642805a6 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > * too big for that. > */ > if (INTEL_GEN(dev_priv) >= 11) { > - plane_width_max = 5120; > + plane_width_max = 7680; This looks misplaced. Planes do no know whether bigjoiner can be used or not. They should not care in fact. The caller should have that knowledge and can deal with it properly. > plane_height_max = 4320; > } else { > plane_width_max = 5120; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index d6295eb20b63..fbfea99fd804 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > - struct intel_encoder *encoder = &dig_port->base; > + 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; > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > + return 2 * dev_priv->max_dotclk_freq; > + > + return dev_priv->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; > } > @@ -527,7 +539,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; > @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > max_bpp_small_joiner_ram); > > @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > @@ -576,7 +602,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) > { > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > u8 min_slice_count, i; > @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > @@ -648,11 +683,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) > @@ -664,6 +703,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); > > @@ -691,23 +745,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); > } > @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > "Compressed BPP/Slice Count not supported\n"); > -- > 2.19.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Sep 07, 2020 at 02:20:56PM +0300, Ville Syrjälä wrote: > On Wed, Jul 15, 2020 at 03:42:15PM -0700, Manasi Navare wrote: > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > 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. > > > > v5: > > * Increase max plane width to support 8K with bigjoiner (Maarten) > > v4: > > * Rebase (Manasi) > > > > 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) > > v3: > > * Rebase (Manasi) > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > > 2 files changed, 91 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 78cbfefbfa62..3ecb642805a6 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > > * too big for that. > > */ > > if (INTEL_GEN(dev_priv) >= 11) { > > - plane_width_max = 5120; > > + plane_width_max = 7680; > > This looks misplaced. Planes do no know whether bigjoiner can be used or > not. They should not care in fact. The caller should have that knowledge > and can deal with it properly. Hmm, so the caller of intel_mode_valid_max_plane_size() should check on the bigjoiner flag and perhaps if bigjoiner is true then increase the plane_width_max to 7680? Am still not sure where this should happen? We need to have the plane max width to be 7680 before we prune the 8K mode in intel_mode_valid Where should this be added according to you? Manasi > > > plane_height_max = 4320; > > } else { > > plane_width_max = 5120; > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index d6295eb20b63..fbfea99fd804 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > > - struct intel_encoder *encoder = &dig_port->base; > > + 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; > > > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > > + return 2 * dev_priv->max_dotclk_freq; > > + > > + return dev_priv->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; > > } > > @@ -527,7 +539,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; > > @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > > max_bpp_small_joiner_ram); > > > > @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > > @@ -576,7 +602,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) > > { > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > u8 min_slice_count, i; > > @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > > @@ -648,11 +683,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) > > @@ -664,6 +703,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); > > > > @@ -691,23 +745,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); > > } > > @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > > "Compressed BPP/Slice Count not supported\n"); > > -- > > 2.19.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
On Mon, Sep 14, 2020 at 12:00:33PM -0700, Navare, Manasi wrote: > On Mon, Sep 07, 2020 at 02:20:56PM +0300, Ville Syrjälä wrote: > > On Wed, Jul 15, 2020 at 03:42:15PM -0700, Manasi Navare wrote: > > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > 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. > > > > > > v5: > > > * Increase max plane width to support 8K with bigjoiner (Maarten) > > > v4: > > > * Rebase (Manasi) > > > > > > 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) > > > v3: > > > * Rebase (Manasi) > > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > > > 2 files changed, 91 insertions(+), 30 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > index 78cbfefbfa62..3ecb642805a6 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > > > * too big for that. > > > */ > > > if (INTEL_GEN(dev_priv) >= 11) { > > > - plane_width_max = 5120; > > > + plane_width_max = 7680; > > > > This looks misplaced. Planes do no know whether bigjoiner can be used or > > not. They should not care in fact. The caller should have that knowledge > > and can deal with it properly. > > Hmm, so the caller of intel_mode_valid_max_plane_size() should check on the bigjoiner > flag and perhaps if bigjoiner is true then increase the plane_width_max to 7680? > > Am still not sure where this should happen? We need to have the plane max width to be 7680 > before we prune the 8K mode in intel_mode_valid > > Where should this be added according to you? Hmm. I guess we do need to put it into this function given the way this is structured. However we still can't assume bigjoiner can be used since it can't be used on DDI A on icl. So we should probably just pass in a bool here to indicate whether bigjoiner can be used or not. Personally I'd just write the thing as something like: intel_mode_valid_max_plane_size(..., bool bigjoiner) { ... plane_width_max = 5120 << bigjoiner; ... } > > Manasi > > > > > plane_height_max = 4320; > > > } else { > > > plane_width_max = 5120; > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > index d6295eb20b63..fbfea99fd804 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > > > - struct intel_encoder *encoder = &dig_port->base; > > > + 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; > > > > > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > > > + return 2 * dev_priv->max_dotclk_freq; > > > + > > > + return dev_priv->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; > > > } > > > @@ -527,7 +539,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; > > > @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > > > max_bpp_small_joiner_ram); > > > > > > @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > > > @@ -576,7 +602,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) > > > { > > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > u8 min_slice_count, i; > > > @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > > > @@ -648,11 +683,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) > > > @@ -664,6 +703,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); > > > > > > @@ -691,23 +745,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); > > > } > > > @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > > > "Compressed BPP/Slice Count not supported\n"); > > > -- > > > 2.19.1 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel
On Mon, Sep 14, 2020 at 10:17:57PM +0300, Ville Syrjälä wrote: > On Mon, Sep 14, 2020 at 12:00:33PM -0700, Navare, Manasi wrote: > > On Mon, Sep 07, 2020 at 02:20:56PM +0300, Ville Syrjälä wrote: > > > On Wed, Jul 15, 2020 at 03:42:15PM -0700, Manasi Navare wrote: > > > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > > 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. > > > > > > > > v5: > > > > * Increase max plane width to support 8K with bigjoiner (Maarten) > > > > v4: > > > > * Rebase (Manasi) > > > > > > > > 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) > > > > v3: > > > > * Rebase (Manasi) > > > > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > > > > 2 files changed, 91 insertions(+), 30 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > > index 78cbfefbfa62..3ecb642805a6 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > > > > * too big for that. > > > > */ > > > > if (INTEL_GEN(dev_priv) >= 11) { > > > > - plane_width_max = 5120; > > > > + plane_width_max = 7680; > > > > > > This looks misplaced. Planes do no know whether bigjoiner can be used or > > > not. They should not care in fact. The caller should have that knowledge > > > and can deal with it properly. > > > > Hmm, so the caller of intel_mode_valid_max_plane_size() should check on the bigjoiner > > flag and perhaps if bigjoiner is true then increase the plane_width_max to 7680? > > > > Am still not sure where this should happen? We need to have the plane max width to be 7680 > > before we prune the 8K mode in intel_mode_valid > > > > Where should this be added according to you? > > Hmm. I guess we do need to put it into this function given the way this > is structured. However we still can't assume bigjoiner can be used since > it can't be used on DDI A on icl. So we should probably just pass in a > bool here to indicate whether bigjoiner can be used or not. > So in intel_dp_mode_valid() we set bigjoiner = true if not edp and higher clock. I think here we need to do the platform check also, 1. because now we are enabling this for TGL+ where big joiner on all pipes. But we should still I think add GEN >=12 check before setting bigjoiner to true in intel_dp_mode_valid() and then pass that to intel_mode_valid_max_plane_size(..., book bigjoiner) Sounds good? > Personally I'd just write the thing as something like: > intel_mode_valid_max_plane_size(..., bool bigjoiner) > { > ... > plane_width_max = 5120 << bigjoiner; > ... > } > > > > > Manasi > > > > > > > plane_height_max = 4320; > > > > } else { > > > > plane_width_max = 5120; > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > > index d6295eb20b63..fbfea99fd804 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > > > > - struct intel_encoder *encoder = &dig_port->base; > > > > + 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; > > > > > > > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > > > > + return 2 * dev_priv->max_dotclk_freq; > > > > + > > > > + return dev_priv->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; > > > > } > > > > @@ -527,7 +539,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; > > > > @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > > > > max_bpp_small_joiner_ram); > > > > > > > > @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > > > > @@ -576,7 +602,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) > > > > { > > > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > u8 min_slice_count, i; > > > > @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > > > > @@ -648,11 +683,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) > > > > @@ -664,6 +703,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); > > > > > > > > @@ -691,23 +745,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); > > > > } > > > > @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > > > > "Compressed BPP/Slice Count not supported\n"); > > > > -- > > > > 2.19.1 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > -- > > > Ville Syrjälä > > > Intel > > -- > Ville Syrjälä > Intel
On Mon, Sep 14, 2020 at 12:38:57PM -0700, Navare, Manasi wrote: > On Mon, Sep 14, 2020 at 10:17:57PM +0300, Ville Syrjälä wrote: > > On Mon, Sep 14, 2020 at 12:00:33PM -0700, Navare, Manasi wrote: > > > On Mon, Sep 07, 2020 at 02:20:56PM +0300, Ville Syrjälä wrote: > > > > On Wed, Jul 15, 2020 at 03:42:15PM -0700, Manasi Navare wrote: > > > > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > > > > 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. > > > > > > > > > > v5: > > > > > * Increase max plane width to support 8K with bigjoiner (Maarten) > > > > > v4: > > > > > * Rebase (Manasi) > > > > > > > > > > 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) > > > > > v3: > > > > > * Rebase (Manasi) > > > > > > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > > > > > 2 files changed, 91 insertions(+), 30 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > > > index 78cbfefbfa62..3ecb642805a6 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > > > > > * too big for that. > > > > > */ > > > > > if (INTEL_GEN(dev_priv) >= 11) { > > > > > - plane_width_max = 5120; > > > > > + plane_width_max = 7680; > > > > > > > > This looks misplaced. Planes do no know whether bigjoiner can be used or > > > > not. They should not care in fact. The caller should have that knowledge > > > > and can deal with it properly. > > > > > > Hmm, so the caller of intel_mode_valid_max_plane_size() should check on the bigjoiner > > > flag and perhaps if bigjoiner is true then increase the plane_width_max to 7680? > > > > > > Am still not sure where this should happen? We need to have the plane max width to be 7680 > > > before we prune the 8K mode in intel_mode_valid > > > > > > Where should this be added according to you? > > > > Hmm. I guess we do need to put it into this function given the way this > > is structured. However we still can't assume bigjoiner can be used since > > it can't be used on DDI A on icl. So we should probably just pass in a > > bool here to indicate whether bigjoiner can be used or not. > > > > So in intel_dp_mode_valid() we set bigjoiner = true if not edp and higher clock. > I think here we need to do the platform check also, 1. because now we are enabling this for TGL+ > where big joiner on all pipes. But we should still I think add GEN >=12 check before setting bigjoiner > to true in intel_dp_mode_valid() and then pass that to intel_mode_valid_max_plane_size(..., book bigjoiner) can_bigjoiner() { return gen >= 12 || (gen==11 && port!=A); } or something. > > Sounds good? > > > Personally I'd just write the thing as something like: > > intel_mode_valid_max_plane_size(..., bool bigjoiner) > > { > > ... > > plane_width_max = 5120 << bigjoiner; > > ... > > } > > > > > > > > Manasi > > > > > > > > > plane_height_max = 4320; > > > > > } else { > > > > > plane_width_max = 5120; > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > index d6295eb20b63..fbfea99fd804 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > > > > > - struct intel_encoder *encoder = &dig_port->base; > > > > > + 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; > > > > > > > > > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > > > > > + return 2 * dev_priv->max_dotclk_freq; > > > > > + > > > > > + return dev_priv->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; > > > > > } > > > > > @@ -527,7 +539,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; > > > > > @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > > > > > max_bpp_small_joiner_ram); > > > > > > > > > > @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > > > > > @@ -576,7 +602,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) > > > > > { > > > > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > > u8 min_slice_count, i; > > > > > @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > > > > > @@ -648,11 +683,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) > > > > > @@ -664,6 +703,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); > > > > > > > > > > @@ -691,23 +745,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); > > > > > } > > > > > @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > > > > > "Compressed BPP/Slice Count not supported\n"); > > > > > -- > > > > > 2.19.1 > > > > > > > > > > _______________________________________________ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > -- > > > > Ville Syrjälä > > > > Intel > > > > -- > > Ville Syrjälä > > Intel
On Mon, Sep 14, 2020 at 10:47:56PM +0300, Ville Syrjälä wrote: > On Mon, Sep 14, 2020 at 12:38:57PM -0700, Navare, Manasi wrote: > > On Mon, Sep 14, 2020 at 10:17:57PM +0300, Ville Syrjälä wrote: > > > On Mon, Sep 14, 2020 at 12:00:33PM -0700, Navare, Manasi wrote: > > > > On Mon, Sep 07, 2020 at 02:20:56PM +0300, Ville Syrjälä wrote: > > > > > On Wed, Jul 15, 2020 at 03:42:15PM -0700, Manasi Navare wrote: > > > > > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > > > > > > 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. > > > > > > > > > > > > v5: > > > > > > * Increase max plane width to support 8K with bigjoiner (Maarten) > > > > > > v4: > > > > > > * Rebase (Manasi) > > > > > > > > > > > > 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) > > > > > > v3: > > > > > > * Rebase (Manasi) > > > > > > > > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > > > > > > 2 files changed, 91 insertions(+), 30 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > index 78cbfefbfa62..3ecb642805a6 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > > > > > > * too big for that. > > > > > > */ > > > > > > if (INTEL_GEN(dev_priv) >= 11) { > > > > > > - plane_width_max = 5120; > > > > > > + plane_width_max = 7680; > > > > > > > > > > This looks misplaced. Planes do no know whether bigjoiner can be used or > > > > > not. They should not care in fact. The caller should have that knowledge > > > > > and can deal with it properly. > > > > > > > > Hmm, so the caller of intel_mode_valid_max_plane_size() should check on the bigjoiner > > > > flag and perhaps if bigjoiner is true then increase the plane_width_max to 7680? > > > > > > > > Am still not sure where this should happen? We need to have the plane max width to be 7680 > > > > before we prune the 8K mode in intel_mode_valid > > > > > > > > Where should this be added according to you? > > > > > > Hmm. I guess we do need to put it into this function given the way this > > > is structured. However we still can't assume bigjoiner can be used since > > > it can't be used on DDI A on icl. So we should probably just pass in a > > > bool here to indicate whether bigjoiner can be used or not. > > > > > > > So in intel_dp_mode_valid() we set bigjoiner = true if not edp and higher clock. > > I think here we need to do the platform check also, 1. because now we are enabling this for TGL+ > > where big joiner on all pipes. But we should still I think add GEN >=12 check before setting bigjoiner > > to true in intel_dp_mode_valid() and then pass that to intel_mode_valid_max_plane_size(..., book bigjoiner) > > can_bigjoiner() { > return gen >= 12 || (gen==11 && port!=A); > } Hmm, gen check can be done but can the port check be done in intel_dp_mode_valid() since we dont have an encoder yet? Else we set the bigjoiner to true here based on gen check but add the port check later in compute_config at what point if not supported then encoder config will fail. or can this be sufficient check: can_bigjoiner() { return gen >= 12 || (gen == 11 && !intel_dp_is_edp()) } Manasi > > or something. > > > > > Sounds good? > > > > > Personally I'd just write the thing as something like: > > > intel_mode_valid_max_plane_size(..., bool bigjoiner) > > > { > > > ... > > > plane_width_max = 5120 << bigjoiner; > > > ... > > > } > > > > > > > > > > > Manasi > > > > > > > > > > > plane_height_max = 4320; > > > > > > } else { > > > > > > plane_width_max = 5120; > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > index d6295eb20b63..fbfea99fd804 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > > > > > > - struct intel_encoder *encoder = &dig_port->base; > > > > > > + 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; > > > > > > > > > > > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > > > > > > + return 2 * dev_priv->max_dotclk_freq; > > > > > > + > > > > > > + return dev_priv->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; > > > > > > } > > > > > > @@ -527,7 +539,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; > > > > > > @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > > > > > > max_bpp_small_joiner_ram); > > > > > > > > > > > > @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > > > > > > @@ -576,7 +602,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) > > > > > > { > > > > > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > > > u8 min_slice_count, i; > > > > > > @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > > > > > > @@ -648,11 +683,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) > > > > > > @@ -664,6 +703,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); > > > > > > > > > > > > @@ -691,23 +745,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); > > > > > > } > > > > > > @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > > > > > > "Compressed BPP/Slice Count not supported\n"); > > > > > > -- > > > > > > 2.19.1 > > > > > > > > > > > > _______________________________________________ > > > > > > Intel-gfx mailing list > > > > > > Intel-gfx@lists.freedesktop.org > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > > > -- > > > > > Ville Syrjälä > > > > > Intel > > > > > > -- > > > Ville Syrjälä > > > Intel > > -- > Ville Syrjälä > Intel
On Tue, Sep 15, 2020 at 04:03:45PM -0700, Navare, Manasi wrote: > On Mon, Sep 14, 2020 at 10:47:56PM +0300, Ville Syrjälä wrote: > > On Mon, Sep 14, 2020 at 12:38:57PM -0700, Navare, Manasi wrote: > > > On Mon, Sep 14, 2020 at 10:17:57PM +0300, Ville Syrjälä wrote: > > > > On Mon, Sep 14, 2020 at 12:00:33PM -0700, Navare, Manasi wrote: > > > > > On Mon, Sep 07, 2020 at 02:20:56PM +0300, Ville Syrjälä wrote: > > > > > > On Wed, Jul 15, 2020 at 03:42:15PM -0700, Manasi Navare wrote: > > > > > > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > v5: > > > > > > > * Increase max plane width to support 8K with bigjoiner (Maarten) > > > > > > > v4: > > > > > > > * Rebase (Manasi) > > > > > > > > > > > > > > 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) > > > > > > > v3: > > > > > > > * Rebase (Manasi) > > > > > > > > > > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > --- > > > > > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > > > > > > > 2 files changed, 91 insertions(+), 30 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > index 78cbfefbfa62..3ecb642805a6 100644 > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > > > > > > > * too big for that. > > > > > > > */ > > > > > > > if (INTEL_GEN(dev_priv) >= 11) { > > > > > > > - plane_width_max = 5120; > > > > > > > + plane_width_max = 7680; > > > > > > > > > > > > This looks misplaced. Planes do no know whether bigjoiner can be used or > > > > > > not. They should not care in fact. The caller should have that knowledge > > > > > > and can deal with it properly. > > > > > > > > > > Hmm, so the caller of intel_mode_valid_max_plane_size() should check on the bigjoiner > > > > > flag and perhaps if bigjoiner is true then increase the plane_width_max to 7680? > > > > > > > > > > Am still not sure where this should happen? We need to have the plane max width to be 7680 > > > > > before we prune the 8K mode in intel_mode_valid > > > > > > > > > > Where should this be added according to you? > > > > > > > > Hmm. I guess we do need to put it into this function given the way this > > > > is structured. However we still can't assume bigjoiner can be used since > > > > it can't be used on DDI A on icl. So we should probably just pass in a > > > > bool here to indicate whether bigjoiner can be used or not. > > > > > > > > > > So in intel_dp_mode_valid() we set bigjoiner = true if not edp and higher clock. > > > I think here we need to do the platform check also, 1. because now we are enabling this for TGL+ > > > where big joiner on all pipes. But we should still I think add GEN >=12 check before setting bigjoiner > > > to true in intel_dp_mode_valid() and then pass that to intel_mode_valid_max_plane_size(..., book bigjoiner) > > > > can_bigjoiner() { > > return gen >= 12 || (gen==11 && port!=A); > > } > > Hmm, gen check can be done but can the port check be done in intel_dp_mode_valid() since we dont have > an encoder yet? The encoder is there already. > Else we set the bigjoiner to true here based on gen check but add the port check later in compute_config at > what point if not supported then encoder config will fail. > > or can this be sufficient check: > > can_bigjoiner() { > return gen >= 12 || (gen == 11 && !intel_dp_is_edp()) > } > > Manasi > > > > > or something. > > > > > > > > Sounds good? > > > > > > > Personally I'd just write the thing as something like: > > > > intel_mode_valid_max_plane_size(..., bool bigjoiner) > > > > { > > > > ... > > > > plane_width_max = 5120 << bigjoiner; > > > > ... > > > > } > > > > > > > > > > > > > > Manasi > > > > > > > > > > > > > plane_height_max = 4320; > > > > > > > } else { > > > > > > > plane_width_max = 5120; > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > > index d6295eb20b63..fbfea99fd804 100644 > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > > @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > > > > > > > - struct intel_encoder *encoder = &dig_port->base; > > > > > > > + 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; > > > > > > > > > > > > > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > > > > > > > + return 2 * dev_priv->max_dotclk_freq; > > > > > > > + > > > > > > > + return dev_priv->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; > > > > > > > } > > > > > > > @@ -527,7 +539,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; > > > > > > > @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > > > > > > > max_bpp_small_joiner_ram); > > > > > > > > > > > > > > @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > > > > > > > @@ -576,7 +602,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) > > > > > > > { > > > > > > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > > > > u8 min_slice_count, i; > > > > > > > @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > > > > > > > @@ -648,11 +683,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) > > > > > > > @@ -664,6 +703,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); > > > > > > > > > > > > > > @@ -691,23 +745,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); > > > > > > > } > > > > > > > @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > > > > > > > "Compressed BPP/Slice Count not supported\n"); > > > > > > > -- > > > > > > > 2.19.1 > > > > > > > > > > > > > > _______________________________________________ > > > > > > > Intel-gfx mailing list > > > > > > > Intel-gfx@lists.freedesktop.org > > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > > > > > -- > > > > > > Ville Syrjälä > > > > > > Intel > > > > > > > > -- > > > > Ville Syrjälä > > > > Intel > > > > -- > > Ville Syrjälä > > Intel
On Thu, Sep 17, 2020 at 03:20:46PM +0300, Ville Syrjälä wrote: > On Tue, Sep 15, 2020 at 04:03:45PM -0700, Navare, Manasi wrote: > > On Mon, Sep 14, 2020 at 10:47:56PM +0300, Ville Syrjälä wrote: > > > On Mon, Sep 14, 2020 at 12:38:57PM -0700, Navare, Manasi wrote: > > > > On Mon, Sep 14, 2020 at 10:17:57PM +0300, Ville Syrjälä wrote: > > > > > On Mon, Sep 14, 2020 at 12:00:33PM -0700, Navare, Manasi wrote: > > > > > > On Mon, Sep 07, 2020 at 02:20:56PM +0300, Ville Syrjälä wrote: > > > > > > > On Wed, Jul 15, 2020 at 03:42:15PM -0700, Manasi Navare wrote: > > > > > > > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > v5: > > > > > > > > * Increase max plane width to support 8K with bigjoiner (Maarten) > > > > > > > > v4: > > > > > > > > * Rebase (Manasi) > > > > > > > > > > > > > > > > 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) > > > > > > > > v3: > > > > > > > > * Rebase (Manasi) > > > > > > > > > > > > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > > --- > > > > > > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > > > > > > > > 2 files changed, 91 insertions(+), 30 deletions(-) > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > > index 78cbfefbfa62..3ecb642805a6 100644 > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > > > > > > > > * too big for that. > > > > > > > > */ > > > > > > > > if (INTEL_GEN(dev_priv) >= 11) { > > > > > > > > - plane_width_max = 5120; > > > > > > > > + plane_width_max = 7680; > > > > > > > > > > > > > > This looks misplaced. Planes do no know whether bigjoiner can be used or > > > > > > > not. They should not care in fact. The caller should have that knowledge > > > > > > > and can deal with it properly. > > > > > > > > > > > > Hmm, so the caller of intel_mode_valid_max_plane_size() should check on the bigjoiner > > > > > > flag and perhaps if bigjoiner is true then increase the plane_width_max to 7680? > > > > > > > > > > > > Am still not sure where this should happen? We need to have the plane max width to be 7680 > > > > > > before we prune the 8K mode in intel_mode_valid > > > > > > > > > > > > Where should this be added according to you? > > > > > > > > > > Hmm. I guess we do need to put it into this function given the way this > > > > > is structured. However we still can't assume bigjoiner can be used since > > > > > it can't be used on DDI A on icl. So we should probably just pass in a > > > > > bool here to indicate whether bigjoiner can be used or not. > > > > > > > > > > > > > So in intel_dp_mode_valid() we set bigjoiner = true if not edp and higher clock. > > > > I think here we need to do the platform check also, 1. because now we are enabling this for TGL+ > > > > where big joiner on all pipes. But we should still I think add GEN >=12 check before setting bigjoiner > > > > to true in intel_dp_mode_valid() and then pass that to intel_mode_valid_max_plane_size(..., book bigjoiner) > > > > > > can_bigjoiner() { > > > return gen >= 12 || (gen==11 && port!=A); On Gen 11, Port A is eDP or MIPI DSI so I could check: can_bigjoiner() { return gen >=12 || (gen == 11 && !intel_dp_is_edp()) } The above should be okay right? Manasi > > > } > > > > Hmm, gen check can be done but can the port check be done in intel_dp_mode_valid() since we dont have > > an encoder yet? > > The encoder is there already. > > > Else we set the bigjoiner to true here based on gen check but add the port check later in compute_config at > > what point if not supported then encoder config will fail. > > > > or can this be sufficient check: > > > > can_bigjoiner() { > > return gen >= 12 || (gen == 11 && !intel_dp_is_edp()) > > } > > > > Manasi > > > > > > > > or something. > > > > > > > > > > > Sounds good? > > > > > > > > > Personally I'd just write the thing as something like: > > > > > intel_mode_valid_max_plane_size(..., bool bigjoiner) > > > > > { > > > > > ... > > > > > plane_width_max = 5120 << bigjoiner; > > > > > ... > > > > > } > > > > > > > > > > > > > > > > > Manasi > > > > > > > > > > > > > > > plane_height_max = 4320; > > > > > > > > } else { > > > > > > > > plane_width_max = 5120; > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > > > index d6295eb20b63..fbfea99fd804 100644 > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > > > > @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); > > > > > > > > - struct intel_encoder *encoder = &dig_port->base; > > > > > > > > + 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; > > > > > > > > > > > > > > > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) > > > > > > > > + return 2 * dev_priv->max_dotclk_freq; > > > > > > > > + > > > > > > > > + return dev_priv->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; > > > > > > > > } > > > > > > > > @@ -527,7 +539,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; > > > > > > > > @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", > > > > > > > > max_bpp_small_joiner_ram); > > > > > > > > > > > > > > > > @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", > > > > > > > > @@ -576,7 +602,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) > > > > > > > > { > > > > > > > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > > > > > u8 min_slice_count, i; > > > > > > > > @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", > > > > > > > > @@ -648,11 +683,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) > > > > > > > > @@ -664,6 +703,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); > > > > > > > > > > > > > > > > @@ -691,23 +745,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); > > > > > > > > } > > > > > > > > @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, > > > > > > > > "Compressed BPP/Slice Count not supported\n"); > > > > > > > > -- > > > > > > > > 2.19.1 > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > Intel-gfx mailing list > > > > > > > > Intel-gfx@lists.freedesktop.org > > > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > > > > > > > -- > > > > > > > Ville Syrjälä > > > > > > > Intel > > > > > > > > > > -- > > > > > Ville Syrjälä > > > > > Intel > > > > > > -- > > > Ville Syrjälä > > > Intel > > -- > Ville Syrjälä > Intel
On Tue, Sep 22, 2020 at 10:46:52PM -0700, Navare, Manasi wrote: > On Thu, Sep 17, 2020 at 03:20:46PM +0300, Ville Syrjälä wrote: > > On Tue, Sep 15, 2020 at 04:03:45PM -0700, Navare, Manasi wrote: > > > On Mon, Sep 14, 2020 at 10:47:56PM +0300, Ville Syrjälä wrote: > > > > On Mon, Sep 14, 2020 at 12:38:57PM -0700, Navare, Manasi wrote: > > > > > On Mon, Sep 14, 2020 at 10:17:57PM +0300, Ville Syrjälä wrote: > > > > > > On Mon, Sep 14, 2020 at 12:00:33PM -0700, Navare, Manasi wrote: > > > > > > > On Mon, Sep 07, 2020 at 02:20:56PM +0300, Ville Syrjälä wrote: > > > > > > > > On Wed, Jul 15, 2020 at 03:42:15PM -0700, Manasi Navare wrote: > > > > > > > > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > v5: > > > > > > > > > * Increase max plane width to support 8K with bigjoiner (Maarten) > > > > > > > > > v4: > > > > > > > > > * Rebase (Manasi) > > > > > > > > > > > > > > > > > > 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) > > > > > > > > > v3: > > > > > > > > > * Rebase (Manasi) > > > > > > > > > > > > > > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > > > --- > > > > > > > > > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > > > > > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 119 ++++++++++++++----- > > > > > > > > > 2 files changed, 91 insertions(+), 30 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > > > index 78cbfefbfa62..3ecb642805a6 100644 > > > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > > > > > @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, > > > > > > > > > * too big for that. > > > > > > > > > */ > > > > > > > > > if (INTEL_GEN(dev_priv) >= 11) { > > > > > > > > > - plane_width_max = 5120; > > > > > > > > > + plane_width_max = 7680; > > > > > > > > > > > > > > > > This looks misplaced. Planes do no know whether bigjoiner can be used or > > > > > > > > not. They should not care in fact. The caller should have that knowledge > > > > > > > > and can deal with it properly. > > > > > > > > > > > > > > Hmm, so the caller of intel_mode_valid_max_plane_size() should check on the bigjoiner > > > > > > > flag and perhaps if bigjoiner is true then increase the plane_width_max to 7680? > > > > > > > > > > > > > > Am still not sure where this should happen? We need to have the plane max width to be 7680 > > > > > > > before we prune the 8K mode in intel_mode_valid > > > > > > > > > > > > > > Where should this be added according to you? > > > > > > > > > > > > Hmm. I guess we do need to put it into this function given the way this > > > > > > is structured. However we still can't assume bigjoiner can be used since > > > > > > it can't be used on DDI A on icl. So we should probably just pass in a > > > > > > bool here to indicate whether bigjoiner can be used or not. > > > > > > > > > > > > > > > > So in intel_dp_mode_valid() we set bigjoiner = true if not edp and higher clock. > > > > > I think here we need to do the platform check also, 1. because now we are enabling this for TGL+ > > > > > where big joiner on all pipes. But we should still I think add GEN >=12 check before setting bigjoiner > > > > > to true in intel_dp_mode_valid() and then pass that to intel_mode_valid_max_plane_size(..., book bigjoiner) > > > > > > > > can_bigjoiner() { > > > > return gen >= 12 || (gen==11 && port!=A); > > On Gen 11, Port A is eDP or MIPI DSI so I could check: > > can_bigjoiner() > { > return gen >=12 || (gen == 11 && !intel_dp_is_edp()) > } > > The above should be okay right? No. Check for port A.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 78cbfefbfa62..3ecb642805a6 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -17400,7 +17400,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv, * too big for that. */ if (INTEL_GEN(dev_priv) >= 11) { - plane_width_max = 5120; + plane_width_max = 7680; plane_height_max = 4320; } else { plane_width_max = 5120; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index d6295eb20b63..fbfea99fd804 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -248,25 +248,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 *dig_port = dp_to_dig_port(intel_dp); - struct intel_encoder *encoder = &dig_port->base; + 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; + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp)) + return 2 * dev_priv->max_dotclk_freq; + + return dev_priv->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; } @@ -527,7 +539,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; @@ -545,6 +558,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_dbg_kms(&i915->drm, "Max small joiner bpp: %u\n", max_bpp_small_joiner_ram); @@ -554,6 +571,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_dbg_kms(&i915->drm, "Unsupported BPP %u, min %u\n", @@ -576,7 +602,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) { struct drm_i915_private *i915 = dp_to_i915(intel_dp); u8 min_slice_count, i; @@ -603,12 +630,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_dbg_kms(&i915->drm, "Unsupported Slice Count %d\n", @@ -648,11 +683,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) @@ -664,6 +703,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); @@ -691,23 +745,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); } @@ -2204,11 +2263,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_dbg_kms(&dev_priv->drm, "Compressed BPP/Slice Count not supported\n");