From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932718AbcLHUxD (ORCPT ); Thu, 8 Dec 2016 15:53:03 -0500 Received: from mail.kmu-office.ch ([178.209.48.109]:57134 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932681AbcLHUxC (ORCPT ); Thu, 8 Dec 2016 15:53:02 -0500 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Thu, 08 Dec 2016 12:46:27 -0800 From: Stefan Agner To: Marek Vasut Cc: airlied@linux.ie, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/mxsfb: fix pixel clock polarity In-Reply-To: References: <20161208002706.10147-1-stefan@agner.ch> <2ddf12d577254ee5b9195f9a16788cbd@agner.ch> Message-ID: <420de41d423a4b24b2651c85d9701123@agner.ch> User-Agent: Roundcube Webmail/1.1.3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. 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. -- Stefan