[PATCHv2 03/22] drm/bridge: tc358767: fix ansi 8b10b use

[Tomi Valkeinen]

On 03/05/2019 15:48, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Fri, May 03, 2019 at 02:43:51PM +0300, Tomi Valkeinen wrote:
>> On 23/04/2019 17:56, Laurent Pinchart wrote:
>>
>>>> During initial driver development I had one eDP display that reports 0 in Bit 0
>>>> (ANSI 8B/10B) of DPCD reg 0x0006 (MAIN_LINK_CHANNEL_CODING).
>>>> Also it does not react on setting Bit 0 (SET_ANSI 8B10B) in 0x0108
>>>> (MAIN_LINK_CHANNEL_CODING_SET) - after reading back it was 0 again.
>>>> So I had to disable 8B10 encoding on tc358767 side to make this display
>>>> work.
>>>
>>> Out of curiosity, how does the eDP display recover the clock without
>>> 8B/10B encoding ?
>>>
>>>> On other hand if there are displays that report zero bit 0 in
>>>> MAIN_LINK_CHANNEL_CODING while needing 8b10b then this patch looks
>>>> reasonable.
>>>>
>>>> May be driver should read back MAIN_LINK_CHANNEL_CODING_SET
>>>> register after setting it and check if 8b10b actually enabled?
>>>
>>> This sounds like a reasonable thing to try. Tomi, do you still have
>>> accesss to those faulty monitors ?
>>
>> On my monitor it does not seem to matter whether I write 0 or 1 to
>> MAIN_LINK_CHANNEL_CODING_SET, as long as I have 8b10b enabled on
>> TC358767 side. The writes do go through, and I can read the written bit
>> back.
>>
>> So... I guess when we talk about eDP panels, there may be all kinds of
>> oddities, as they're usually meant to be used with a certain configuration.
>>
>> But if everyone agrees that 8B10B is a normal, required feature of DP,
>> and there is an eDP panel that needs 8B10B disabled, maybe that panel
>> has to be handled separately as a special case? A dts entry to disable
>> 8B10B? Or something. But it does not sound like a good idea for the
>> driver to try to guess this.
> 
> As reported by Andrey, there is at least one panel that would break with
> this patch. I agree 8b10b should be the default, but if the above
> procedure works for all the panels we know about, is there an issue
> implementing it ?

Well, we don't have data for a big set of panels. I'm worried that such
a change, which, in my opinion, makes the driver guess whether to enable
or disable 8b10b, can break other panels or monitors if the guess
doesn't go right. Especially as disabling 8b10b does not sound like a
valid setup "officially".

I agree that if the panel Andrey mentioned is still used, we need to
handle it somehow. But I think explicitly handling such a case is better
than guessing.

And isn't this something that's not really TC358767 specific? If that
panel is used with other bridges, we need to deal with this case with
those bridges too? Or is TC358767 the only bridge that allows disabling
8b10b?

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki