diff mbox series

[v6,5/6] drm/i915/dp: Change a link bandwidth computation for DP

Message ID 20190508081757.28042-6-gwan-gyeong.mun@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dp: Support for DP YCbCr4:2:0 outputs | expand

Commit Message

Gwan-gyeong Mun May 8, 2019, 8:17 a.m. UTC 
Data M/N calculations were assumed a bpp as RGB format. But when we are
using YCbCr 4:2:0 output format on DP, we should change bpp calculations
as YCbCr 4:2:0 format. The pipe_bpp value was assumed RGB format,
therefore, it was multiplied with 3. But YCbCr 4:2:0 requires a multiplier
value to 1.5.
Therefore we need to divide pipe_bpp to 2 while DP output uses YCbCr4:2:0
format.
 - RGB format bpp = bpc x 3
 - YCbCr 4:2:0 format bpp = bpc x 1.5

But Link M/N values are calculated and applied based on the Full Clock for
YCbCr 4:2:0. And DP YCbCr 4:2:0 does not need to pixel clock double for
a dotclock caluation. Only for HDMI YCbCr 4:2:0 needs to pixel clock double
for a dot clock calculation.

And it adds missed bpc values for a programming of VSC Header.
It only affects dp and edp port which use YCbCr 4:2:0 output format.
And for now, it does not consider a use case of DSC + YCbCr 4:2:0.

v2:
  Addressed review comments from Ville.
  Remove a changing of pipe_bpp on intel_ddi_set_pipe_settings().
  Because the pipe is running at the full bpp, keep pipe_bpp as RGB
  even though YCbCr 4:2:0 output format is used.
  Add a link bandwidth computation for YCbCr4:2:0 output format.

v3:
  Addressed reivew comments from Ville.
  In order to make codes simple, it adds and uses intel_dp_output_bpp()
  function.

v6:
  Link M/N values are calculated and applied based on the Full Clock for
  YCbCr420. The Bit per Pixel needs to be adjusted for YUV420 mode as it
  requires only half of the RGB case.
    - Link M/N values are calculated and applied based on the Full Clock
    - Data M/N values needs to be calculated considering the data is half
      due to subsampling
  Remove a doubling of pixel clock on a dot clock calculator for
  DP YCbCr 4:2:0.
  Rebase and remove a duplicate setting of vsc_sdp.DB17.
  Add a setting of dynamic range bit to  vsc_sdp.DB17.
  Change Content Type bit to "Graphics" from "Not defined".
  Change a dividing of pipe_bpp to muliplying to constant values on a
  switch-case statement.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  3 ++-
 drivers/gpu/drm/i915/intel_dp.c  | 42 +++++++++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 4 deletions(-)

Comments

