diff mbox

[v2] drm/mxsfb: fix pixel clock polarity

Message ID 20161214204809.27802-1-stefan@agner.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Agner Dec. 14, 2016, 8:48 p.m. UTC
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@agner.ch>
---
Changes since v1:
- Improved comments/fixed typo

 drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Marek Vasut Dec. 14, 2016, 9:25 p.m. UTC | #1
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 <stefan@agner.ch>

Acked-by: Marek Vasut <marex@denx.de>

> ---
> 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);
>
Stefan Agner Feb. 8, 2017, 5:19 a.m. UTC | #2
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 <stefan@agner.ch>
> 
> Acked-by: Marek Vasut <marex@denx.de>
> 

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);
>>
Daniel Vetter Feb. 8, 2017, 7:19 a.m. UTC | #3
On Wed, Feb 8, 2017 at 6:19 AM, Stefan Agner <stefan@agner.ch> 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 <stefan@agner.ch>
>>
>> Acked-by: Marek Vasut <marex@denx.de>
>>
>
> 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
Marek Vasut Feb. 8, 2017, 10:19 a.m. UTC | #4
On 02/08/2017 08:19 AM, Daniel Vetter wrote:
> On Wed, Feb 8, 2017 at 6:19 AM, Stefan Agner <stefan@agner.ch> 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 <stefan@agner.ch>
>>>
>>> Acked-by: Marek Vasut <marex@denx.de>
>>>
>>
>> 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.
diff mbox

Patch

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);