diff mbox series

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

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

Commit Message

Navare, Manasi Oct. 8, 2020, 9:45 p.m. UTC
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.

v7:
* Add can_bigjoiner() helper (Ville)
* Pass bigjoiner to plane_size validation (Ville)
v6:
* Rebase after dp_downstream mode valid changes (Manasi)
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 |   5 +-
 drivers/gpu/drm/i915/display/intel_display.h |   3 +-
 drivers/gpu/drm/i915/display/intel_dp.c      | 126 +++++++++++++++----
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |   2 +-
 drivers/gpu/drm/i915/display/intel_dsi.c     |   2 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c    |   2 +-
 6 files changed, 111 insertions(+), 29 deletions(-)

Comments

Ville Syrjala Oct. 14, 2020, 11:26 a.m. UTC | #1
On Thu, Oct 08, 2020 at 02:45:28PM -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.
> 
> v7:
> * Add can_bigjoiner() helper (Ville)
> * Pass bigjoiner to plane_size validation (Ville)
> v6:
> * Rebase after dp_downstream mode valid changes (Manasi)
> 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 |   5 +-
>  drivers/gpu/drm/i915/display/intel_display.h |   3 +-
>  drivers/gpu/drm/i915/display/intel_dp.c      | 126 +++++++++++++++----
>  drivers/gpu/drm/i915/display/intel_dp_mst.c  |   2 +-
>  drivers/gpu/drm/i915/display/intel_dsi.c     |   2 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c    |   2 +-
>  6 files changed, 111 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 723766b1eae3..cc540c7b7dcd 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -17642,7 +17642,8 @@ intel_mode_valid(struct drm_device *dev,
>  
>  enum drm_mode_status
>  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> -				const struct drm_display_mode *mode)
> +				const struct drm_display_mode *mode,
> +				bool bigjoiner)
>  {
>  	int plane_width_max, plane_height_max;
>  
> @@ -17659,7 +17660,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 = 5120 << bigjoiner;
>  		plane_height_max = 4320;
>  	} else {
>  		plane_width_max = 5120;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index d10b7c8cde3f..3d860a9da8fe 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -496,7 +496,8 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
>  bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
>  enum drm_mode_status
>  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> -				const struct drm_display_mode *mode);
> +				const struct drm_display_mode *mode,
> +				bool bigjoiner);
>  enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
>  bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8a522edd7386..af2ff425e5d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -247,6 +247,29 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
>  	return max_link_clock * max_lanes;
>  }
>  
> +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_encoder *encoder = &intel_dig_port->base;
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> +	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
> +intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> +{
> +	int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
> +
> +	if (intel_dp->dfp.max_dotclock)

No. dfp checks should stay where they are.

> +		return min(max_dotclk, intel_dp->dfp.max_dotclock);
> +
> +	return max_dotclk;
> +}
> +
>  static int cnl_max_source_rate(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> @@ -512,7 +535,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;
> @@ -530,6 +554,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);
>  
> @@ -539,6 +567,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",
> @@ -561,7 +598,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;
> @@ -588,12 +626,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",
> @@ -676,10 +722,6 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
>  	const struct drm_display_info *info = &connector->base.display_info;
>  	int tmds_clock;
>  
> -	if (intel_dp->dfp.max_dotclock &&
> -	    target_clock > intel_dp->dfp.max_dotclock)
> -		return MODE_CLOCK_HIGH;
> -
>  	/* Assume 8bpc for the DP++/HDMI/DVI TMDS clock check */
>  	tmds_clock = target_clock;
>  	if (drm_mode_is_420_only(info, mode))
> @@ -695,6 +737,16 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
>  	return MODE_OK;
>  }
>  
> +static bool intel_dp_can_bigjoiner(struct drm_connector *connector)
> +{
> +	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +
> +	return INTEL_GEN(dev_priv) >= 12 ||
> +		(INTEL_GEN(dev_priv) == 11 &&
> +		 encoder->port != PORT_A);
> +}
> +
>  static enum drm_mode_status
>  intel_dp_mode_valid(struct drm_connector *connector,
>  		    struct drm_display_mode *mode)
> @@ -709,10 +761,16 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  	u16 dsc_max_output_bpp = 0;
>  	u8 dsc_slice_count = 0;
>  	enum drm_mode_status status;
> +	bool dsc = false, bigjoiner = false;
>  
>  	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return MODE_NO_DBLESCAN;
>  
> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> +		return MODE_H_ILLEGAL;

This random shuffling of things makes for a messy diff.

> +
> +	max_dotclk = intel_dp_max_dotclock(intel_dp, false);
> +
>  	if (intel_dp_is_edp(intel_dp) && fixed_mode) {
>  		if (mode->hdisplay > fixed_mode->hdisplay)
>  			return MODE_PANEL;
> @@ -723,6 +781,21 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  		target_clock = fixed_mode->clock;
>  	}
>  
> +	if (mode->clock < 10000)
> +		return MODE_CLOCK_LOW;

