Re: [Intel-gfx] [PATCH v3 11/17] drm/i915: Program DP SDPs with computed configs

2020-02-08 Thread Mun, Gwan-gyeong
On Wed, 2020-02-05 at 22:21 +0530, Shankar, Uma wrote:
> > -Original Message-
> > From: Intel-gfx  On Behalf
> > Of Gwan-
> > gyeong Mun
> > Sent: Tuesday, February 4, 2020 4:50 AM
> > To: intel-...@lists.freedesktop.org
> > Cc: linux-fb...@vger.kernel.org; dri-devel@lists.freedesktop.org
> > Subject: [Intel-gfx] [PATCH v3 11/17] drm/i915: Program DP SDPs
> > with computed
> > configs
> > 
> > In order to use computed config for DP SDPs (DP VSC SDP and DP HDR
> > Metadata
> > Infoframe SDP), it replaces intel_dp_vsc_enable() function and
> > intel_dp_hdr_metadata_enable() function to
> > intel_dp_set_infoframes() function.
> > 
> > Before applying it, routines of program SDP always calculated
> > configs when they
> > called. And it removes unused functions.
> 
> Fix the sentence, seems unclear.
> With that fixed,
Okay, I'll update with the condition of before / after.

> Reviewed-by: Uma Shankar 
> 
> > v3: Rebased
> > 
> > Signed-off-by: Gwan-gyeong Mun 
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c |   3 +-
> >  drivers/gpu/drm/i915/display/intel_dp.c  | 226 -
> > --
> >  drivers/gpu/drm/i915/display/intel_dp.h  |   6 -
> >  3 files changed, 1 insertion(+), 234 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index c96f629cddc3..374ab6a3757c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3900,8 +3900,7 @@ static void intel_enable_ddi_dp(struct
> > intel_encoder
> > *encoder,
> > 
> > intel_edp_backlight_on(crtc_state, conn_state);
> > intel_psr_enable(intel_dp, crtc_state);
> > -   intel_dp_vsc_enable(intel_dp, crtc_state, conn_state);
> > -   intel_dp_hdr_metadata_enable(intel_dp, crtc_state, conn_state);
> > +   intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
> > intel_edp_drrs_enable(intel_dp, crtc_state);
> > 
> > if (crtc_state->has_audio)
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index d4ece0a824c0..cffb77daec96 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5095,232 +5095,6 @@ void intel_read_dp_sdp(struct intel_encoder
> > *encoder,
> > }
> >  }
> > 
> > -static void
> > -intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
> > -  const struct intel_crtc_state *crtc_state,
> > -  const struct drm_connector_state *conn_state)
> > -{
> > -   struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > -   struct dp_sdp vsc_sdp = {};
> > -
> > -   /* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
> > */
> > -   vsc_sdp.sdp_header.HB0 = 0;
> > -   vsc_sdp.sdp_header.HB1 = 0x7;
> > -
> > -   /*
> > -* VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
> > -* Colorimetry Format indication.
> > -*/
> > -   vsc_sdp.sdp_header.HB2 = 0x5;
> > -
> > -   /*
> > -* VSC SDP supporting 3D stereo, + PSR2, + Pixel Encoding/
> > -* Colorimetry Format indication (HB2 = 05h).
> > -*/
> > -   vsc_sdp.sdp_header.HB3 = 0x13;
> > -
> > -   /* DP 1.4a spec, Table 2-120 */
> > -   switch (crtc_state->output_format) {
> > -   case INTEL_OUTPUT_FORMAT_YCBCR444:
> > -   vsc_sdp.db[16] = 0x1 << 4; /* YCbCr 444 : DB16[7:4] =
> > 1h */
> > -   break;
> > -   case INTEL_OUTPUT_FORMAT_YCBCR420:
> > -   vsc_sdp.db[16] = 0x3 << 4; /* YCbCr 420 : DB16[7:4] =
> > 3h */
> > -   break;
> > -   case INTEL_OUTPUT_FORMAT_RGB:
> > -   default:
> > -   /* RGB: DB16[7:4] = 0h */
> > -   break;
> > -   }
> > -
> > -   switch (conn_state->colorspace) {
> > -   case DRM_MODE_COLORIMETRY_BT709_YCC:
> > -   vsc_sdp.db[16] |= 0x1;
> > -   break;
> > -   case DRM_MODE_COLORIMETRY_XVYCC_601:
> > -   vsc_sdp.db[16] |= 0x2;
> > -   break;
> > -   case DRM_MODE_COLORIMETRY_XVYCC_709:
> > -   vsc_sdp.db[16] |= 0x3;
> > -   break;
> > -   case DRM_MODE_COLORIMETRY_SYCC_601:
> > -   vsc_sdp.db[16] |= 0x4;
> > -   break;
> > -   case DRM_MODE_COLORIMETRY_OPYCC_601:
> > -   vsc_sdp.d

RE: [Intel-gfx] [PATCH v3 11/17] drm/i915: Program DP SDPs with computed configs

2020-02-05 Thread Shankar, Uma


> -Original Message-
> From: Intel-gfx  On Behalf Of Gwan-
> gyeong Mun
> Sent: Tuesday, February 4, 2020 4:50 AM
> To: intel-...@lists.freedesktop.org
> Cc: linux-fb...@vger.kernel.org; dri-devel@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v3 11/17] drm/i915: Program DP SDPs with computed
> configs
> 
> In order to use computed config for DP SDPs (DP VSC SDP and DP HDR Metadata
> Infoframe SDP), it replaces intel_dp_vsc_enable() function and
> intel_dp_hdr_metadata_enable() function to intel_dp_set_infoframes() function.
> 
> Before applying it, routines of program SDP always calculated configs when 
> they
> called. And it removes unused functions.

Fix the sentence, seems unclear.
With that fixed,
Reviewed-by: Uma Shankar 

> 
> v3: Rebased
> 
> Signed-off-by: Gwan-gyeong Mun 
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c |   3 +-
>  drivers/gpu/drm/i915/display/intel_dp.c  | 226 ---
>  drivers/gpu/drm/i915/display/intel_dp.h  |   6 -
>  3 files changed, 1 insertion(+), 234 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index c96f629cddc3..374ab6a3757c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3900,8 +3900,7 @@ static void intel_enable_ddi_dp(struct intel_encoder
> *encoder,
> 
>   intel_edp_backlight_on(crtc_state, conn_state);
>   intel_psr_enable(intel_dp, crtc_state);
> - intel_dp_vsc_enable(intel_dp, crtc_state, conn_state);
> - intel_dp_hdr_metadata_enable(intel_dp, crtc_state, conn_state);
> + intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
>   intel_edp_drrs_enable(intel_dp, crtc_state);
> 
>   if (crtc_state->has_audio)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index d4ece0a824c0..cffb77daec96 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5095,232 +5095,6 @@ void intel_read_dp_sdp(struct intel_encoder *encoder,
>   }
>  }
> 
> -static void
> -intel_dp_setup_vsc_sdp(struct intel_dp *intel_dp,
> -const struct intel_crtc_state *crtc_state,
> -const struct drm_connector_state *conn_state)
> -{
> - struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - struct dp_sdp vsc_sdp = {};
> -
> - /* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119 */
> - vsc_sdp.sdp_header.HB0 = 0;
> - vsc_sdp.sdp_header.HB1 = 0x7;
> -
> - /*
> -  * VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
> -  * Colorimetry Format indication.
> -  */
> - vsc_sdp.sdp_header.HB2 = 0x5;
> -
> - /*
> -  * VSC SDP supporting 3D stereo, + PSR2, + Pixel Encoding/
> -  * Colorimetry Format indication (HB2 = 05h).
> -  */
> - vsc_sdp.sdp_header.HB3 = 0x13;
> -
> - /* DP 1.4a spec, Table 2-120 */
> - switch (crtc_state->output_format) {
> - case INTEL_OUTPUT_FORMAT_YCBCR444:
> - vsc_sdp.db[16] = 0x1 << 4; /* YCbCr 444 : DB16[7:4] = 1h */
> - break;
> - case INTEL_OUTPUT_FORMAT_YCBCR420:
> - vsc_sdp.db[16] = 0x3 << 4; /* YCbCr 420 : DB16[7:4] = 3h */
> - break;
> - case INTEL_OUTPUT_FORMAT_RGB:
> - default:
> - /* RGB: DB16[7:4] = 0h */
> - break;
> - }
> -
> - switch (conn_state->colorspace) {
> - case DRM_MODE_COLORIMETRY_BT709_YCC:
> - vsc_sdp.db[16] |= 0x1;
> - break;
> - case DRM_MODE_COLORIMETRY_XVYCC_601:
> - vsc_sdp.db[16] |= 0x2;
> - break;
> - case DRM_MODE_COLORIMETRY_XVYCC_709:
> - vsc_sdp.db[16] |= 0x3;
> - break;
> - case DRM_MODE_COLORIMETRY_SYCC_601:
> - vsc_sdp.db[16] |= 0x4;
> - break;
> - case DRM_MODE_COLORIMETRY_OPYCC_601:
> - vsc_sdp.db[16] |= 0x5;
> - break;
> - case DRM_MODE_COLORIMETRY_BT2020_CYCC:
> - case DRM_MODE_COLORIMETRY_BT2020_RGB:
> - vsc_sdp.db[16] |= 0x6;
> - break;
> - case DRM_MODE_COLORIMETRY_BT2020_YCC:
> - vsc_sdp.db[16] |= 0x7;
> - break;
> - case DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
> - case DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER:
> - vsc_sdp.db[16] |= 0x4; /* DCI-P3 (SMPTE RP 431-2) */
> - break;
> - default:
> - /* sRGB (IEC 61966-2-1) / ITU-R BT.601: DB16[0:3] = 0h */
> -
> - /* RGB