Re: [PATCH] drm/mxsfb: fix pixel clock polarity

[Marek Vasut]

On 12/08/2016 02:26 AM, Stefan Agner wrote:
> On 2016-12-07 16:59, Stefan Agner wrote:
>> On 2016-12-07 16:49, Marek Vasut wrote:
>>> On 12/08/2016 01:27 AM, Stefan Agner wrote:
>>>> The DRM subsystem specifies the pixel clock polarity from a
>>>> controllers perspective: DRM_BUS_FLAG_PIXDATA_NEGEDGE means
>>>> the controller drives the data on pixel clocks falling edge.
>>>> That is the controllers DOTCLK_POL=0 (Default is data launched
>>>> at negative edge).
>>>>
>>>> Also change the data enable logic to be high active by default
>>>> and only change if explicitly requested via bus_flags. With
>>>> that defaults are:
>>>> - Data enable: high active
>>>> - Pixel clock polarity: controller drives data on negative edge
>>>>
>>>> Signed-off-by: Stefan Agner <stefan@xxxxxxxx>
>>>> ---
>>>> Hi Marek,
>>>
>>> Hi, that was quick, thanks for checking this.
>>
>> Yeah, I couldn't wait seeing it flying :-)
>>
>>>
>>>> I discovered this while testing on a i.MX 7 eLCDIF IP. Particularly the
>>>> non-standard DE polarity was causing issues. I was using a EDT display
>>>> which is part of simple panel driver since a while now and does not
>>>> specify any bus_flags currently... Of course I could (and probably should)
>>>> add the proper bus_flags there too, but there are several displays
>>>> which do not specify any polarity and likely rely on sensible driver
>>>> standards (which is afact high active for the DE signal).
>>>
>>> I actually use a panel which requires correct settings of the flags, see
>>> e0932f9d7ba9a16f99a84943b720f109de8e3e06 in mainline , so this patch
>>> would break things for me. So I wonder whether you should fix the panel
>>> driver or whether the mxsfb should be fixed ?
>>
>> If you ask me, mxsfb.
>>
>> Ok, there are actually two things, one is a bug, one is a default
>> change.
>>
>> The bug: Pixel clock polarity is clearly defined to be controller
>> centric (see comments around DRM_BUS_FLAG_PIXDATA_*EDGE in
>> include/drm/drm_connector.h). The driver does it wrong currently.
>>
>> This might affect your display, and if it does, it is actually wrong
>> also in your display... However, since it is a bug, I think it is not
>> really a debate, it should be fixed...
> 
> FWIW, it seems that Ortustech com43h4m85ulc samples on falling edge, so
> DRM_BUS_FLAG_PIXDATA_POSEDGE seems right. And it means that DOTCLK_POL
> should be 1 (inverted), so with this patch the polarity should actually
> be correct for that panel.

Well, if I apply this patch, my image is shifted by 1 px to the left and
there is a 1px white bar on the right side, so I think I have some
polarity problem now ?

-- 
Best regards,
Marek Vasut