Another random shuffling. Hmm. What on earth is this even and why is it
checking mode->clock and not target_clock?

Anywyas, plese split out the unrelated stuff so we at least have a
chance of getting a legible diff.

> +
> +	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 = intel_dp_can_bigjoiner(connector);
> +	}
> +
>  	max_link_clock = intel_dp_max_link_rate(intel_dp);
>  	max_lanes = intel_dp_max_lane_count(intel_dp);
>  
> @@ -751,30 +824,35 @@ 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;
> +	}
>  
>  	status = intel_dp_mode_valid_downstream(intel_connector,
>  						mode, target_clock);
>  	if (status != MODE_OK)
>  		return status;
>  
> -	return intel_mode_valid_max_plane_size(dev_priv, mode);
> +	return intel_mode_valid_max_plane_size(dev_priv, mode, bigjoiner);
>  }
>  
>  u32 intel_dp_pack_aux(const u8 *src, int src_bytes)
> @@ -2324,11 +2402,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");
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index e948aacbd4ab..0fe2a3929ce6 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -714,7 +714,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  		return 0;
>  	}
>  
> -	*status = intel_mode_valid_max_plane_size(dev_priv, mode);
> +	*status = intel_mode_valid_max_plane_size(dev_priv, mode, true);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi.c b/drivers/gpu/drm/i915/display/intel_dsi.c
> index afa4e6817e8c..f453ceb8d149 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi.c
> @@ -75,7 +75,7 @@ enum drm_mode_status intel_dsi_mode_valid(struct drm_connector *connector,
>  			return MODE_CLOCK_HIGH;
>  	}
>  
> -	return intel_mode_valid_max_plane_size(dev_priv, mode);
> +	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
>  }
>  
>  struct intel_dsi_host *intel_dsi_host_init(struct intel_dsi *intel_dsi,
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index f90838bc74fb..82674a8853c6 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2274,7 +2274,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>  	if (status != MODE_OK)
>  		return status;
>  
> -	return intel_mode_valid_max_plane_size(dev_priv, mode);
> +	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
>  }
>  
>  bool intel_hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
> -- 
> 2.19.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Navare, Manasi Oct. 14, 2020, 7:04 p.m. UTC | #2
On Wed, Oct 14, 2020 at 02:26:34PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 08, 2020 at 02:45:28PM -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.
> > 
> > v7:
> > * Add can_bigjoiner() helper (Ville)
> > * Pass bigjoiner to plane_size validation (Ville)
> > v6:
> > * Rebase after dp_downstream mode valid changes (Manasi)
> > 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 |   5 +-
> >  drivers/gpu/drm/i915/display/intel_display.h |   3 +-
> >  drivers/gpu/drm/i915/display/intel_dp.c      | 126 +++++++++++++++----
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c  |   2 +-
> >  drivers/gpu/drm/i915/display/intel_dsi.c     |   2 +-
> >  drivers/gpu/drm/i915/display/intel_hdmi.c    |   2 +-
> >  6 files changed, 111 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 723766b1eae3..cc540c7b7dcd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -17642,7 +17642,8 @@ intel_mode_valid(struct drm_device *dev,
> >  
> >  enum drm_mode_status
> >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > -				const struct drm_display_mode *mode)
> > +				const struct drm_display_mode *mode,
> > +				bool bigjoiner)
> >  {
> >  	int plane_width_max, plane_height_max;
> >  
> > @@ -17659,7 +17660,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 = 5120 << bigjoiner;
> >  		plane_height_max = 4320;
> >  	} else {
> >  		plane_width_max = 5120;
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index d10b7c8cde3f..3d860a9da8fe 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -496,7 +496,8 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> >  bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
> >  enum drm_mode_status
> >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > -				const struct drm_display_mode *mode);
> > +				const struct drm_display_mode *mode,
> > +				bool bigjoiner);
> >  enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
> >  bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 8a522edd7386..af2ff425e5d5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -247,6 +247,29 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> >  	return max_link_clock * max_lanes;
> >  }
> >  
> > +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > +{
> > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > +	struct intel_encoder *encoder = &intel_dig_port->base;
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +
> > +	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
> > +intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > +{
> > +	int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
> > +
> > +	if (intel_dp->dfp.max_dotclock)
> 
> No. dfp checks should stay where they are.

I am using dfp.max_dotclock because we populate that with drm_dp_downstream_max_dotclock()
should that be used here directly from drm_dp_downstream_max_dotclock instead of using dfp.maxdotclock ?


> 
> > +		return min(max_dotclk, intel_dp->dfp.max_dotclock);
> > +
> > +	return max_dotclk;
> > +}
> > +
> >  static int cnl_max_source_rate(struct intel_dp *intel_dp)
> >  {
> >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > @@ -512,7 +535,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;
> > @@ -530,6 +554,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);
> >  
> > @@ -539,6 +567,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",
> > @@ -561,7 +598,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;
> > @@ -588,12 +626,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",
> > @@ -676,10 +722,6 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
> >  	const struct drm_display_info *info = &connector->base.display_info;
> >  	int tmds_clock;
> >  
> > -	if (intel_dp->dfp.max_dotclock &&
> > -	    target_clock > intel_dp->dfp.max_dotclock)
> > -		return MODE_CLOCK_HIGH;
> > -
> >  	/* Assume 8bpc for the DP++/HDMI/DVI TMDS clock check */
> >  	tmds_clock = target_clock;
> >  	if (drm_mode_is_420_only(info, mode))
> > @@ -695,6 +737,16 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
> >  	return MODE_OK;
> >  }
> >  
> > +static bool intel_dp_can_bigjoiner(struct drm_connector *connector)
> > +{
> > +	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
> > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > +
> > +	return INTEL_GEN(dev_priv) >= 12 ||
> > +		(INTEL_GEN(dev_priv) == 11 &&
> > +		 encoder->port != PORT_A);
> > +}
> > +
> >  static enum drm_mode_status
> >  intel_dp_mode_valid(struct drm_connector *connector,
> >  		    struct drm_display_mode *mode)
> > @@ -709,10 +761,16 @@ intel_dp_mode_valid(struct drm_connector *connector,
> >  	u16 dsc_max_output_bpp = 0;
> >  	u8 dsc_slice_count = 0;
> >  	enum drm_mode_status status;
> > +	bool dsc = false, bigjoiner = false;
> >  
> >  	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> >  		return MODE_NO_DBLESCAN;
> >  
> > +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> > +		return MODE_H_ILLEGAL;
> 
> This random shuffling of things makes for a messy diff.

Yea this is just a shuffle of this condition from bottom of this function to above
but will try to add this in a separate patch.

> 
> > +
> > +	max_dotclk = intel_dp_max_dotclock(intel_dp, false);
> > +
> >  	if (intel_dp_is_edp(intel_dp) && fixed_mode) {
> >  		if (mode->hdisplay > fixed_mode->hdisplay)
> >  			return MODE_PANEL;
> > @@ -723,6 +781,21 @@ intel_dp_mode_valid(struct drm_connector *connector,
> >  		target_clock = fixed_mode->clock;
> >  	}
> >  
> > +	if (mode->clock < 10000)
> > +		return MODE_CLOCK_LOW;
> 
> Another random shuffling. Hmm. What on earth is this even and why is it
> checking mode->clock and not target_clock?

This was always there in intel_dp_mode_valid() just been moved up at the top
will try to add this shuffling in a separate patch.

everything else looks good?

Manasi


> 
> Anywyas, plese split out the unrelated stuff so we at least have a
> chance of getting a legible diff.
> 
> > +
> > +	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 = intel_dp_can_bigjoiner(connector);
> > +	}
> > +
> >  	max_link_clock = intel_dp_max_link_rate(intel_dp);
> >  	max_lanes = intel_dp_max_lane_count(intel_dp);
> >  
> > @@ -751,30 +824,35 @@ 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;
> > +	}
> >  
> >  	status = intel_dp_mode_valid_downstream(intel_connector,
> >  						mode, target_clock);
> >  	if (status != MODE_OK)
> >  		return status;
> >  
> > -	return intel_mode_valid_max_plane_size(dev_priv, mode);
> > +	return intel_mode_valid_max_plane_size(dev_priv, mode, bigjoiner);
> >  }
> >  
> >  u32 intel_dp_pack_aux(const u8 *src, int src_bytes)
> > @@ -2324,11 +2402,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");
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index e948aacbd4ab..0fe2a3929ce6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -714,7 +714,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
> >  		return 0;
> >  	}
> >  
> > -	*status = intel_mode_valid_max_plane_size(dev_priv, mode);
> > +	*status = intel_mode_valid_max_plane_size(dev_priv, mode, true);
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dsi.c b/drivers/gpu/drm/i915/display/intel_dsi.c
> > index afa4e6817e8c..f453ceb8d149 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dsi.c
> > @@ -75,7 +75,7 @@ enum drm_mode_status intel_dsi_mode_valid(struct drm_connector *connector,
> >  			return MODE_CLOCK_HIGH;
> >  	}
> >  
> > -	return intel_mode_valid_max_plane_size(dev_priv, mode);
> > +	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
> >  }
> >  
> >  struct intel_dsi_host *intel_dsi_host_init(struct intel_dsi *intel_dsi,
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index f90838bc74fb..82674a8853c6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -2274,7 +2274,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
> >  	if (status != MODE_OK)
> >  		return status;
> >  
> > -	return intel_mode_valid_max_plane_size(dev_priv, mode);
> > +	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
> >  }
> >  
> >  bool intel_hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
> > -- 
> > 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 Syrjala Oct. 15, 2020, 11:52 a.m. UTC | #3
On Wed, Oct 14, 2020 at 12:04:10PM -0700, Navare, Manasi wrote:
> On Wed, Oct 14, 2020 at 02:26:34PM +0300, Ville Syrjälä wrote:
> > On Thu, Oct 08, 2020 at 02:45:28PM -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.
> > > 
> > > v7:
> > > * Add can_bigjoiner() helper (Ville)
> > > * Pass bigjoiner to plane_size validation (Ville)
> > > v6:
> > > * Rebase after dp_downstream mode valid changes (Manasi)
> > > 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 |   5 +-
> > >  drivers/gpu/drm/i915/display/intel_display.h |   3 +-
> > >  drivers/gpu/drm/i915/display/intel_dp.c      | 126 +++++++++++++++----
> > >  drivers/gpu/drm/i915/display/intel_dp_mst.c  |   2 +-
> > >  drivers/gpu/drm/i915/display/intel_dsi.c     |   2 +-
> > >  drivers/gpu/drm/i915/display/intel_hdmi.c    |   2 +-
> > >  6 files changed, 111 insertions(+), 29 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 723766b1eae3..cc540c7b7dcd 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -17642,7 +17642,8 @@ intel_mode_valid(struct drm_device *dev,
> > >  
> > >  enum drm_mode_status
> > >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > -				const struct drm_display_mode *mode)
> > > +				const struct drm_display_mode *mode,
> > > +				bool bigjoiner)
> > >  {
> > >  	int plane_width_max, plane_height_max;
> > >  
> > > @@ -17659,7 +17660,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 = 5120 << bigjoiner;
> > >  		plane_height_max = 4320;
> > >  	} else {
> > >  		plane_width_max = 5120;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > > index d10b7c8cde3f..3d860a9da8fe 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > > @@ -496,7 +496,8 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> > >  bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
> > >  enum drm_mode_status
> > >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > -				const struct drm_display_mode *mode);
> > > +				const struct drm_display_mode *mode,
> > > +				bool bigjoiner);
> > >  enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
> > >  bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 8a522edd7386..af2ff425e5d5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -247,6 +247,29 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> > >  	return max_link_clock * max_lanes;
> > >  }
> > >  
> > > +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > > +{
> > > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > +	struct intel_encoder *encoder = &intel_dig_port->base;
> > > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > +
> > > +	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
> > > +intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > > +{
> > > +	int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
> > > +
> > > +	if (intel_dp->dfp.max_dotclock)
> > 
> > No. dfp checks should stay where they are.
> 
> I am using dfp.max_dotclock because we populate that with drm_dp_downstream_max_dotclock()
> should that be used here directly from drm_dp_downstream_max_dotclock instead of using dfp.maxdotclock ?

Can you explain how bigjoiner and DFP dotclock limits relate to each
other?
Navare, Manasi Oct. 15, 2020, 4:26 p.m. UTC | #4
On Thu, Oct 15, 2020 at 02:52:47PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 14, 2020 at 12:04:10PM -0700, Navare, Manasi wrote:
> > On Wed, Oct 14, 2020 at 02:26:34PM +0300, Ville Syrjälä wrote:
> > > On Thu, Oct 08, 2020 at 02:45:28PM -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.
> > > > 
> > > > v7:
> > > > * Add can_bigjoiner() helper (Ville)
> > > > * Pass bigjoiner to plane_size validation (Ville)
> > > > v6:
> > > > * Rebase after dp_downstream mode valid changes (Manasi)
> > > > 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 |   5 +-
> > > >  drivers/gpu/drm/i915/display/intel_display.h |   3 +-
> > > >  drivers/gpu/drm/i915/display/intel_dp.c      | 126 +++++++++++++++----
> > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c  |   2 +-
> > > >  drivers/gpu/drm/i915/display/intel_dsi.c     |   2 +-
> > > >  drivers/gpu/drm/i915/display/intel_hdmi.c    |   2 +-
> > > >  6 files changed, 111 insertions(+), 29 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 723766b1eae3..cc540c7b7dcd 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -17642,7 +17642,8 @@ intel_mode_valid(struct drm_device *dev,
> > > >  
> > > >  enum drm_mode_status
> > > >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > > -				const struct drm_display_mode *mode)
> > > > +				const struct drm_display_mode *mode,
> > > > +				bool bigjoiner)
> > > >  {
> > > >  	int plane_width_max, plane_height_max;
> > > >  
> > > > @@ -17659,7 +17660,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 = 5120 << bigjoiner;
> > > >  		plane_height_max = 4320;
> > > >  	} else {
> > > >  		plane_width_max = 5120;
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > > > index d10b7c8cde3f..3d860a9da8fe 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > > > @@ -496,7 +496,8 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> > > >  bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
> > > >  enum drm_mode_status
> > > >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > > -				const struct drm_display_mode *mode);
> > > > +				const struct drm_display_mode *mode,
> > > > +				bool bigjoiner);
> > > >  enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
> > > >  bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
> > > >  
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > index 8a522edd7386..af2ff425e5d5 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > @@ -247,6 +247,29 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> > > >  	return max_link_clock * max_lanes;
> > > >  }
> > > >  
> > > > +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > > > +{
> > > > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > +	struct intel_encoder *encoder = &intel_dig_port->base;
> > > > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > +
> > > > +	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
> > > > +intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > > > +{
> > > > +	int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
> > > > +
> > > > +	if (intel_dp->dfp.max_dotclock)
> > > 
> > > No. dfp checks should stay where they are.
> > 
> > I am using dfp.max_dotclock because we populate that with drm_dp_downstream_max_dotclock()
> > should that be used here directly from drm_dp_downstream_max_dotclock instead of using dfp.maxdotclock ?
> 
> Can you explain how bigjoiner and DFP dotclock limits relate to each
> other?

Before the dfp dotclock checks were added, we were obtaining the max dotclock as min (source_max_dotclock, downstream max dotclock from dpcd)
And thats why I was using the dfp.max_dotclock

But while addressing your feedback , I have now max_dotclock = source max dotclock
and the downstream max dotclock checks happen in intel_dp_mode_valid_downstream(), so I think we dont need to consider
this here in max dotclock computation.

Is this correct?

Manasi
 
> 
> -- 
> Ville Syrjälä
> Intel
Ville Syrjala Oct. 19, 2020, 4:30 p.m. UTC | #5
On Thu, Oct 15, 2020 at 09:26:45AM -0700, Navare, Manasi wrote:
> On Thu, Oct 15, 2020 at 02:52:47PM +0300, Ville Syrjälä wrote:
> > On Wed, Oct 14, 2020 at 12:04:10PM -0700, Navare, Manasi wrote:
> > > On Wed, Oct 14, 2020 at 02:26:34PM +0300, Ville Syrjälä wrote:
> > > > On Thu, Oct 08, 2020 at 02:45:28PM -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.
> > > > > 
> > > > > v7:
> > > > > * Add can_bigjoiner() helper (Ville)
> > > > > * Pass bigjoiner to plane_size validation (Ville)
> > > > > v6:
> > > > > * Rebase after dp_downstream mode valid changes (Manasi)
> > > > > 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 |   5 +-
> > > > >  drivers/gpu/drm/i915/display/intel_display.h |   3 +-
> > > > >  drivers/gpu/drm/i915/display/intel_dp.c      | 126 +++++++++++++++----
> > > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c  |   2 +-
> > > > >  drivers/gpu/drm/i915/display/intel_dsi.c     |   2 +-
> > > > >  drivers/gpu/drm/i915/display/intel_hdmi.c    |   2 +-
> > > > >  6 files changed, 111 insertions(+), 29 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index 723766b1eae3..cc540c7b7dcd 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -17642,7 +17642,8 @@ intel_mode_valid(struct drm_device *dev,
> > > > >  
> > > > >  enum drm_mode_status
> > > > >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > > > -				const struct drm_display_mode *mode)
> > > > > +				const struct drm_display_mode *mode,
> > > > > +				bool bigjoiner)
> > > > >  {
> > > > >  	int plane_width_max, plane_height_max;
> > > > >  
> > > > > @@ -17659,7 +17660,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 = 5120 << bigjoiner;
> > > > >  		plane_height_max = 4320;
> > > > >  	} else {
> > > > >  		plane_width_max = 5120;
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > > > > index d10b7c8cde3f..3d860a9da8fe 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > > > > @@ -496,7 +496,8 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> > > > >  bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
> > > > >  enum drm_mode_status
> > > > >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > > > -				const struct drm_display_mode *mode);
> > > > > +				const struct drm_display_mode *mode,
> > > > > +				bool bigjoiner);
> > > > >  enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
> > > > >  bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
> > > > >  
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > index 8a522edd7386..af2ff425e5d5 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > @@ -247,6 +247,29 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> > > > >  	return max_link_clock * max_lanes;
> > > > >  }
> > > > >  
> > > > > +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > > > > +{
> > > > > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > +	struct intel_encoder *encoder = &intel_dig_port->base;
> > > > > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > > +
> > > > > +	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
> > > > > +intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > > > > +{
> > > > > +	int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
> > > > > +
> > > > > +	if (intel_dp->dfp.max_dotclock)
> > > > 
> > > > No. dfp checks should stay where they are.
> > > 
> > > I am using dfp.max_dotclock because we populate that with drm_dp_downstream_max_dotclock()
> > > should that be used here directly from drm_dp_downstream_max_dotclock instead of using dfp.maxdotclock ?
> > 
> > Can you explain how bigjoiner and DFP dotclock limits relate to each
> > other?
> 
> Before the dfp dotclock checks were added, we were obtaining the max dotclock as min (source_max_dotclock, downstream max dotclock from dpcd)
> And thats why I was using the dfp.max_dotclock
> 
> But while addressing your feedback , I have now max_dotclock = source max dotclock
> and the downstream max dotclock checks happen in intel_dp_mode_valid_downstream(), so I think we dont need to consider
> this here in max dotclock computation.
> 
> Is this correct?

DFP's have nothing to do with bigjoiner. It is a purely internal
implemntation detail of our display engine. So mixing up the two
would be very wrong.
Navare, Manasi Oct. 19, 2020, 10:51 p.m. UTC | #6
On Mon, Oct 19, 2020 at 07:30:07PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 15, 2020 at 09:26:45AM -0700, Navare, Manasi wrote:
> > On Thu, Oct 15, 2020 at 02:52:47PM +0300, Ville Syrjälä wrote:
> > > On Wed, Oct 14, 2020 at 12:04:10PM -0700, Navare, Manasi wrote:
> > > > On Wed, Oct 14, 2020 at 02:26:34PM +0300, Ville Syrjälä wrote:
> > > > > On Thu, Oct 08, 2020 at 02:45:28PM -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.
> > > > > > 
> > > > > > v7:
> > > > > > * Add can_bigjoiner() helper (Ville)
> > > > > > * Pass bigjoiner to plane_size validation (Ville)
> > > > > > v6:
> > > > > > * Rebase after dp_downstream mode valid changes (Manasi)
> > > > > > 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 |   5 +-
> > > > > >  drivers/gpu/drm/i915/display/intel_display.h |   3 +-
> > > > > >  drivers/gpu/drm/i915/display/intel_dp.c      | 126 +++++++++++++++----
> > > > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c  |   2 +-
> > > > > >  drivers/gpu/drm/i915/display/intel_dsi.c     |   2 +-
> > > > > >  drivers/gpu/drm/i915/display/intel_hdmi.c    |   2 +-
> > > > > >  6 files changed, 111 insertions(+), 29 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > index 723766b1eae3..cc540c7b7dcd 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > @@ -17642,7 +17642,8 @@ intel_mode_valid(struct drm_device *dev,
> > > > > >  
> > > > > >  enum drm_mode_status
> > > > > >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > > > > -				const struct drm_display_mode *mode)
> > > > > > +				const struct drm_display_mode *mode,
> > > > > > +				bool bigjoiner)
> > > > > >  {
> > > > > >  	int plane_width_max, plane_height_max;
> > > > > >  
> > > > > > @@ -17659,7 +17660,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 = 5120 << bigjoiner;
> > > > > >  		plane_height_max = 4320;
> > > > > >  	} else {
> > > > > >  		plane_width_max = 5120;
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > > > > > index d10b7c8cde3f..3d860a9da8fe 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > > > > > @@ -496,7 +496,8 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> > > > > >  bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
> > > > > >  enum drm_mode_status
> > > > > >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > > > > -				const struct drm_display_mode *mode);
> > > > > > +				const struct drm_display_mode *mode,
> > > > > > +				bool bigjoiner);
> > > > > >  enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
> > > > > >  bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
> > > > > >  
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > index 8a522edd7386..af2ff425e5d5 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > @@ -247,6 +247,29 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> > > > > >  	return max_link_clock * max_lanes;
> > > > > >  }
> > > > > >  
> > > > > > +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > > > > > +{
> > > > > > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > > +	struct intel_encoder *encoder = &intel_dig_port->base;
> > > > > > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > > > +
> > > > > > +	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
> > > > > > +intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > > > > > +{
> > > > > > +	int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
> > > > > > +
> > > > > > +	if (intel_dp->dfp.max_dotclock)
> > > > > 
> > > > > No. dfp checks should stay where they are.
> > > > 
> > > > I am using dfp.max_dotclock because we populate that with drm_dp_downstream_max_dotclock()
> > > > should that be used here directly from drm_dp_downstream_max_dotclock instead of using dfp.maxdotclock ?
> > > 
> > > Can you explain how bigjoiner and DFP dotclock limits relate to each
> > > other?
> > 
> > Before the dfp dotclock checks were added, we were obtaining the max dotclock as min (source_max_dotclock, downstream max dotclock from dpcd)
> > And thats why I was using the dfp.max_dotclock
> > 
> > But while addressing your feedback , I have now max_dotclock = source max dotclock
> > and the downstream max dotclock checks happen in intel_dp_mode_valid_downstream(), so I think we dont need to consider
> > this here in max dotclock computation.
> > 
> > Is this correct?

> 
> DFP's have nothing to do with bigjoiner. It is a purely internal
> implemntation detail of our display engine. So mixing up the two
> would be very wrong.

I understand that bigjoiner is a purely internal implementation
detail of our display engine, however whethera  mode can be supported with bigjoiner
also depends on whether the clock of that mode is supported
by downstream port hence we were using the max downstream dotclock.
But I have removed the downstream port clock check for now and only using
the source dotclock check tod ecide on whether the mode is valid with bigjoiner or not since
if the downstream port doesnt support it, it will get pruned in the newly added
intel_dp_downstream_mode_valid() function.

Manasi


> 
> -- 
> Ville Syrjälä
> Intel
Ville Syrjala Oct. 20, 2020, 6:39 p.m. UTC | #7
On Thu, Oct 08, 2020 at 02:45:28PM -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.
> 
> v7:
> * Add can_bigjoiner() helper (Ville)
> * Pass bigjoiner to plane_size validation (Ville)
> v6:
> * Rebase after dp_downstream mode valid changes (Manasi)
> 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 |   5 +-
>  drivers/gpu/drm/i915/display/intel_display.h |   3 +-
>  drivers/gpu/drm/i915/display/intel_dp.c      | 126 +++++++++++++++----
>  drivers/gpu/drm/i915/display/intel_dp_mst.c  |   2 +-
>  drivers/gpu/drm/i915/display/intel_dsi.c     |   2 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c    |   2 +-
>  6 files changed, 111 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 723766b1eae3..cc540c7b7dcd 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -17642,7 +17642,8 @@ intel_mode_valid(struct drm_device *dev,
>  
>  enum drm_mode_status
>  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> -				const struct drm_display_mode *mode)
> +				const struct drm_display_mode *mode,
> +				bool bigjoiner)
>  {
>  	int plane_width_max, plane_height_max;
>  
> @@ -17659,7 +17660,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 = 5120 << bigjoiner;
>  		plane_height_max = 4320;
>  	} else {
>  		plane_width_max = 5120;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index d10b7c8cde3f..3d860a9da8fe 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -496,7 +496,8 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
>  bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
>  enum drm_mode_status
>  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> -				const struct drm_display_mode *mode);
> +				const struct drm_display_mode *mode,
> +				bool bigjoiner);
>  enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
>  bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8a522edd7386..af2ff425e5d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -247,6 +247,29 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
>  	return max_link_clock * max_lanes;
>  }
>  
> +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_encoder *encoder = &intel_dig_port->base;
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> +	if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp))

