Re: [PATCH] drm/i915/lspcon: Fix Parade LSPCON scrambling fail

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Oct 12, 2018 at 10:17:57PM +0300, Ville Syrjälä wrote:
> On Sat, Oct 13, 2018 at 12:26:57AM +0530, Sharma, Shashank wrote:
> > Regards
> > 
> > Shashank
> > 
> > 
> > On 10/13/2018 12:08 AM, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > The Parade LSPCON on KBL NUCs forgets to turn off scrambling/bit clock
> > > rate when switching from a mode that needs them to a mode that does
> > > not. This manifests as a "no signal" on my TV when I try to go from
> > > 4k to 1080p for example. Resetting the SCDC register bits with
> > > i2cset is sufficient to restore the picture to the screen.
> > >
> > > Here's the OUI/fw revision for the LSPCON chip in question:
> > > DP branch: OUI 00-1c-f8 dev-ID 175IB0 HW-rev 1.0 SW-rev 7.32 quirks 0x0000
> > >
> > > Asking users to poke at SCDC with i2cset is a bit much, so
> > > let's work around this in the driver. We don't need to go all
> > > out here and compute whether scrambling is needed or not as
> > > LSPCON will do that itself. If scrambling is actually
> > > required LSPCON does not forget to enable it.
> > >
> > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/intel_ddi.c  | 52 ++++++++++++++++++++++++++++---
> > >   drivers/gpu/drm/i915/intel_drv.h  |  1 +
> > >   drivers/gpu/drm/i915/intel_hdmi.c |  5 ++-
> > >   3 files changed, 51 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 47960c92cbbf..ef502fc9add1 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -2984,6 +2984,23 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
> > >   		intel_ddi_pre_enable_dp(encoder, crtc_state, conn_state);
> > >   }
> > >   
> > > +static void intel_ddi_pre_enable_lspcon(struct intel_encoder *encoder,
> > > +					const struct intel_crtc_state *crtc_state,
> > > +					const struct drm_connector_state *conn_state)
> > > +{
> > > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > > +
> > > +	intel_ddi_pre_enable(encoder, crtc_state, conn_state);
> > > +
> > > +	/*
> > > +	 * Parade LSPCON forgets to turn off scrambling/bit clock rate
> > > +	 * when switching from a mode that needs them to a mode that
> > > +	 * does not.
> > > +	 */
> > > +	intel_hdmi_handle_sink_scrambling(encoder, conn_state->connector,
> > > +					  &intel_dp->aux.ddc, false, false);
> > > +}
> > > +
> > >   static void intel_disable_ddi_buf(struct intel_encoder *encoder)
> > >   {
> > >   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > @@ -3086,6 +3103,23 @@ static void intel_ddi_post_disable(struct intel_encoder *encoder,
> > >   					  old_crtc_state, old_conn_state);
> > >   }
> > >   
> > > +static void intel_ddi_post_disable_lspcon(struct intel_encoder *encoder,
> > > +					  const struct intel_crtc_state *old_crtc_state,
> > > +					  const struct drm_connector_state *old_conn_state)
> > > +{
> > > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > > +
> > > +	/*
> > > +	 * Parade LSPCON forgets to turn off scrambling/bit clock rate
> > > +	 * when switching from a mode that needs them to a mode that
> > > +	 * does not.
> > > +	 */
> > > +	intel_hdmi_handle_sink_scrambling(encoder, old_conn_state->connector,
> > > +					  &intel_dp->aux.ddc, false, false);
> > > +
> > > +	intel_ddi_post_disable(encoder, old_crtc_state, old_conn_state);
> > > +}
> > Few thoughts:
> > - Would it make more sense to move these 2 functions to intel_lspcon.c ?
> 
> Would require more non-statics. But I guess it might be nice to attempt
> isolating it as much as possible.
> 
> > -  And then add a lspcon->vendor == VENDOR_PARADE check, so that we will 
> > run the code only when needed.
> 
> Maybe. The ->vendor thing isn't in yet, and we'd have to backport it
> as well. Is it big?
> 
> Also at this time I have no idea whether Megachips LSPCON is similarly
> bugged. Would need to find one and test it.

Oh, and another open question is what happens if one of these chips is
in a DP->HDMI 2.0 dongle. In that case we might end up needing the same
workaround in the normal DP codepath as well :(

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx