Re: [PATCH v2] drm/display: Don't assume dual mode adaptors support i2c sub-addressing

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

On Tue, Nov 15, 2022 at 06:31:31PM +0200, Jani Nikula wrote:
> On Fri, 07 Oct 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Oct 06, 2022 at 06:11:37PM +0300, Ville Syrjälä wrote:
> >> On Thu, Oct 06, 2022 at 11:33:14AM +0200, Simon Rettberg wrote:
> >> > Current dual mode adaptor ("DP++") detection code assumes that all
> >> > adaptors support i2c sub-addressing for read operations from the
> >> > DP-HDMI adaptor ID buffer.  It has been observed that multiple
> >> > adaptors do not in fact support this, and always return data starting
> >> > at register 0.  On affected adaptors, the code fails to read the proper
> >> > registers that would identify the device as a type 2 adaptor, and
> >> > handles those as type 1, limiting the TMDS clock to 165MHz, even if
> >> > the according register would announce a higher TMDS clock.
> >> > Fix this by always reading the ID buffer starting from offset 0, and
> >> > discarding any bytes before the actual offset of interest.
> >> > 
> >> > We tried finding authoritative documentation on whether or not this is
> >> > allowed behaviour, but since all the official VESA docs are paywalled,
> >> > the best we could come up with was the spec sheet for Texas Instruments'
> >> > SNx5DP149 chip family.[1]  It explicitly mentions that sub-addressing is
> >> > supported for register writes, but *not* for reads (See NOTE in
> >> > section 8.5.3).  Unless TI openly decided to violate the VESA spec, one
> >> > could take that as a hint that sub-addressing is in fact not mandated
> >> > by VESA.
> >> > The other two adaptors affected used the PS8409(A) and the LT8611,
> >> > according to the data returned from their ID buffers.
> >> > 
> >> > [1] https://www.ti.com/lit/ds/symlink/sn75dp149.pdf
> >> > 
> >> > Signed-off-by: Simon Rettberg <simon.rettberg@rz.uni-freiburg.de>
> >> > Reviewed-by: Rafael Gieschke <rafael.gieschke@rz.uni-freiburg.de>
> >> > ---
> >> > 
> >> > v2 changes form last submission's feedback (thanks for taking the time):
> >> > - Added a shortened version of our "background story" to the commit message
> >> > - Only use tmpbuf if the read offset is != 0
> >> 
> >> Bounced to intel-gfx to get the i915 CI to check it...
> >
> > CI didn't blow up, and I also gave this a quick smoking on my end
> > with both type 1 HDMI and type 2 HDMI adaptors. 
> >
> > I'm thinking we want a cc:stable on this? I can slap that on
> > when pushing if there are no objections?
> 
> I guess this fell between the cracks? :(

I think I just got stuck waiting for people to say
something about the cc:stable idea. But presumbly
since no one objected everyone is happy with it.

> 
> Ville, r-b? Going to push?

Pushed to drm-misc-fixes now, with cc:stable.
Thanks for the patch.

-- 
Ville Syrjälä
Intel