diff mbox series

[v1,6/6] drm/i915/display: Reduce blanking to support 4k60@10bpp for LSPCON

Message ID 20191016103249.32121-7-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series Enable HDR on MCA LSPCON based Gen9 devices | expand

Commit Message

Shankar, Uma Oct. 16, 2019, 10:32 a.m. UTC
Blanking needs to be reduced to incorporate DP and HDMI timing/link
bandwidth limitations for CEA modes (4k@60 at 10 bpp). DP can drive
17.28Gbs while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps.
This will cause mode to blank out. Reduced Htotal by shortening the
back porch and front porch within permissible limits.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Ville Syrjälä Oct. 16, 2019, 1:13 p.m. UTC | #1
On Wed, Oct 16, 2019 at 04:02:49PM +0530, Uma Shankar wrote:
> Blanking needs to be reduced to incorporate DP and HDMI timing/link
> bandwidth limitations for CEA modes (4k@60 at 10 bpp). DP can drive
> 17.28Gbs while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps.
> This will cause mode to blank out. Reduced Htotal by shortening the
> back porch and front porch within permissible limits.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index d92777bd3bed..a12b6916023d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -597,8 +597,10 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
>  	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> +	struct intel_lspcon *lspcon = enc_to_intel_lspcon(&intel_encoder->base);
>  	int target_clock = mode->clock;
>  	int max_rate, mode_rate, max_lanes, max_link_clock;
>  	int max_dotclk;
> @@ -620,6 +622,21 @@ intel_dp_mode_valid(struct drm_connector *connector,
>  		target_clock = fixed_mode->clock;
>  	}
>  
> +	/*
> +	 * Reducing Blanking to incorporate DP and HDMI timing/link bandwidth
> +	 * limitations for CEA modes (4k@60 at 10 bpp). DP can drive 17.28Gbs
> +	 * while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. This will
> +	 * cause mode to blank out. Reduced Htotal by shortening the back porch
> +	 * and front porch within permissible limits.
> +	 */
> +	if (lspcon->active && lspcon->hdr_supported &&
> +	    mode->clock > 570000) {
> +		mode->clock = 570000;
> +		mode->htotal -= 180;
> +		mode->hsync_start -= 72;
> +		mode->hsync_end -= 72;
> +	}

I don't think we want these kind of hacks. Either the mode works or it
doesn't.