Wasn't this supposed to be s/edp/port==A/ ?

> +		return 2 * dev_priv->max_dotclk_freq;
> +
> +	return dev_priv->max_dotclk_freq;
> +}
Navare, Manasi Oct. 20, 2020, 6:53 p.m. UTC | #8
On Tue, Oct 20, 2020 at 09:39:53PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 08, 2020 at 02:45:28PM -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.
> > 
> > v7:
> > * Add can_bigjoiner() helper (Ville)
> > * Pass bigjoiner to plane_size validation (Ville)
> > v6:
> > * Rebase after dp_downstream mode valid changes (Manasi)
> > 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 |   5 +-
> >  drivers/gpu/drm/i915/display/intel_display.h |   3 +-
> >  drivers/gpu/drm/i915/display/intel_dp.c      | 126 +++++++++++++++----
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c  |   2 +-
> >  drivers/gpu/drm/i915/display/intel_dsi.c     |   2 +-
> >  drivers/gpu/drm/i915/display/intel_hdmi.c    |   2 +-
> >  6 files changed, 111 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 723766b1eae3..cc540c7b7dcd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -17642,7 +17642,8 @@ intel_mode_valid(struct drm_device *dev,
> >  
> >  enum drm_mode_status
> >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > -				const struct drm_display_mode *mode)
> > +				const struct drm_display_mode *mode,
> > +				bool bigjoiner)
> >  {
> >  	int plane_width_max, plane_height_max;
> >  
> > @@ -17659,7 +17660,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 = 5120 << bigjoiner;
> >  		plane_height_max = 4320;
> >  	} else {
> >  		plane_width_max = 5120;
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index d10b7c8cde3f..3d860a9da8fe 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -496,7 +496,8 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> >  bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
> >  enum drm_mode_status
> >  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > -				const struct drm_display_mode *mode);
> > +				const struct drm_display_mode *mode,
> > +				bool bigjoiner);
> >  enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
> >  bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 8a522edd7386..af2ff425e5d5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -247,6 +247,29 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> >  	return max_link_clock * max_lanes;
> >  }
> >  
> > +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > +{
> > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > +	struct intel_encoder *encoder = &intel_dig_port->base;
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +
> > +	if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp))
> 
> Wasn't this supposed to be s/edp/port==A/ ?