Ville Syrjala May 8, 2019, 5:58 p.m. UTC | #1 
On Wed, May 08, 2019 at 11:17:56AM +0300, Gwan-gyeong Mun wrote:
> Data M/N calculations were assumed a bpp as RGB format. But when we are
> using YCbCr 4:2:0 output format on DP, we should change bpp calculations
> as YCbCr 4:2:0 format. The pipe_bpp value was assumed RGB format,
> therefore, it was multiplied with 3. But YCbCr 4:2:0 requires a multiplier
> value to 1.5.
> Therefore we need to divide pipe_bpp to 2 while DP output uses YCbCr4:2:0
> format.
>  - RGB format bpp = bpc x 3
>  - YCbCr 4:2:0 format bpp = bpc x 1.5
> 
> But Link M/N values are calculated and applied based on the Full Clock for
> YCbCr 4:2:0. And DP YCbCr 4:2:0 does not need to pixel clock double for
> a dotclock caluation. Only for HDMI YCbCr 4:2:0 needs to pixel clock double
> for a dot clock calculation.
> 
> And it adds missed bpc values for a programming of VSC Header.
> It only affects dp and edp port which use YCbCr 4:2:0 output format.
> And for now, it does not consider a use case of DSC + YCbCr 4:2:0.
> 
> v2:
>   Addressed review comments from Ville.
>   Remove a changing of pipe_bpp on intel_ddi_set_pipe_settings().
>   Because the pipe is running at the full bpp, keep pipe_bpp as RGB
>   even though YCbCr 4:2:0 output format is used.
>   Add a link bandwidth computation for YCbCr4:2:0 output format.
> 
> v3:
>   Addressed reivew comments from Ville.
>   In order to make codes simple, it adds and uses intel_dp_output_bpp()
>   function.
> 
> v6:
>   Link M/N values are calculated and applied based on the Full Clock for
>   YCbCr420. The Bit per Pixel needs to be adjusted for YUV420 mode as it
>   requires only half of the RGB case.
>     - Link M/N values are calculated and applied based on the Full Clock
>     - Data M/N values needs to be calculated considering the data is half
>       due to subsampling
>   Remove a doubling of pixel clock on a dot clock calculator for
>   DP YCbCr 4:2:0.
>   Rebase and remove a duplicate setting of vsc_sdp.DB17.
>   Add a setting of dynamic range bit to  vsc_sdp.DB17.
>   Change Content Type bit to "Graphics" from "Not defined".
>   Change a dividing of pipe_bpp to muliplying to constant values on a
>   switch-case statement.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  3 ++-
>  drivers/gpu/drm/i915/intel_dp.c  | 42 +++++++++++++++++++++++++++++---
>  2 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 4441c5ba71fb..e22a0898b957 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1457,7 +1457,8 @@ static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
>  	else
>  		dotclock = pipe_config->port_clock;
>  
> -	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> +	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
> +	    !intel_crtc_has_dp_encoder(pipe_config))
>  		dotclock *= 2;
>  
>  	if (pipe_config->pixel_multiplier)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 74aad8830a80..c75e2bbe612a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1842,6 +1842,19 @@ intel_dp_adjust_compliance_config(struct intel_dp *intel_dp,
>  	}
>  }
>  
> +static int intel_dp_output_bpp(const struct intel_crtc_state *crtc_state, int bpp)
> +{
> +	/*
> +	 * bpp value was assumed to RGB format. And YCbCr 4:2:0 output
> +	 * format of the number of bytes per pixel will be half the number
> +	 * of bytes of RGB pixel.
> +	 */
> +	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> +		bpp /= 2;
> +
> +	return bpp;
> +}
> +
>  /* Optimize link config in order: max bpp, min clock, min lanes */
>  static int
>  intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
> @@ -2212,7 +2225,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	if (pipe_config->dsc_params.compression_enable)
>  		output_bpp = pipe_config->dsc_params.compressed_bpp;
>  	else
> -		output_bpp = pipe_config->pipe_bpp;
> +		output_bpp = intel_dp_output_bpp(pipe_config, pipe_config->pipe_bpp);
>  
>  	intel_link_compute_m_n(output_bpp,
>  			       pipe_config->lane_count,
> @@ -4439,7 +4452,30 @@ intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
>  	 * 011b = 12bpc.
>  	 * 100b = 16bpc.
>  	 */
> -	vsc_sdp.DB17 = 0x1;
> +	switch (crtc_state->pipe_bpp) {
> +	case 24: /* 8bpc */
> +		vsc_sdp.DB17 = 0x1;
> +		break;
> +	case 30: /* 10bpc */
> +		vsc_sdp.DB17 = 0x2;
> +		break;
> +	case 36: /* 12bpc */
> +		vsc_sdp.DB17 = 0x3;
> +		break;
> +	case 48: /* 16bpc */
> +		vsc_sdp.DB17 = 0x4;
> +		break;
> +	default:
> +		DRM_DEBUG_KMS("Invalid bpp value '%d'\n", crtc_state->pipe_bpp);
> +		break;
> +	}
> +
> +	/*
> +	 * Dynamic Range (Bit 7)
> +	 * 0 = VESA range, 1 = CTA range.
> +	 * all YCbCr are always limited range
> +	 */
> +	vsc_sdp.DB17 |= 0x80;

Ah, it was all here already. Never mind then.

>  
>  	/*
>  	 * Content Type (Bits 2:0)
> @@ -4452,7 +4488,7 @@ intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
>  	 * Note: See CTA-861-G for the definition and expected
>  	 * processing by a stream sink for the above contect types.
>  	 */
> -	vsc_sdp.DB18 = 0;
> +	vsc_sdp.DB18 = 0x1;

Why is this here?

>  
>  	intel_dig_port->write_infoframe(&intel_dig_port->base,
>  			crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp));
> -- 
> 2.21.0
Gwan-gyeong Mun May 10, 2019, 1:43 a.m. UTC | #2 
On Wed, 2019-05-08 at 20:58 +0300, Ville Syrjälä wrote:
> On Wed, May 08, 2019 at 11:17:56AM +0300, Gwan-gyeong Mun wrote:
> > Data M/N calculations were assumed a bpp as RGB format. But when we
> > are
> > using YCbCr 4:2:0 output format on DP, we should change bpp
> > calculations
> > as YCbCr 4:2:0 format. The pipe_bpp value was assumed RGB format,
> > therefore, it was multiplied with 3. But YCbCr 4:2:0 requires a
> > multiplier
> > value to 1.5.
> > Therefore we need to divide pipe_bpp to 2 while DP output uses
> > YCbCr4:2:0
> > format.
> >  - RGB format bpp = bpc x 3
> >  - YCbCr 4:2:0 format bpp = bpc x 1.5
> > 
> > But Link M/N values are calculated and applied based on the Full
> > Clock for
> > YCbCr 4:2:0. And DP YCbCr 4:2:0 does not need to pixel clock double
> > for
> > a dotclock caluation. Only for HDMI YCbCr 4:2:0 needs to pixel
> > clock double
> > for a dot clock calculation.
> > 
> > And it adds missed bpc values for a programming of VSC Header.
> > It only affects dp and edp port which use YCbCr 4:2:0 output
> > format.
> > And for now, it does not consider a use case of DSC + YCbCr 4:2:0.
> > 
> > v2:
> >   Addressed review comments from Ville.
> >   Remove a changing of pipe_bpp on intel_ddi_set_pipe_settings().
> >   Because the pipe is running at the full bpp, keep pipe_bpp as RGB
> >   even though YCbCr 4:2:0 output format is used.
> >   Add a link bandwidth computation for YCbCr4:2:0 output format.
> > 
> > v3:
> >   Addressed reivew comments from Ville.
> >   In order to make codes simple, it adds and uses
> > intel_dp_output_bpp()
> >   function.
> > 
> > v6:
> >   Link M/N values are calculated and applied based on the Full
> > Clock for
> >   YCbCr420. The Bit per Pixel needs to be adjusted for YUV420 mode
> > as it
> >   requires only half of the RGB case.
> >     - Link M/N values are calculated and applied based on the Full
> > Clock
> >     - Data M/N values needs to be calculated considering the data
> > is half
> >       due to subsampling
> >   Remove a doubling of pixel clock on a dot clock calculator for
> >   DP YCbCr 4:2:0.
> >   Rebase and remove a duplicate setting of vsc_sdp.DB17.
> >   Add a setting of dynamic range bit to  vsc_sdp.DB17.
> >   Change Content Type bit to "Graphics" from "Not defined".
> >   Change a dividing of pipe_bpp to muliplying to constant values on
> > a
> >   switch-case statement.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c |  3 ++-
> >  drivers/gpu/drm/i915/intel_dp.c  | 42
> > +++++++++++++++++++++++++++++---
> >  2 files changed, 41 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 4441c5ba71fb..e22a0898b957 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1457,7 +1457,8 @@ static void ddi_dotclock_get(struct
> > intel_crtc_state *pipe_config)
> >  	else
> >  		dotclock = pipe_config->port_clock;
> >  
> > -	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> > +	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420
> > &&
> > +	    !intel_crtc_has_dp_encoder(pipe_config))
> >  		dotclock *= 2;
> >  
> >  	if (pipe_config->pixel_multiplier)
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 74aad8830a80..c75e2bbe612a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1842,6 +1842,19 @@ intel_dp_adjust_compliance_config(struct
> > intel_dp *intel_dp,
> >  	}
> >  }
> >  
> > +static int intel_dp_output_bpp(const struct intel_crtc_state
> > *crtc_state, int bpp)
> > +{
> > +	/*
> > +	 * bpp value was assumed to RGB format. And YCbCr 4:2:0 output
> > +	 * format of the number of bytes per pixel will be half the
> > number
> > +	 * of bytes of RGB pixel.
> > +	 */
> > +	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> > +		bpp /= 2;
> > +
> > +	return bpp;
> > +}
> > +
> >  /* Optimize link config in order: max bpp, min clock, min lanes */
> >  static int
> >  intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
> > @@ -2212,7 +2225,7 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> >  	if (pipe_config->dsc_params.compression_enable)
> >  		output_bpp = pipe_config->dsc_params.compressed_bpp;
> >  	else
> > -		output_bpp = pipe_config->pipe_bpp;
> > +		output_bpp = intel_dp_output_bpp(pipe_config,
> > pipe_config->pipe_bpp);
> >  
> >  	intel_link_compute_m_n(output_bpp,
> >  			       pipe_config->lane_count,
> > @@ -4439,7 +4452,30 @@ intel_pixel_encoding_setup_vsc(struct
> > intel_dp *intel_dp,
> >  	 * 011b = 12bpc.
> >  	 * 100b = 16bpc.
> >  	 */
> > -	vsc_sdp.DB17 = 0x1;
> > +	switch (crtc_state->pipe_bpp) {
> > +	case 24: /* 8bpc */
> > +		vsc_sdp.DB17 = 0x1;
> > +		break;
> > +	case 30: /* 10bpc */
> > +		vsc_sdp.DB17 = 0x2;
> > +		break;
> > +	case 36: /* 12bpc */
> > +		vsc_sdp.DB17 = 0x3;
> > +		break;
> > +	case 48: /* 16bpc */
> > +		vsc_sdp.DB17 = 0x4;
> > +		break;
> > +	default:
> > +		DRM_DEBUG_KMS("Invalid bpp value '%d'\n", crtc_state-
> > >pipe_bpp);
> > +		break;
> > +	}
> > +
> > +	/*
> > +	 * Dynamic Range (Bit 7)
> > +	 * 0 = VESA range, 1 = CTA range.
> > +	 * all YCbCr are always limited range
> > +	 */
> > +	vsc_sdp.DB17 |= 0x80;
> 
> Ah, it was all here already. Never mind then.
> 
In order to program VSC Header and DB for Pixel Encoding/Colorimetry
Format in one commit, I'll move them to a "drm/i915/dp: Program VSC
Header and DB for Pixel Encoding/Colorimetry Format" commit.
> >  
> >  	/*
> >  	 * Content Type (Bits 2:0)
> > @@ -4452,7 +4488,7 @@ intel_pixel_encoding_setup_vsc(struct
> > intel_dp *intel_dp,
> >  	 * Note: See CTA-861-G for the definition and expected
> >  	 * processing by a stream sink for the above contect types.
> >  	 */
> > -	vsc_sdp.DB18 = 0;
> > +	vsc_sdp.DB18 = 0x1;
> 
> Why is this here?
> 
When I tested HDMI YCbCr4:2:0, logs from i915 showed "Content types" to
"Graphics". We don't have a way to set content type by property on
DP.  For now I'll remove it. And after having properties for DP output,
I'll improve it

> >  
> >  	intel_dig_port->write_infoframe(&intel_dig_port->base,
> >  			crtc_state, DP_SDP_VSC, &vsc_sdp,
> > sizeof(vsc_sdp));
> > -- 
> > 2.21.0
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 4441c5ba71fb..e22a0898b957 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1457,7 +1457,8 @@  static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
 	else
 		dotclock = pipe_config->port_clock;
 
-	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
+	    !intel_crtc_has_dp_encoder(pipe_config))
 		dotclock *= 2;
 
 	if (pipe_config->pixel_multiplier)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 74aad8830a80..c75e2bbe612a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1842,6 +1842,19 @@  intel_dp_adjust_compliance_config(struct intel_dp *intel_dp,
 	}
 }
 