> +
>  	max_link_clock = intel_dp_max_link_rate(intel_dp);
>  	max_lanes = intel_dp_max_lane_count(intel_dp);
>  
> -- 
> 2.22.0
Shankar, Uma Oct. 16, 2019, 1:46 p.m. UTC | #2
>-----Original Message-----
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Sent: Wednesday, October 16, 2019 6:44 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Mun, Gwan-gyeong <gwan-
>gyeong.mun@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>
>Subject: Re: [v1 6/6] drm/i915/display: Reduce blanking to support 4k60@10bpp for
>LSPCON
>
>On Wed, Oct 16, 2019 at 04:02:49PM +0530, Uma Shankar wrote:
>> Blanking needs to be reduced to incorporate DP and HDMI timing/link
>> bandwidth limitations for CEA modes (4k@60 at 10 bpp). DP can drive
>> 17.28Gbs while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps.
>> This will cause mode to blank out. Reduced Htotal by shortening the
>> back porch and front porch within permissible limits.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index d92777bd3bed..a12b6916023d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -597,8 +597,10 @@ intel_dp_mode_valid(struct drm_connector
>> *connector,  {
>>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>  	struct intel_connector *intel_connector =
>> to_intel_connector(connector);
>> +	struct intel_encoder *intel_encoder =
>> +intel_attached_encoder(connector);
>>  	struct drm_display_mode *fixed_mode = intel_connector-
>>panel.fixed_mode;
>>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>> +	struct intel_lspcon *lspcon =
>> +enc_to_intel_lspcon(&intel_encoder->base);
>>  	int target_clock = mode->clock;
>>  	int max_rate, mode_rate, max_lanes, max_link_clock;
>>  	int max_dotclk;
>> @@ -620,6 +622,21 @@ intel_dp_mode_valid(struct drm_connector *connector,
>>  		target_clock = fixed_mode->clock;
>>  	}
>>
>> +	/*
>> +	 * Reducing Blanking to incorporate DP and HDMI timing/link bandwidth
>> +	 * limitations for CEA modes (4k@60 at 10 bpp). DP can drive 17.28Gbs
>> +	 * while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. This will
>> +	 * cause mode to blank out. Reduced Htotal by shortening the back porch
>> +	 * and front porch within permissible limits.
>> +	 */
>> +	if (lspcon->active && lspcon->hdr_supported &&
>> +	    mode->clock > 570000) {
>> +		mode->clock = 570000;
>> +		mode->htotal -= 180;
>> +		mode->hsync_start -= 72;
>> +		mode->hsync_end -= 72;
>> +	}
>
>I don't think we want these kind of hacks. Either the mode works or it doesn't.

Hi Ville,
Yeah this is not ideal. But in order to enable HDR which is mostly 10bit content on Lspcon based
Gen9 devices there are limitations on bandwidth side on DP. So with that limit, we cannot drive
10bit content at 4k@60. But practically we can get this working and able to drive the sink without
any real issues with above timing adjustments. This gets enabled if firmware advertise HDR capabilities,
 so in case a vendor doesn't want this, it can be disabled in the LSPCON firmware.

I tested on HDMI analyzer and multiple sinks and also data from other OS teams suggest that this
configuration works and is enabled in some of the products as well.

Definitely not ideal, but at least we get HDR working on Gen9 devices with this, with an option
of disabling if not required. This can be more of quirk kind of thing.

What do you suggest.

Regards,
Uma Shankar

>> +
>>  	max_link_clock = intel_dp_max_link_rate(intel_dp);
>>  	max_lanes = intel_dp_max_lane_count(intel_dp);
>>
>> --
>> 2.22.0
>
>--
>Ville Syrjälä
>Intel
Ville Syrjälä Oct. 16, 2019, 1:56 p.m. UTC | #3
On Wed, Oct 16, 2019 at 01:46:24PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >Sent: Wednesday, October 16, 2019 6:44 PM
> >To: Shankar, Uma <uma.shankar@intel.com>
> >Cc: intel-gfx@lists.freedesktop.org; Mun, Gwan-gyeong <gwan-
> >gyeong.mun@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>
> >Subject: Re: [v1 6/6] drm/i915/display: Reduce blanking to support 4k60@10bpp for
> >LSPCON
> >
> >On Wed, Oct 16, 2019 at 04:02:49PM +0530, Uma Shankar wrote:
> >> Blanking needs to be reduced to incorporate DP and HDMI timing/link
> >> bandwidth limitations for CEA modes (4k@60 at 10 bpp). DP can drive
> >> 17.28Gbs while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps.
> >> This will cause mode to blank out. Reduced Htotal by shortening the
> >> back porch and front porch within permissible limits.
> >>
> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++
> >>  1 file changed, 17 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >> b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index d92777bd3bed..a12b6916023d 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -597,8 +597,10 @@ intel_dp_mode_valid(struct drm_connector
> >> *connector,  {
> >>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> >>  	struct intel_connector *intel_connector =
> >> to_intel_connector(connector);
> >> +	struct intel_encoder *intel_encoder =
> >> +intel_attached_encoder(connector);
> >>  	struct drm_display_mode *fixed_mode = intel_connector-
> >>panel.fixed_mode;
> >>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> >> +	struct intel_lspcon *lspcon =
> >> +enc_to_intel_lspcon(&intel_encoder->base);
> >>  	int target_clock = mode->clock;
> >>  	int max_rate, mode_rate, max_lanes, max_link_clock;
> >>  	int max_dotclk;
> >> @@ -620,6 +622,21 @@ intel_dp_mode_valid(struct drm_connector *connector,
> >>  		target_clock = fixed_mode->clock;
> >>  	}
> >>
> >> +	/*
> >> +	 * Reducing Blanking to incorporate DP and HDMI timing/link bandwidth
> >> +	 * limitations for CEA modes (4k@60 at 10 bpp). DP can drive 17.28Gbs
> >> +	 * while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. This will
> >> +	 * cause mode to blank out. Reduced Htotal by shortening the back porch
> >> +	 * and front porch within permissible limits.
> >> +	 */
> >> +	if (lspcon->active && lspcon->hdr_supported &&
> >> +	    mode->clock > 570000) {
> >> +		mode->clock = 570000;
> >> +		mode->htotal -= 180;
> >> +		mode->hsync_start -= 72;
> >> +		mode->hsync_end -= 72;
> >> +	}
> >
> >I don't think we want these kind of hacks. Either the mode works or it doesn't.
> 
> Hi Ville,
> Yeah this is not ideal. But in order to enable HDR which is mostly 10bit content on Lspcon based
> Gen9 devices there are limitations on bandwidth side on DP. So with that limit, we cannot drive
> 10bit content at 4k@60. But practically we can get this working and able to drive the sink without
> any real issues with above timing adjustments. This gets enabled if firmware advertise HDR capabilities,
>  so in case a vendor doesn't want this, it can be disabled in the LSPCON firmware.
> 
> I tested on HDMI analyzer and multiple sinks and also data from other OS teams suggest that this
> configuration works and is enabled in some of the products as well.
> 
> Definitely not ideal, but at least we get HDR working on Gen9 devices with this, with an option
> of disabling if not required. This can be more of quirk kind of thing.
> 
> What do you suggest.

If user wants HDR user overrides the mode manually.
Shankar, Uma Oct. 16, 2019, 2:13 p.m. UTC | #4
>> >-----Original Message-----
>> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >Sent: Wednesday, October 16, 2019 6:44 PM
>> >To: Shankar, Uma <uma.shankar@intel.com>
>> >Cc: intel-gfx@lists.freedesktop.org; Mun, Gwan-gyeong <gwan-
>> >gyeong.mun@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>
>> >Subject: Re: [v1 6/6] drm/i915/display: Reduce blanking to support
>> >4k60@10bpp for LSPCON
>> >
>> >On Wed, Oct 16, 2019 at 04:02:49PM +0530, Uma Shankar wrote:
>> >> Blanking needs to be reduced to incorporate DP and HDMI timing/link
>> >> bandwidth limitations for CEA modes (4k@60 at 10 bpp). DP can drive
>> >> 17.28Gbs while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps.
>> >> This will cause mode to blank out. Reduced Htotal by shortening the
>> >> back porch and front porch within permissible limits.
>> >>
>> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++
>> >>  1 file changed, 17 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> >> b/drivers/gpu/drm/i915/display/intel_dp.c
>> >> index d92777bd3bed..a12b6916023d 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> >> @@ -597,8 +597,10 @@ intel_dp_mode_valid(struct drm_connector
>> >> *connector,  {
>> >>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> >>  	struct intel_connector *intel_connector =
>> >> to_intel_connector(connector);
>> >> +	struct intel_encoder *intel_encoder =
>> >> +intel_attached_encoder(connector);
>> >>  	struct drm_display_mode *fixed_mode = intel_connector-
>> >>panel.fixed_mode;
>> >>  	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>> >> +	struct intel_lspcon *lspcon =
>> >> +enc_to_intel_lspcon(&intel_encoder->base);
>> >>  	int target_clock = mode->clock;
>> >>  	int max_rate, mode_rate, max_lanes, max_link_clock;
>> >>  	int max_dotclk;
>> >> @@ -620,6 +622,21 @@ intel_dp_mode_valid(struct drm_connector
>*connector,
>> >>  		target_clock = fixed_mode->clock;
>> >>  	}
>> >>
>> >> +	/*
>> >> +	 * Reducing Blanking to incorporate DP and HDMI timing/link bandwidth
>> >> +	 * limitations for CEA modes (4k@60 at 10 bpp). DP can drive 17.28Gbs
>> >> +	 * while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. This will
>> >> +	 * cause mode to blank out. Reduced Htotal by shortening the back porch
>> >> +	 * and front porch within permissible limits.
>> >> +	 */
>> >> +	if (lspcon->active && lspcon->hdr_supported &&
>> >> +	    mode->clock > 570000) {
>> >> +		mode->clock = 570000;
>> >> +		mode->htotal -= 180;
>> >> +		mode->hsync_start -= 72;
>> >> +		mode->hsync_end -= 72;
>> >> +	}
>> >
>> >I don't think we want these kind of hacks. Either the mode works or it doesn't.
>>
>> Hi Ville,
>> Yeah this is not ideal. But in order to enable HDR which is mostly
>> 10bit content on Lspcon based
>> Gen9 devices there are limitations on bandwidth side on DP. So with
>> that limit, we cannot drive 10bit content at 4k@60. But practically we
>> can get this working and able to drive the sink without any real
>> issues with above timing adjustments. This gets enabled if firmware advertise HDR
>capabilities,  so in case a vendor doesn't want this, it can be disabled in the LSPCON
>firmware.
>>
>> I tested on HDMI analyzer and multiple sinks and also data from other
>> OS teams suggest that this configuration works and is enabled in some of the
>products as well.
>>
>> Definitely not ideal, but at least we get HDR working on Gen9 devices
>> with this, with an option of disabling if not required. This can be more of quirk kind
>of thing.
>>
>> What do you suggest.
>
>If user wants HDR user overrides the mode manually.

Yeah that can also be an option. We can tell product teams to have these hacks on the
userspace side. We just need to educate them of these.

Thanks Ville for your inputs. Will drop this from the series.

Regards,
Uma Shankar

>--
>Ville Syrjälä
>Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index d92777bd3bed..a12b6916023d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -597,8 +597,10 @@  intel_dp_mode_valid(struct drm_connector *connector,
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
 	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+	struct intel_lspcon *lspcon = enc_to_intel_lspcon(&intel_encoder->base);
 	int target_clock = mode->clock;
 	int max_rate, mode_rate, max_lanes, max_link_clock;
 	int max_dotclk;
@@ -620,6 +622,21 @@  intel_dp_mode_valid(struct drm_connector *connector,
 		target_clock = fixed_mode->clock;
 	}
 
+	/*
+	 * Reducing Blanking to incorporate DP and HDMI timing/link bandwidth
+	 * limitations for CEA modes (4k@60 at 10 bpp). DP can drive 17.28Gbs
+	 * while 4k modes (VIC97 etc) at 10 bpp required 17.8 Gbps. This will
+	 * cause mode to blank out. Reduced Htotal by shortening the back porch
+	 * and front porch within permissible limits.
+	 */
+	if (lspcon->active && lspcon->hdr_supported &&
+	    mode->clock > 570000) {
+		mode->clock = 570000;
+		mode->htotal -= 180;
+		mode->hsync_start -= 72;
+		mode->hsync_end -= 72;
+	}
+
 	max_link_clock = intel_dp_max_link_rate(intel_dp);
 	max_lanes = intel_dp_max_lane_count(intel_dp);