Yes will use the new function intel_dp_can_bigjoiner() here instead that checks for PORT A

Manasi

> 
> > +		return 2 * dev_priv->max_dotclk_freq;
> > +
> > +	return dev_priv->max_dotclk_freq;
> > +}
> -- 
> Ville Syrjälä
> Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 723766b1eae3..cc540c7b7dcd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -17642,7 +17642,8 @@  intel_mode_valid(struct drm_device *dev,
 
 enum drm_mode_status
 intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
-				const struct drm_display_mode *mode)
+				const struct drm_display_mode *mode,
+				bool bigjoiner)
 {
 	int plane_width_max, plane_height_max;
 
@@ -17659,7 +17660,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 = 5120 << bigjoiner;
 		plane_height_max = 4320;
 	} else {
 		plane_width_max = 5120;
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index d10b7c8cde3f..3d860a9da8fe 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -496,7 +496,8 @@  u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
 bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
 enum drm_mode_status
 intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
-				const struct drm_display_mode *mode);
+				const struct drm_display_mode *mode,
+				bool bigjoiner);
 enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
 bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 8a522edd7386..af2ff425e5d5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -247,6 +247,29 @@  intel_dp_max_data_rate(int max_link_clock, int max_lanes)
 	return max_link_clock * max_lanes;
 }
 