+static int intel_dp_output_bpp(const struct intel_crtc_state *crtc_state, int bpp)
+{
+	/*
+	 * bpp value was assumed to RGB format. And YCbCr 4:2:0 output
+	 * format of the number of bytes per pixel will be half the number
+	 * of bytes of RGB pixel.
+	 */
+	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+		bpp /= 2;
+
+	return bpp;
+}
+
 /* Optimize link config in order: max bpp, min clock, min lanes */
 static int
 intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
@@ -2212,7 +2225,7 @@  intel_dp_compute_config(struct intel_encoder *encoder,
 	if (pipe_config->dsc_params.compression_enable)
 		output_bpp = pipe_config->dsc_params.compressed_bpp;
 	else
-		output_bpp = pipe_config->pipe_bpp;
+		output_bpp = intel_dp_output_bpp(pipe_config, pipe_config->pipe_bpp);
 
 	intel_link_compute_m_n(output_bpp,
 			       pipe_config->lane_count,
@@ -4439,7 +4452,30 @@  intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
 	 * 011b = 12bpc.
 	 * 100b = 16bpc.
 	 */
-	vsc_sdp.DB17 = 0x1;
+	switch (crtc_state->pipe_bpp) {
+	case 24: /* 8bpc */
+		vsc_sdp.DB17 = 0x1;
+		break;
+	case 30: /* 10bpc */
+		vsc_sdp.DB17 = 0x2;
+		break;
+	case 36: /* 12bpc */
+		vsc_sdp.DB17 = 0x3;
+		break;
+	case 48: /* 16bpc */
+		vsc_sdp.DB17 = 0x4;
+		break;
+	default:
+		DRM_DEBUG_KMS("Invalid bpp value '%d'\n", crtc_state->pipe_bpp);
+		break;
+	}
+
+	/*
+	 * Dynamic Range (Bit 7)
+	 * 0 = VESA range, 1 = CTA range.
+	 * all YCbCr are always limited range
+	 */
+	vsc_sdp.DB17 |= 0x80;
 
 	/*
 	 * Content Type (Bits 2:0)
@@ -4452,7 +4488,7 @@  intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
 	 * Note: See CTA-861-G for the definition and expected
 	 * processing by a stream sink for the above contect types.
 	 */
-	vsc_sdp.DB18 = 0;
+	vsc_sdp.DB18 = 0x1;
 
 	intel_dig_port->write_infoframe(&intel_dig_port->base,
 			crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp));