On 2016-12-08 15:38, Marek Vasut wrote: > On 12/08/2016 09:46 PM, Stefan Agner wrote: >> On 2016-12-07 18:37, Marek Vasut wrote: >>> 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 ? >> >> Ok, lets create facts here: >> 1. SoloX Refrence Manual, Figure 37-13. shows DOTCLK_POL=0, and it shows >> that the controller drives signals on falling edge of the pixel clock. >> The i.MX 7 has the same figure. >> 2. Just to verify, I hooked up an oscilloscope on my i.MX 7: It shows >> that with DOTCLK_POL=0 the controller drives on falling edge: >> http://imgur.com/a/2f2Xt >> >> So my measurements verify what is in the i.MX data sheets. > > Good > >> The current code sets the bit when negative edge (falling edge) is >> requested, which is wrong: >> #define VDCTRL0_DOTCLK_ACT_FALLING (1 << 25) >> ... >> if (bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE) >> vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING; >> >> Not sure what is going on with your display, maybe the datasheet is just >> wrong (it requires DRM_BUS_FLAG_PIXDATA_NEGEDGE in fact) or it is some >> other artifact. > > This is probably where the problem crept in [1], droping PIXDATA_POSEDGE > actually makes this patch work for me. CCing Philipp. > > [1] https://patchwork.kernel.org/patch/9301517/ I looked at a old data sheet of that display and it seemed that PIXDATA_POSEDGE is the right thing. Panelook.cn lists newer data sheets, but I couldn't find them on the open internet, do you have access to a newer one? http://www.panelook.cn/COM43H4M85ULC_ORTUSTECH_4.3_LCM_overview_17296.html I guess in the end it doesn't matter: Given that it is verified that the controllers data sheet is right, I vote for merging that patch and fix the displays polarity... -- Stefan _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
