Re: [PATCH v2] drm/mxsfb: fix pixel clock polarity
On 02/08/2017 08:19 AM, Daniel Vetter wrote: > On Wed, Feb 8, 2017 at 6:19 AM, Stefan Agner wrote: >> On 2016-12-14 13:25, Marek Vasut wrote: >>> On 12/14/2016 09:48 PM, 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 >>> >>> Acked-by: Marek Vasut >>> >> >> This seems not have made into drm-next yet. Not sure what the plan is >> here, who will pick this up? There is also another fix on ML for the >> same driver ("drm: mxsfb: Fix crash when provided invalid DT bindings"). > > By default, driver maintainers are expected to pick up driver patches. > Exception is trivial patches by newcomers, to get them on board fast > (and avoid frustration when a maintainer isn't around). drm-misc and > Dave by default don't pick up anything. Aha, OK. -- Best regards, Marek Vasut
Re: [PATCH v2] drm/mxsfb: fix pixel clock polarity
On Wed, Feb 8, 2017 at 6:19 AM, Stefan Agner wrote: > On 2016-12-14 13:25, Marek Vasut wrote: >> On 12/14/2016 09:48 PM, 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 >> >> Acked-by: Marek Vasut >> > > This seems not have made into drm-next yet. Not sure what the plan is > here, who will pick this up? There is also another fix on ML for the > same driver ("drm: mxsfb: Fix crash when provided invalid DT bindings"). By default, driver maintainers are expected to pick up driver patches. Exception is trivial patches by newcomers, to get them on board fast (and avoid frustration when a maintainer isn't around). drm-misc and Dave by default don't pick up anything. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH v2] drm/mxsfb: fix pixel clock polarity
Dave, Marek, On 2016-12-14 13:25, Marek Vasut wrote: > On 12/14/2016 09:48 PM, 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 > > Acked-by: Marek Vasut > This seems not have made into drm-next yet. Not sure what the plan is here, who will pick this up? There is also another fix on ML for the same driver ("drm: mxsfb: Fix crash when provided invalid DT bindings"). -- Stefan >> --- >> Changes since v1: >> - Improved comments/fixed typo >> >> 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 3770dd2..5556e53 100644 >> --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c >> +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c >> @@ -195,9 +195,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) >> +/* Make sure Data Enable is 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) >> +/* >> + * DRM_BUS_FLAG_PIXDATA_ defines are controller centric, >> + * controllers VDCTRL0_DOTCLK is display centric. >> + * Drive on positive edge -> 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); >>
Re: [PATCH v2] drm/mxsfb: fix pixel clock polarity
On 12/14/2016 09:48 PM, 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 Acked-by: Marek Vasut > --- > Changes since v1: > - Improved comments/fixed typo > > 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 3770dd2..5556e53 100644 > --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c > +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c > @@ -195,9 +195,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) > + /* Make sure Data Enable is 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) > + /* > + * DRM_BUS_FLAG_PIXDATA_ defines are controller centric, > + * controllers VDCTRL0_DOTCLK is display centric. > + * Drive on positive edge -> 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); > -- Best regards, Marek Vasut
[PATCH v2] drm/mxsfb: fix pixel clock polarity
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 --- Changes since v1: - Improved comments/fixed typo 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 3770dd2..5556e53 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -195,9 +195,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) + /* Make sure Data Enable is 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) + /* +* DRM_BUS_FLAG_PIXDATA_ defines are controller centric, +* controllers VDCTRL0_DOTCLK is display centric. +* Drive on positive edge -> 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); -- 2.10.2