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. -- Stefan > The default change: Data enable should be high by default because that > is what most displays require, including yours. This won't break your > display, since you request DRM_BUS_FLAG_DE_HIGH anyway... > > We could debate whether that default change is necessary, but since it > also won't affect your display, I think it is a worthwhile change. > > -- > Stefan > > >> >>> -- >>> Stefan >>> >>> drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 11 +++++++++-- >>> 1 file changed, 9 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c >>> index 0818903..4bcc8a3 100644 >>> --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c >>> +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c >>> @@ -168,9 +168,16 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) >>> vdctrl0 |= VDCTRL0_HSYNC_ACT_HIGH; >>> if (m->flags & DRM_MODE_FLAG_PVSYNC) >>> vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH; >>> - if (bus_flags & DRM_BUS_FLAG_DE_HIGH) >>> + /* Data Enable should be high active by default */ >>> + if (!(bus_flags & DRM_BUS_FLAG_DE_LOW)) >>> vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH; >>> - if (bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE) >>> + /* >>> + * Note: DRM_BUS_FLAG_PIXDATA defines are controller centric, >>> + * controllers VDCTRL0_DOTCLK is display centric. >>> + * Drive on positive edige -> display samples on falling edge >>> + * DRM_BUS_FLAG_PIXDATA_POSEDGE -> VDCTRL0_DOTCLK_ACT_FALLING >>> + */ >>> + if (bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE) >>> vdctrl0 |= VDCTRL0_DOTCLK_ACT_FALLING; >>> >>> writel(vdctrl0, mxsfb->base + LCDC_VDCTRL0); >>> _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
