On Thu, Jul 13, 2017 at 10:32:00AM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 7/12/2017 10:45 PM, Ville Syrjälä wrote:
> > On Mon, Jul 10, 2017 at 04:48:46PM +0530, Shashank Sharma wrote:
> >> LSPCON chips support YCBCR420 outputs. To be able to get
> >> YCBCR420 output from LSPCON chip, the source should:
> >> - Generate YCBCR444 HDMI output
> >> - Set AVI infoframes for a YCBCR420 output.
> >>
> >> LSPCON FW gets the information from AVI infoframes, and generates
> >> YCBCR420 output from a YCBCR444 input. This patch adds the necessary
> >> changes to drive YCBCR420 output from LSPCON based HDMI output.
> >>
> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/i915/intel_display.c | 10 +++++++---
> >> drivers/gpu/drm/i915/intel_dp.c | 16 +++++++++++++++-
> >> drivers/gpu/drm/i915/intel_drv.h | 20 +++++++++++++++++---
> >> drivers/gpu/drm/i915/intel_hdmi.c | 7 +++++--
> >> drivers/gpu/drm/i915/intel_lspcon.c | 17 +++++++++++++++++
> >> 5 files changed, 61 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index c5ff568..c3c7b31 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -8125,9 +8125,11 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
> >> val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
> >>
> >> if (config->ycbcr420) {
> >> - val |= PIPEMISC_OUTPUT_YCBCR |
> >> - PIPEMISC_YCBCR420_ENABLE |
> >> - PIPEMISC_YCBCR420_MODE_BLEND;
> >> + val |= PIPEMISC_OUTPUT_YCBCR;
> >> +
> >> + if (!config->lspcon_active)
> >> + val |= PIPEMISC_YCBCR420_ENABLE |
> >> + PIPEMISC_YCBCR420_MODE_BLEND;
> >> }
> >>
> >> I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
> >> @@ -14205,11 +14207,13 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
> >> * DDI_BUF_CTL_A or SFUSE_STRAP registers, find another way to
> >> * detect the ports.
> >> */
> >> +
> >> intel_ddi_init(dev_priv, PORT_A);
> >> intel_ddi_init(dev_priv, PORT_B);
> >> intel_ddi_init(dev_priv, PORT_C);
> >>
> >> intel_dsi_init(dev_priv);
> >> +
> >> } else if (HAS_DDI(dev_priv)) {
> >> int found;
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 67bc8a7a..1690aa9 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -1614,7 +1614,9 @@ 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);
> >> - enum port port = dp_to_dig_port(intel_dp)->port;
> >> + struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >> + enum port port = dig_port->port;
> >> + struct intel_lspcon *lspcon = &dig_port->lspcon;
> >> struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
> >> struct intel_connector *intel_connector = intel_dp->attached_connector;
> >> struct intel_digital_connector_state *intel_conn_state =
> >> @@ -1635,6 +1637,18 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >> common_len = intel_dp_common_len_rate_limit(intel_dp,
> >> intel_dp->max_link_rate);
> >>
> >> + /* LSPCON needs special handling to drive YCBCR420 outputs */
> >> + if (lspcon->active) {
> >> + struct drm_connector *connector = &intel_connector->base;
> >> + int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock;
> >> + int clock_12bpc = clock_8bpc * 3 / 2;
> >> +
> >> + pipe_config->lspcon_active = true;
> >> + pipe_config->ycbcr420 = lspcon_ycbcr420_config(connector,
> >> + pipe_config, &clock_12bpc,
> >> + &clock_8bpc);
> > All this clock stuff here seems pointless. So I'd just replace this
> > stuff with the straightforward 'pipe_config->ycbcr420 = mode_needs_420';
> pipe_config->ycbcr_420 = true means we have committed into state that we
> can support this mode in YCBCR420.
> But for that, we need to check connector->ycbcr420_allowed too.
> Also, when the mode is 420, we need to half the clock_8bpc and clock_12bpc.
We don't use those clocks with DP. You've just added them here because
the function call requires them as parameters.
Also the function call is actually doing the wrong thing for DP
by halving port_clock.
>
> This whole stuff is happening into lspcon_ycbcr420_config function
> (which internally re-use hdmi_ycncr420_config from HDMI 2.0 series).
> That's why the call is important.
> >> + }
> >> +
> >> /* No common link rates between source and sink */
> >> WARN_ON(common_len <= 0);
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> index ed04de9..40d56f2 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -790,6 +790,9 @@ struct intel_crtc_state {
> >>
> >> /* HDMI output type */
> >> bool ycbcr420;
> >> +
> >> + /* LSPCON is active on port */
> >> + bool lspcon_active;
> >> };
> >>
> >> struct intel_crtc {
> >> @@ -1205,6 +1208,12 @@ static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
> >> return &enc_to_dig_port(encoder)->dp;
> >> }
> >>
> >> +static inline struct intel_lspcon *
> >> +enc_to_intel_lspcon(struct drm_encoder *encoder)
> >> +{
> >> + return &enc_to_dig_port(encoder)->lspcon;
> >> +}
> >> +
> >> static inline struct intel_digital_port *
> >> dp_to_dig_port(struct intel_dp *intel_dp)
> >> {
> >> @@ -1675,14 +1684,16 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >> struct intel_connector *intel_connector);
> >> struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
> >> bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >> - struct intel_crtc_state *pipe_config,
> >> - struct drm_connector_state *conn_state);
> >> + struct intel_crtc_state *pipe_config,
> >> + struct drm_connector_state *conn_state);
> >> void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder,
> >> struct drm_connector *connector,
> >> bool high_tmds_clock_ratio,
> >> bool scrambling);
> >> void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
> >> -
> >> +bool intel_hdmi_ycbcr420_config(struct drm_connector *connector,
> >> + struct intel_crtc_state *config,
> >> + int *clock_12bpc, int *clock_8bpc);
> >>
> >> /* intel_lvds.c */
> >> void intel_lvds_init(struct drm_i915_private *dev_priv);
> >> @@ -2003,6 +2014,9 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state);
> >> bool lspcon_init(struct intel_digital_port *intel_dig_port);
> >> void lspcon_resume(struct intel_lspcon *lspcon);
> >> void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
> >> +bool lspcon_ycbcr420_config(struct drm_connector *connector,
> >> + struct intel_crtc_state *config,
> >> + int *clock_12bpc, int *clock_8bpc);
> >>
> >> /* intel_pipe_crc.c */
> >> int intel_pipe_crc_create(struct drm_minor *minor);
> >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >> index d1b1efc..a08ab99 100644
> >> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >> @@ -1362,8 +1362,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state,
> >> return true;
> >> }
> >>
> >> -static bool
> >> -intel_hdmi_ycbcr420_config(struct drm_connector *connector,
> >> +bool intel_hdmi_ycbcr420_config(struct drm_connector *connector,
> >> struct intel_crtc_state *config,
> >> int *clock_12bpc, int *clock_8bpc)
> >> {
> >> @@ -1380,6 +1379,10 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
> >> *clock_8bpc /= 2;
> >> config->ycbcr420 = true;
> >>
> >> + /* LSPCON doesn't need scaler for YCBCR420 output */
> >> + if (config->lspcon_active)
> >> + return true;
> >> +
> >> /* YCBCR 420 output conversion needs a scaler */
> >> if (skl_update_scaler_crtc_420_output(config)) {
> >> DRM_ERROR("Scaler allocation for output failed\n");
> >> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> >> index a350d79..f611b6d 100644
> >> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> >> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> >> @@ -202,6 +202,21 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
> >> DRM_DEBUG_KMS("LSPCON DP descriptor mismatch after resume\n");
> >> }
> >>
> >> +bool lspcon_ycbcr420_config(struct drm_connector *connector,
> >> + struct intel_crtc_state *config,
> >> + int *clock_12bpc, int *clock_8bpc)
> >> +{
> >> + 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)) {
> >> + return intel_hdmi_ycbcr420_config(connector, config,
> >> + clock_12bpc, clock_8bpc);
> >> + }
> >> +
> >> + return false;
> >> +}
> >> +
> >> void lspcon_resume(struct intel_lspcon *lspcon)
> >> {
> >> enum drm_lspcon_mode expected_mode;
> >> @@ -233,6 +248,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 (!IS_GEN9(dev_priv)) {
> >> DRM_ERROR("LSPCON is supported on GEN9 only\n");
> >> @@ -264,6 +280,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
> >> return false;
> >> }
> >>
> >> + connector->ycbcr_420_allowed = true;
> >> drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd));
> >>
> >> DRM_DEBUG_KMS("Success: LSPCON init\n");
> >> --
> >> 2.7.4
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