+static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *encoder = &intel_dig_port->base;
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
+	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
+intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
+{
+	int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
+
+	if (intel_dp->dfp.max_dotclock)
+		return min(max_dotclk, intel_dp->dfp.max_dotclock);
+
+	return max_dotclk;
+}
+
 static int cnl_max_source_rate(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
@@ -512,7 +535,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;
@@ -530,6 +554,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);
 
@@ -539,6 +567,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",
@@ -561,7 +598,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;
@@ -588,12 +626,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",
@@ -676,10 +722,6 @@  intel_dp_mode_valid_downstream(struct intel_connector *connector,
 	const struct drm_display_info *info = &connector->base.display_info;
 	int tmds_clock;
 
-	if (intel_dp->dfp.max_dotclock &&
-	    target_clock > intel_dp->dfp.max_dotclock)
-		return MODE_CLOCK_HIGH;
-
 	/* Assume 8bpc for the DP++/HDMI/DVI TMDS clock check */
 	tmds_clock = target_clock;
 	if (drm_mode_is_420_only(info, mode))
@@ -695,6 +737,16 @@  intel_dp_mode_valid_downstream(struct intel_connector *connector,
 	return MODE_OK;
 }
 
+static bool intel_dp_can_bigjoiner(struct drm_connector *connector)
+{
+	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+
+	return INTEL_GEN(dev_priv) >= 12 ||
+		(INTEL_GEN(dev_priv) == 11 &&
+		 encoder->port != PORT_A);
+}
+
 static enum drm_mode_status
 intel_dp_mode_valid(struct drm_connector *connector,
 		    struct drm_display_mode *mode)
@@ -709,10 +761,16 @@  intel_dp_mode_valid(struct drm_connector *connector,
 	u16 dsc_max_output_bpp = 0;
 	u8 dsc_slice_count = 0;
 	enum drm_mode_status status;
+	bool dsc = false, bigjoiner = false;
 
 	if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return MODE_NO_DBLESCAN;
 
+	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)
 			return MODE_PANEL;
@@ -723,6 +781,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 = intel_dp_can_bigjoiner(connector);
+	}
+
 	max_link_clock = intel_dp_max_link_rate(intel_dp);
 	max_lanes = intel_dp_max_lane_count(intel_dp);
 
