On Fri, Jan 05, 2018 at 03:15:35PM +0530, Shashank Sharma wrote:
> LSPCON chips can generate YCBCR outputs, if asked nicely :).
>
> In order to generate YCBCR 4:2:0 outputs, a source must:
> - send YCBCR 4:4:4 signals to LSPCON
> - program color space as 4:2:0 in AVI infoframes
>
> Whereas for YCBCR 4:4:4 outputs, the source must:
> - send YCBCR 4:4:4 signals to LSPCON
> - program color space as 4:4:4 in AVI infoframes
>
> So for both 4:2:0 as well as 4:4:4 outputs, we are driving the
> pipe for YCBCR 4:4:4 output, but AVI infoframe's color space
> information indicates LSPCON FW to start scaling down from YCBCR
> 4:4:4 and generate YCBCR 4:2:0 output. As the scaling is done by
> LSPCON device, we need not to reserve a scaler for 4:2:0 outputs.
>
> V2: rebase
> V3: Addressed review comments from Ville
> - add enum crtc_output_format instead of bool ycbcr420
> - use crtc_output_format=4:4:4 for modeset of LSPCON 4:2:0 output
> cases in this way we will have YCBCR 4:4:4 framework ready (except
> the ABI part)
>
> Cc: Ville Syrjala <[email protected]>
> Cc: Maarten Lankhorst <[email protected]>
> Signed-off-by: Shashank Sharma <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++
> drivers/gpu/drm/i915/intel_dp.c | 10 ++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_lspcon.c | 28 ++++++++++++++++++++++++++++
> 5 files changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 966e4df..45ee264 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8547,6 +8547,8 @@ enum skl_power_gate {
> #define TRANS_MSA_MISC(tran) _MMIO_TRANS2(tran, _TRANSA_MSA_MISC)
>
> #define TRANS_MSA_SYNC_CLK (1<<0)
> +#define TRANS_MSA_SAMPLING_444 (2<<1)
> +#define TRANS_MSA_CLRSP_YCBCR (2<<3)
> #define TRANS_MSA_6_BPC (0<<5)
> #define TRANS_MSA_8_BPC (1<<5)
> #define TRANS_MSA_10_BPC (2<<5)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 7b89f2a..7616f6f 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1499,6 +1499,13 @@ void intel_ddi_set_pipe_settings(const struct
> intel_crtc_state *crtc_state)
> break;
> }
>
> + /*
> + * As per DP 1.2 spec section 2.3.4.3 while sending
> + * YCBCR 444 signals we should program MSA MISC1/0 fields with
> + * colorspace information.
> + */
> + if (crtc_state->output_format == CRTC_OUTPUT_YCBCR444)
> + temp |= TRANS_MSA_SAMPLING_444 | TRANS_MSA_CLRSP_YCBCR;
This fails to state that we're indicating BT.601 encoding. I think we
should spell that out explicitly.
> I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 35c5299..3bf82ea 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1613,6 +1613,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct drm_display_mode *adjusted_mode =
> &pipe_config->base.adjusted_mode;
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> + struct intel_lspcon *lspcon = enc_to_intel_lspcon(&encoder->base);
> enum port port = encoder->port;
> struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
> struct intel_connector *intel_connector = intel_dp->attached_connector;
> @@ -1642,6 +1643,15 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
> pipe_config->has_pch_encoder = true;
>
> + if (lspcon->active) {
> + struct drm_connector *connector = &intel_connector->base;
> +
> + if (lspcon_ycbcr420_config(connector, pipe_config)) {
> + pipe_config->output_format = CRTC_OUTPUT_YCBCR444;
I think I'd like to see all compute_config hooks explicitly set the
outout_format (to RGB usually obviously).
> + lspcon->output_format = CRTC_OUTPUT_YCBCR420;
You should not modify any non-atomic state like that in compute_config.
> + }
> + }
> +
> pipe_config->has_drrs = false;
> if (IS_G4X(dev_priv) || port == PORT_A)
> pipe_config->has_audio = false;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 2de6b41..5edba06 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2047,6 +2047,8 @@ void lspcon_set_infoframes(struct drm_encoder *encoder,
> const struct drm_connector_state *conn_state);
> bool lspcon_infoframe_enabled(struct drm_encoder *encoder,
> const struct intel_crtc_state *pipe_config);
> +bool lspcon_ycbcr420_config(struct drm_connector *connector,
> + struct intel_crtc_state *config);
>
> /* intel_pipe_crc.c */
> int intel_pipe_crc_create(struct drm_minor *minor);
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c
> b/drivers/gpu/drm/i915/intel_lspcon.c
> index 066ea91..cb88138 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -180,6 +180,25 @@ static bool lspcon_wake_native_aux_ch(struct
> intel_lspcon *lspcon)
> return true;
> }
>
> +bool lspcon_ycbcr420_config(struct drm_connector *connector,
> + struct intel_crtc_state *config)
> +{
> + struct drm_display_info *info = &connector->display_info;
> + struct drm_display_mode *mode = &config->base.adjusted_mode;
> +
> + if (drm_mode_is_420_only(info, mode)) {
> + if (!connector->ycbcr_420_allowed) {
> + DRM_ERROR("Platform doesn't support YCBCR420 output\n");
> + return false;
> + }
> +
> + config->port_clock /= 2;
> + return true;
> + }
> +
> + return false;
> +}
> +
> static bool lspcon_probe(struct intel_lspcon *lspcon)
> {
> int retry;
> @@ -459,6 +478,13 @@ void lspcon_set_infoframes(struct drm_encoder *encoder,
> return;
> }
>
> + if (lspcon->output_format == CRTC_OUTPUT_YCBCR420)
> + frame.avi.colorspace = HDMI_COLORSPACE_YUV420;
> + else if (lspcon->output_format == CRTC_OUTPUT_YCBCR444)
> + frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> + else
> + frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> +
> drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
> crtc_state->limited_color_range ?
> HDMI_QUANTIZATION_RANGE_LIMITED :
> @@ -512,6 +538,7 @@ bool lspcon_init(struct intel_digital_port
> *intel_dig_port)
> struct intel_lspcon *lspcon = &intel_dig_port->lspcon;
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_connector *connector = &dp->attached_connector->base;
>
> if (!HAS_LSPCON(dev_priv)) {
> DRM_ERROR("LSPCON is not supported on this platform\n");
> @@ -536,6 +563,7 @@ bool lspcon_init(struct intel_digital_port
> *intel_dig_port)
> return false;
> }
>
> + connector->ycbcr_420_allowed = true;
> lspcon->active = true;
> DRM_DEBUG_KMS("Success: LSPCON init\n");
> return true;
> --
> 2.7.4
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
