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@xxxxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxx>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
