| Message ID | 20190510015353.15119-4-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 |
Op 10-05-2019 om 03:53 schreef Gwan-gyeong Mun: > Function intel_pixel_encoding_setup_vsc handles vsc header and data block > setup for pixel encoding / colorimetry format. > > Setup VSC header and data block in function intel_pixel_encoding_setup_vsc > for pixel encoding / colorimetry format as per dp 1.4a spec, > section 2.2.5.7.1, table 2-119: VSC SDP Header Bytes, section 2.2.5.7.5, > table 2-120:VSC SDP Payload for DB16 through DB18. > > v2: > Minor style fix. [Maarten] > Refer to commit ids instead of patchwork. [Maarten] > > v6: Rebase > > v7: > Rebase and addressed review comments from Ville. > Use a structure initializer instead of memset(). > Fix non-standard comment format. > Remove a referring to specific commit. > Add a setting of dynamic range bit to vsc_sdp.DB17. > Add a setting of bpc which is based on pipe_bpp. > Remove duplicated checking of connector's ycbcr_420_allowed from > intel_pixel_encoding_setup_vsc(). It is already checked from > intel_dp_ycbcr420_config(). > Remove comments for VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED. It is > already implemented on intel_dp_get_colorimetry_status(). > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > 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 | 1 + > drivers/gpu/drm/i915/intel_dp.c | 90 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 2 + > 3 files changed, 93 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index cd5277d98b03..2f1688ea5a2c 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -3391,6 +3391,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_ycbcr_420_enable(intel_dp, crtc_state); > intel_edp_drrs_enable(intel_dp, crtc_state); > > if (crtc_state->has_audio) > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 9f219f8f0c71..a3fd279a5c52 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4407,6 +4407,96 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp, > return 0; > } > > +static void > +intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp, > + const struct intel_crtc_state *crtc_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; > + > + /* > + * YCbCr 420 = 3h DB16[7:4] ITU-R BT.601 = 0h, ITU-R BT.709 = 1h > + * DB16[3:0] DP 1.4a spec, Table 2-120 > + */ > + vsc_sdp.DB[16] = 0x3 << 4; /* 0x3 << 4 , YCbCr 420*/ > + /* RGB->YCBCR color conversion uses the BT.709 color space. */ > + vsc_sdp.DB[16] |= 0x1; /* 0x1, ITU-R BT.709 */ > + > + /* > + * For pixel encoding formats YCbCr444, YCbCr422, YCbCr420, and Y Only, > + * the following Component Bit Depth values are defined: > + * 001b = 8bpc. > + * 010b = 10bpc. > + * 011b = 12bpc. > + * 100b = 16bpc. > + */ > + switch (crtc_state->pipe_bpp) { > + case 24: /* 8bpc */ > + vsc_sdp.DB[17] = 0x1; > + break; > + case 30: /* 10bpc */ > + vsc_sdp.DB[17] = 0x2; > + break; > + case 36: /* 12bpc */ > + vsc_sdp.DB[17] = 0x3; > + break; > + case 48: /* 16bpc */ > + vsc_sdp.DB[17] = 0x4; > + break; > + default: > + DRM_DEBUG_KMS("Invalid bpp value '%d'\n", crtc_state->pipe_bpp); I would use MISSING_CASE(crtc_state->pipe_bpp); here, as it's pretty fatal to VSC setup. :) Otherwise series looks good from an atomic point of view. I would try to get an ACK from the analogix bridge maintainer for inclusion in the drm-intel-next-queued tree, since it seems harmless enough to include from there. So for whole series: Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > + break; > + } > + > + /* > + * Dynamic Range (Bit 7) > + * 0 = VESA range, 1 = CTA range. > + * all YCbCr are always limited range > + */ > + vsc_sdp.DB[17] |= 0x80; > + > + /* > + * Content Type (Bits 2:0) > + * 000b = Not defined. > + * 001b = Graphics. > + * 010b = Photo. > + * 011b = Video. > + * 100b = Game > + * All other values are RESERVED. > + * Note: See CTA-861-G for the definition and expected > + * processing by a stream sink for the above contect types. > + */ > + vsc_sdp.DB[18] = 0; > + > + intel_dig_port->write_infoframe(&intel_dig_port->base, > + crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp)); > +} > + > +void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp, > + const struct intel_crtc_state *crtc_state) > +{ > + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420) > + return; > + > + intel_pixel_encoding_setup_vsc(intel_dp, crtc_state); > +} > + > static u8 intel_dp_autotest_link_training(struct intel_dp *intel_dp) > { > int status = 0; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 247893ed1543..5d1845526cf8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1576,6 +1576,8 @@ void intel_dp_get_m_n(struct intel_crtc *crtc, > struct intel_crtc_state *pipe_config); > void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state, > enum link_m_n_set m_n); > +void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp, > + const struct intel_crtc_state *crtc_state); > int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n); > bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, > struct dpll *best_clock);
On Fri, 2019-05-17 at 15:36 +0200, Maarten Lankhorst wrote: > Op 10-05-2019 om 03:53 schreef Gwan-gyeong Mun: > > Function intel_pixel_encoding_setup_vsc handles vsc header and data > > block > > setup for pixel encoding / colorimetry format. > > > > Setup VSC header and data block in function > > intel_pixel_encoding_setup_vsc > > for pixel encoding / colorimetry format as per dp 1.4a spec, > > section 2.2.5.7.1, table 2-119: VSC SDP Header Bytes, section > > 2.2.5.7.5, > > table 2-120:VSC SDP Payload for DB16 through DB18. > > > > v2: > > Minor style fix. [Maarten] > > Refer to commit ids instead of patchwork. [Maarten] > > > > v6: Rebase > > > > v7: > > Rebase and addressed review comments from Ville. > > Use a structure initializer instead of memset(). > > Fix non-standard comment format. > > Remove a referring to specific commit. > > Add a setting of dynamic range bit to vsc_sdp.DB17. > > Add a setting of bpc which is based on pipe_bpp. > > Remove duplicated checking of connector's ycbcr_420_allowed from > > intel_pixel_encoding_setup_vsc(). It is already checked from > > intel_dp_ycbcr420_config(). > > Remove comments for VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED. > > It is > > already implemented on intel_dp_get_colorimetry_status(). > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > 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 | 1 + > > drivers/gpu/drm/i915/intel_dp.c | 90 > > ++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 2 + > > 3 files changed, 93 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index cd5277d98b03..2f1688ea5a2c 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -3391,6 +3391,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_ycbcr_420_enable(intel_dp, crtc_state); > > intel_edp_drrs_enable(intel_dp, crtc_state); > > > > if (crtc_state->has_audio) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index 9f219f8f0c71..a3fd279a5c52 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4407,6 +4407,96 @@ u8 intel_dp_dsc_get_slice_count(struct > > intel_dp *intel_dp, > > return 0; > > } > > > > +static void > > +intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp, > > + const struct intel_crtc_state > > *crtc_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; > > + > > + /* > > + * YCbCr 420 = 3h DB16[7:4] ITU-R BT.601 = 0h, ITU-R BT.709 = > > 1h > > + * DB16[3:0] DP 1.4a spec, Table 2-120 > > + */ > > + vsc_sdp.DB[16] = 0x3 << 4; /* 0x3 << 4 , YCbCr 420*/ > > + /* RGB->YCBCR color conversion uses the BT.709 color space. */ > > + vsc_sdp.DB[16] |= 0x1; /* 0x1, ITU-R BT.709 */ > > + > > + /* > > + * For pixel encoding formats YCbCr444, YCbCr422, YCbCr420, and > > Y Only, > > + * the following Component Bit Depth values are defined: > > + * 001b = 8bpc. > > + * 010b = 10bpc. > > + * 011b = 12bpc. > > + * 100b = 16bpc. > > + */ > > + switch (crtc_state->pipe_bpp) { > > + case 24: /* 8bpc */ > > + vsc_sdp.DB[17] = 0x1; > > + break; > > + case 30: /* 10bpc */ > > + vsc_sdp.DB[17] = 0x2; > > + break; > > + case 36: /* 12bpc */ > > + vsc_sdp.DB[17] = 0x3; > > + break; > > + case 48: /* 16bpc */ > > + vsc_sdp.DB[17] = 0x4; > > + break; > > + default: > > + DRM_DEBUG_KMS("Invalid bpp value '%d'\n", crtc_state- > > >pipe_bpp); > > I would use MISSING_CASE(crtc_state->pipe_bpp); here, as it's pretty > fatal to VSC setup. :) > > Otherwise series looks good from an atomic point of view. I would try > to get an ACK from the analogix bridge maintainer for > inclusion in the drm-intel-next-queued tree, since it seems harmless > enough to include from there. > Hi, Thank you for reviewing the series. And I'll replace DRM_DEBUG_KMS() to MISSING_CASE() on default switch- case. > So for whole series: > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > + break; > > + } > > + > > + /* > > + * Dynamic Range (Bit 7) > > + * 0 = VESA range, 1 = CTA range. > > + * all YCbCr are always limited range > > + */ > > + vsc_sdp.DB[17] |= 0x80; > > + > > + /* > > + * Content Type (Bits 2:0) > > + * 000b = Not defined. > > + * 001b = Graphics. > > + * 010b = Photo. > > + * 011b = Video. > > + * 100b = Game > > + * All other values are RESERVED. > > + * Note: See CTA-861-G for the definition and expected > > + * processing by a stream sink for the above contect types. > > + */ > > + vsc_sdp.DB[18] = 0; > > + > > + intel_dig_port->write_infoframe(&intel_dig_port->base, > > + crtc_state, DP_SDP_VSC, &vsc_sdp, > > sizeof(vsc_sdp)); > > +} > > + > > +void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp, > > + const struct intel_crtc_state > > *crtc_state) > > +{ > > + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420) > > + return; > > + > > + intel_pixel_encoding_setup_vsc(intel_dp, crtc_state); > > +} > > + > > static u8 intel_dp_autotest_link_training(struct intel_dp > > *intel_dp) > > { > > int status = 0; > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index 247893ed1543..5d1845526cf8 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1576,6 +1576,8 @@ void intel_dp_get_m_n(struct intel_crtc > > *crtc, > > struct intel_crtc_state *pipe_config); > > void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state, > > enum link_m_n_set m_n); > > +void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp, > > + const struct intel_crtc_state > > *crtc_state); > > int intel_dotclock_calculate(int link_freq, const struct > > intel_link_m_n *m_n); > > bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, > > struct dpll *best_clock); > >
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index cd5277d98b03..2f1688ea5a2c 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -3391,6 +3391,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_ycbcr_420_enable(intel_dp, crtc_state); intel_edp_drrs_enable(intel_dp, crtc_state); if (crtc_state->has_audio) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 9f219f8f0c71..a3fd279a5c52 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4407,6 +4407,96 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp, return 0; } +static void +intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_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; + + /* + * YCbCr 420 = 3h DB16[7:4] ITU-R BT.601 = 0h, ITU-R BT.709 = 1h + * DB16[3:0] DP 1.4a spec, Table 2-120 + */ + vsc_sdp.DB[16] = 0x3 << 4; /* 0x3 << 4 , YCbCr 420*/ + /* RGB->YCBCR color conversion uses the BT.709 color space. */ + vsc_sdp.DB[16] |= 0x1; /* 0x1, ITU-R BT.709 */ + + /* + * For pixel encoding formats YCbCr444, YCbCr422, YCbCr420, and Y Only, + * the following Component Bit Depth values are defined: + * 001b = 8bpc. + * 010b = 10bpc. + * 011b = 12bpc. + * 100b = 16bpc. + */ + switch (crtc_state->pipe_bpp) { + case 24: /* 8bpc */ + vsc_sdp.DB[17] = 0x1; + break; + case 30: /* 10bpc */ + vsc_sdp.DB[17] = 0x2; + break; + case 36: /* 12bpc */ + vsc_sdp.DB[17] = 0x3; + break; + case 48: /* 16bpc */ + vsc_sdp.DB[17] = 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.DB[17] |= 0x80; + + /* + * Content Type (Bits 2:0) + * 000b = Not defined. + * 001b = Graphics. + * 010b = Photo. + * 011b = Video. + * 100b = Game + * All other values are RESERVED. + * Note: See CTA-861-G for the definition and expected + * processing by a stream sink for the above contect types. + */ + vsc_sdp.DB[18] = 0; + + intel_dig_port->write_infoframe(&intel_dig_port->base, + crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp)); +} + +void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state) +{ + if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420) + return; + + intel_pixel_encoding_setup_vsc(intel_dp, crtc_state); +} + static u8 intel_dp_autotest_link_training(struct intel_dp *intel_dp) { int status = 0; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 247893ed1543..5d1845526cf8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1576,6 +1576,8 @@ void intel_dp_get_m_n(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config); void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state, enum link_m_n_set m_n); +void intel_dp_ycbcr_420_enable(struct intel_dp *intel_dp, + const struct intel_crtc_state *crtc_state); int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n); bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, struct dpll *best_clock);
Function intel_pixel_encoding_setup_vsc handles vsc header and data block setup for pixel encoding / colorimetry format. Setup VSC header and data block in function intel_pixel_encoding_setup_vsc for pixel encoding / colorimetry format as per dp 1.4a spec, section 2.2.5.7.1, table 2-119: VSC SDP Header Bytes, section 2.2.5.7.5, table 2-120:VSC SDP Payload for DB16 through DB18. v2: Minor style fix. [Maarten] Refer to commit ids instead of patchwork. [Maarten] v6: Rebase v7: Rebase and addressed review comments from Ville. Use a structure initializer instead of memset(). Fix non-standard comment format. Remove a referring to specific commit. Add a setting of dynamic range bit to vsc_sdp.DB17. Add a setting of bpc which is based on pipe_bpp. Remove duplicated checking of connector's ycbcr_420_allowed from intel_pixel_encoding_setup_vsc(). It is already checked from intel_dp_ycbcr420_config(). Remove comments for VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED. It is already implemented on intel_dp_get_colorimetry_status(). Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> 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 | 1 + drivers/gpu/drm/i915/intel_dp.c | 90 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 2 + 3 files changed, 93 insertions(+)