From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756672AbcLNUir (ORCPT ); Wed, 14 Dec 2016 15:38:47 -0500 Received: from mail-out.m-online.net ([212.18.0.10]:39854 "EHLO mail-out.m-online.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752720AbcLNUip (ORCPT ); Wed, 14 Dec 2016 15:38:45 -0500 X-Auth-Info: fKjtR6YxC6Sd4icbGO7oyHbl5vrfEo/8U4cBwq2a1K4= Subject: Re: [PATCH] drm/mxsfb: fix pixel clock polarity To: Stefan Agner References: <20161208002706.10147-1-stefan@agner.ch> <2ddf12d577254ee5b9195f9a16788cbd@agner.ch> <420de41d423a4b24b2651c85d9701123@agner.ch> <1da939e2-f616-e60c-416b-a71e6ecd1dd4@denx.de> <7ab51983dde9ae6539e7ecb6a38777f6@agner.ch> <4c0f22825853b7ec9b55e8f3dece82f8@agner.ch> Cc: airlied@linux.ie, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Philipp Zabel From: Marek Vasut Message-ID: <4aa4b746-07f2-8539-9f06-a5f9b57063ce@denx.de> Date: Wed, 14 Dec 2016 21:38:40 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 In-Reply-To: <4c0f22825853b7ec9b55e8f3dece82f8@agner.ch> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/14/2016 09:29 PM, Stefan Agner wrote: > On 2016-12-14 00:04, Marek Vasut wrote: >> On 12/14/2016 01:01 AM, Stefan Agner wrote: >>> 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 >>>>>>>>>> --- >>>>>>>>>> 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? >> >> Which "version" do you have ? Probably not though. >> > > I couldn't find it anymore, but I think it said Rev. 0 > >>> 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... >> >> Merging which patch ? > > This patch. > > And I guess, to keep your display working, a patch which changes the > Ortustech pixel clock flag... Well, let's do that. Btw can you send a V2 with s/edige/edge/ ? ;-) Thanks! -- Best regards, Marek Vasut