@@ -751,30 +824,35 @@  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;
+	}
 
 	status = intel_dp_mode_valid_downstream(intel_connector,
 						mode, target_clock);
 	if (status != MODE_OK)
 		return status;
 
-	return intel_mode_valid_max_plane_size(dev_priv, mode);
+	return intel_mode_valid_max_plane_size(dev_priv, mode, bigjoiner);
 }
 
 u32 intel_dp_pack_aux(const u8 *src, int src_bytes)
@@ -2324,11 +2402,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");
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index e948aacbd4ab..0fe2a3929ce6 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -714,7 +714,7 @@  intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 		return 0;
 	}
 
-	*status = intel_mode_valid_max_plane_size(dev_priv, mode);
+	*status = intel_mode_valid_max_plane_size(dev_priv, mode, true);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.c b/drivers/gpu/drm/i915/display/intel_dsi.c
index afa4e6817e8c..f453ceb8d149 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi.c
+++ b/drivers/gpu/drm/i915/display/intel_dsi.c
@@ -75,7 +75,7 @@  enum drm_mode_status intel_dsi_mode_valid(struct drm_connector *connector,
 			return MODE_CLOCK_HIGH;
 	}
 
-	return intel_mode_valid_max_plane_size(dev_priv, mode);
+	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
 }
 
 struct intel_dsi_host *intel_dsi_host_init(struct intel_dsi *intel_dsi,
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index f90838bc74fb..82674a8853c6 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2274,7 +2274,7 @@  intel_hdmi_mode_valid(struct drm_connector *connector,
 	if (status != MODE_OK)
 		return status;
 
-	return intel_mode_valid_max_plane_size(dev_priv, mode);
+	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
 }
 
 bool intel_hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,