> -----Original Message-----
> From: Nautiyal, Ankit K <ankit.k.nautiyal@xxxxxxxxx>
> Sent: Wednesday, December 16, 2020 5:01 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; Shankar, Uma <uma.shankar@xxxxxxxxx>;
> airlied@xxxxxxxx; jani.nikula@xxxxxxxxxxxxxxx; ville.syrjala@xxxxxxxxxxxxxxx;
> Kulkarni, Vandita <vandita.kulkarni@xxxxxxxxx>; Sharma, Swati2
> <swati2.sharma@xxxxxxxxx>
> Subject: [PATCH v6 15/15] drm/i915/display: Let PCON convert from RGB to YUV
> if it can
>
> If PCON has capability to convert RGB->YUV colorspace and also to 444->420
> downsampling then for any YUV420 only mode, we can let the PCON do all the
> conversion.
>
> v2: As suggested by Uma Shankar, considered case for colorspace
> BT709 and BT2020, and default to BT609. Also appended dir 'display' in commit
> message.
>
> v3: Fixed typo in condition for printing one of the error msg.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 3 +-
> .../drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 68 +++++++++++++++----
> drivers/gpu/drm/i915/display/intel_dp.h | 3 +-
> 4 files changed, 58 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index fbc07a93504b..17eaa56c5a99 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3644,6 +3644,7 @@ static void tgl_ddi_pre_enable_dp(struct
> intel_atomic_state *state,
> if (!is_mst)
> intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
>
> + intel_dp_configure_protocol_converter(intel_dp, crtc_state);
> intel_dp_sink_set_decompression_state(intel_dp, crtc_state, true);
> /*
> * DDI FEC: "anticipates enabling FEC encoding sets the FEC_READY bit
> @@ -3731,7 +3732,7 @@ static void hsw_ddi_pre_enable_dp(struct
> intel_atomic_state *state,
> intel_ddi_init_dp_buf_reg(encoder, crtc_state);
> if (!is_mst)
> intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
> - intel_dp_configure_protocol_converter(intel_dp);
> + intel_dp_configure_protocol_converter(intel_dp, crtc_state);
> intel_dp_sink_set_decompression_state(intel_dp, crtc_state,
> true);
> intel_dp_sink_set_fec_ready(intel_dp, crtc_state); diff --git
> a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 4c01c7c23dfd..2009ae9e9678 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1460,6 +1460,7 @@ struct intel_dp {
> int pcon_max_frl_bw;
> u8 max_bpc;
> bool ycbcr_444_to_420;
> + bool rgb_to_ycbcr;
> } dfp;
>
> /* Display stream compression testing */ diff --git
> a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index abc9b772d1c8..366b2e4e7f4a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -651,6 +651,10 @@ intel_dp_output_format(struct drm_connector
> *connector,
> !drm_mode_is_420_only(info, mode))
> return INTEL_OUTPUT_FORMAT_RGB;
>
> + if (intel_dp->dfp.rgb_to_ycbcr &&
> + intel_dp->dfp.ycbcr_444_to_420)
> + return INTEL_OUTPUT_FORMAT_RGB;
> +
> if (intel_dp->dfp.ycbcr_444_to_420)
> return INTEL_OUTPUT_FORMAT_YCBCR444;
> else
> @@ -4311,7 +4315,8 @@ static void intel_dp_enable_port(struct intel_dp
> *intel_dp,
> intel_de_posting_read(dev_priv, intel_dp->output_reg); }
>
> -void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
> +void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
> + const struct intel_crtc_state *crtc_state)
> {
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> u8 tmp;
> @@ -4338,14 +4343,34 @@ void intel_dp_configure_protocol_converter(struct
> intel_dp *intel_dp)
> drm_dbg_kms(&i915->drm,
> "Failed to set protocol converter YCbCr 4:2:0
> conversion mode to %s\n",
> enableddisabled(intel_dp->dfp.ycbcr_444_to_420));
> -
> - tmp = 0;
> -
> - if (drm_dp_dpcd_writeb(&intel_dp->aux,
> - DP_PROTOCOL_CONVERTER_CONTROL_2, tmp) <= 0)
> + if (intel_dp->dfp.rgb_to_ycbcr) {
> + bool bt2020, bt709;
> +
> + bt2020 =
> drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp->dpcd,
> + intel_dp-
> >downstream_ports,
> +
> DP_DS_HDMI_BT2020_RGB_YCBCR_CONV);
Next line should match the parenthesis, seems off.
> + bt709 =
> drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp->dpcd,
> + intel_dp-
> >downstream_ports,
> +
> DP_DS_HDMI_BT709_RGB_YCBCR_CONV);
> + switch (crtc_state->infoframes.vsc.colorimetry) {
> + case DP_COLORIMETRY_BT2020_RGB:
> + case DP_COLORIMETRY_BT2020_YCC:
> + tmp = bt2020 ?
> DP_CONVERSION_BT2020_RGB_YCBCR_ENABLE : 0;
One concern I have here is what will happen if pcon returns 0 here. From SOC we will be sending
BT2020 RGB, so we expect it to be able to support this conversion. So ideally this should not be optional
but kind of mandatory for pcon to support.
> + break;
> + case DP_COLORIMETRY_BT709_YCC:
> + case DP_COLORIMETRY_XVYCC_709:
> + tmp = bt709 ?
> DP_CONVERSION_BT709_RGB_YCBCR_ENABLE : 0;
> + break;
> + default:
> + tmp = DP_CONVERSION_BT601_RGB_YCBCR_ENABLE;
> + }
> + } else {
> + tmp = 0;
> + }
Just initialize tmp to 0 and drop this else.
> + if (drm_dp_pcon_convert_rgb_to_ycbcr(&intel_dp->aux, tmp) < 0)
> drm_dbg_kms(&i915->drm,
> - "Failed to set protocol converter YCbCr 4:2:2
> conversion mode to %s\n",
> - enableddisabled(false));
> + "Failed to set protocol converter RGB->YCbCr
> conversion mode to %s\n",
> + enableddisabled(tmp ? true : false));
> }
>
> static void intel_enable_dp(struct intel_atomic_state *state, @@ -4385,7
> +4410,7 @@ static void intel_enable_dp(struct intel_atomic_state *state,
> }
>
> intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
> - intel_dp_configure_protocol_converter(intel_dp);
> + intel_dp_configure_protocol_converter(intel_dp, pipe_config);
> intel_dp_check_frl_training(intel_dp);
> intel_dp_pcon_dsc_configure(intel_dp, pipe_config);
> intel_dp_start_link_train(intel_dp, pipe_config); @@ -6853,7 +6878,7
> @@ intel_dp_update_420(struct intel_dp *intel_dp) {
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> struct intel_connector *connector = intel_dp->attached_connector;
> - bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420;
> + bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420, rgb_to_ycbcr;
>
> /* No YCbCr output support on gmch platforms */
> if (HAS_GMCH(i915))
> @@ -6875,14 +6900,26 @@ intel_dp_update_420(struct intel_dp *intel_dp)
> dp_to_dig_port(intel_dp)->lspcon.active ||
> drm_dp_downstream_444_to_420_conversion(intel_dp->dpcd,
> intel_dp-
> >downstream_ports);
> + rgb_to_ycbcr = drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp-
> >dpcd,
> + intel_dp-
> >downstream_ports,
> +
> DP_DS_HDMI_BT601_RGB_YCBCR_CONV ||
> +
> DP_DS_HDMI_BT709_RGB_YCBCR_CONV ||
> +
> DP_DS_HDMI_BT2020_RGB_YCBCR_CONV);
Here also, match the parenthesis.
>
> if (INTEL_GEN(i915) >= 11) {
> + /* Let PCON convert from RGB->YCbCr if possible */
> + if (is_branch && rgb_to_ycbcr && ycbcr_444_to_420) {
> + intel_dp->dfp.rgb_to_ycbcr = true;
> + intel_dp->dfp.ycbcr_444_to_420 = true;
> + connector->base.ycbcr_420_allowed = true;
> + } else {
> /* Prefer 4:2:0 passthrough over 4:4:4->4:2:0 conversion */
> - intel_dp->dfp.ycbcr_444_to_420 =
> - ycbcr_444_to_420 && !ycbcr_420_passthrough;
> + intel_dp->dfp.ycbcr_444_to_420 =
> + ycbcr_444_to_420 && !ycbcr_420_passthrough;
>
> - connector->base.ycbcr_420_allowed =
> - !is_branch || ycbcr_444_to_420 ||
> ycbcr_420_passthrough;
> + connector->base.ycbcr_420_allowed =
> + !is_branch || ycbcr_444_to_420 ||
> ycbcr_420_passthrough;
> + }
> } else {
> /* 4:4:4->4:2:0 conversion is the only way */
> intel_dp->dfp.ycbcr_444_to_420 = ycbcr_444_to_420; @@ -
> 6891,8 +6928,9 @@ intel_dp_update_420(struct intel_dp *intel_dp)
> }
>
> drm_dbg_kms(&i915->drm,
> - "[CONNECTOR:%d:%s] YCbCr 4:2:0 allowed? %s, YCbCr 4:4:4-
> >4:2:0 conversion? %s\n",
> + "[CONNECTOR:%d:%s] RGB->YcbCr conversion? %s, YCbCr 4:2:0
> +allowed? %s, YCbCr 4:4:4->4:2:0 conversion? %s\n",
> connector->base.base.id, connector->base.name,
> + yesno(intel_dp->dfp.rgb_to_ycbcr),
> yesno(connector->base.ycbcr_420_allowed),
> yesno(intel_dp->dfp.ycbcr_444_to_420));
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index 1bfde4f89019..4280a09fd8fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -51,7 +51,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp
> *intel_dp, int intel_dp_retrain_link(struct intel_encoder *encoder,
> struct drm_modeset_acquire_ctx *ctx); void
> intel_dp_set_power(struct intel_dp *intel_dp, u8 mode); -void
> intel_dp_configure_protocol_converter(struct intel_dp *intel_dp);
> +void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp,
> + const struct intel_crtc_state
> *crtc_state);
> void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state,
> bool enable);
> --
> 2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
