[RESEND PATCH 1/2] devicetree: mxsfb: add reset-active property

[RESEND PATCH 1/2] devicetree: mxsfb: add reset-active property

[Mans Rullgard]

Some boards connect the LCD_RESET pin to a reset input on the
display panel.  On these boards, this pin must be set to the
proper level for the display to function.

This adds an optional "reset-active" property to the "display"
subnode such that devicetrees can specify the desired polarity
of the LCD_RESET pin.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 Documentation/devicetree/bindings/display/mxsfb.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt
index 96ec5179c8a0..cb7212a6bdf2 100644
--- a/Documentation/devicetree/bindings/display/mxsfb.txt
+++ b/Documentation/devicetree/bindings/display/mxsfb.txt
@@ -13,6 +13,11 @@ Required properties:
 - bits-per-pixel : <16> for RGB565, <32> for RGB888/666.
 - bus-width : number of data lines.  Could be <8>, <16>, <18> or <24>.
 
+Optional properties:
+- reset-active : <0>: reset pin is active low
+                 <1>: reset pin is active high
+                 omitted: reset pin not used
+
 Required sub-node:
 - display-timings : Refer to binding doc display-timing.txt for details.
 
-- 
2.7.0

[RESEND PATCH 2/2] video: mxsfb: manage LCD_RESET signal according to reset-active property

[Mans Rullgard]

Activate/deactivate the LCD_RESET signal as specified by the
reset-active DT property when the controller is disabled/enabled.
If the property is missing, leave the signal unchanged.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/video/fbdev/mxsfb.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
index 4e6608ceac09..0200a0f16675 100644
--- a/drivers/video/fbdev/mxsfb.c
+++ b/drivers/video/fbdev/mxsfb.c
@@ -99,6 +99,7 @@
 #define CTRL1_FIFO_CLEAR		(1 << 21)
 #define CTRL1_SET_BYTE_PACKAGING(x)	(((x) & 0xf) << 16)
 #define CTRL1_GET_BYTE_PACKAGING(x)	(((x) >> 16) & 0xf)
+#define CTRL1_RESET			(1 << 0)
 
 #define TRANSFER_COUNT_SET_VCOUNT(x)	(((x) & 0xffff) << 16)
 #define TRANSFER_COUNT_GET_VCOUNT(x)	(((x) >> 16) & 0xffff)
@@ -152,6 +153,9 @@
 #define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
 #define MXSFB_SYNC_DOTCLK_FALLING_ACT	(1 << 7) /* negtive edge sampling */
 
+#define MXSFB_RESET_LOW			1
+#define MXSFB_RESET_HIGH		2
+
 enum mxsfb_devtype {
 	MXSFB_V3,
 	MXSFB_V4,
@@ -181,6 +185,7 @@ struct mxsfb_info {
 	unsigned dotclk_delay;
 	const struct mxsfb_devdata *devdata;
 	u32 sync;
+	u32 reset;
 	struct regulator *reg_lcd;
 };
 
@@ -362,6 +367,11 @@ static void mxsfb_enable_controller(struct fb_info *fb_info)
 
 	writel(CTRL_RUN, host->base + LCDC_CTRL + REG_SET);
 
+	if (host->reset == MXSFB_RESET_HIGH)
+		writel(CTRL1_RESET, host->base + LCDC_CTRL1 + REG_CLR);
+	else if (host->reset == MXSFB_RESET_LOW)
+		writel(CTRL1_RESET, host->base + LCDC_CTRL1 + REG_SET);
+
 	host->enabled = 1;
 }
 
@@ -388,6 +398,11 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
 		loop--;
 	}
 
+	if (host->reset == MXSFB_RESET_HIGH)
+		writel(CTRL1_RESET, host->base + LCDC_CTRL1 + REG_SET);
+	else if (host->reset == MXSFB_RESET_LOW)
+		writel(CTRL1_RESET, host->base + LCDC_CTRL1 + REG_CLR);
+
 	reg = readl(host->base + LCDC_VDCTRL4);
 	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
 
@@ -410,7 +425,7 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
 static int mxsfb_set_par(struct fb_info *fb_info)
 {
 	struct mxsfb_info *host = to_imxfb_host(fb_info);
-	u32 ctrl, vdctrl0, vdctrl4;
+	u32 ctrl, ctrl1, vdctrl0, vdctrl4;
 	int line_size, fb_size;
 	int reenable = 0;
 
@@ -439,12 +454,13 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 
 	ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER |
 		CTRL_SET_BUS_WIDTH(host->ld_intf_width);
+	ctrl1 = readl(host->base + LCDC_CTRL1) & CTRL1_RESET;
 
 	switch (fb_info->var.bits_per_pixel) {
 	case 16:
 		dev_dbg(&host->pdev->dev, "Setting up RGB565 mode\n");
 		ctrl |= CTRL_SET_WORD_LENGTH(0);
-		writel(CTRL1_SET_BYTE_PACKAGING(0xf), host->base + LCDC_CTRL1);
+		ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf);
 		break;
 	case 32:
 		dev_dbg(&host->pdev->dev, "Setting up RGB888/666 mode\n");
@@ -462,7 +478,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 			break;
 		}
 		/* do not use packed pixels = one pixel per word instead */
-		writel(CTRL1_SET_BYTE_PACKAGING(0x7), host->base + LCDC_CTRL1);
+		ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0x7);
 		break;
 	default:
 		mxsfb_disable_axi_clk(host);
@@ -472,6 +488,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 	}
 
 	writel(ctrl, host->base + LCDC_CTRL);
+	writel(ctrl1, host->base + LCDC_CTRL1);
 
 	writel(TRANSFER_COUNT_SET_VCOUNT(fb_info->var.yres) |
 			TRANSFER_COUNT_SET_HCOUNT(fb_info->var.xres),
@@ -736,6 +753,7 @@ static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
 	struct device_node *display_np;
 	struct videomode vm;
 	u32 width;
+	u32 reset;
 	int ret;
 
 	display_np = of_parse_phandle(np, "display", 0);
@@ -776,6 +794,10 @@ static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
 		goto put_display_node;
 	}
 
+	ret = of_property_read_u32(display_np, "reset-active", &reset);
+	if (!ret)
+		host->reset = reset ? MXSFB_RESET_HIGH : MXSFB_RESET_LOW;
+
 	ret = of_get_videomode(display_np, &vm, OF_USE_NATIVE_MODE);
 	if (ret) {
 		dev_err(dev, "failed to get videomode from DT\n");
-- 
2.7.0

Re: [RESEND PATCH 1/2] devicetree: mxsfb: add reset-active property

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1999 bytes --]


On 12/01/16 12:22, Mans Rullgard wrote:
> Some boards connect the LCD_RESET pin to a reset input on the
> display panel.  On these boards, this pin must be set to the
> proper level for the display to function.
> 
> This adds an optional "reset-active" property to the "display"
> subnode such that devicetrees can specify the desired polarity
> of the LCD_RESET pin.
> 
> Signed-off-by: Mans Rullgard <mans-2StjZFpD7GcAvxtiuMwx3w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/display/mxsfb.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt
> index 96ec5179c8a0..cb7212a6bdf2 100644
> --- a/Documentation/devicetree/bindings/display/mxsfb.txt
> +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
> @@ -13,6 +13,11 @@ Required properties:
>  - bits-per-pixel : <16> for RGB565, <32> for RGB888/666.
>  - bus-width : number of data lines.  Could be <8>, <16>, <18> or <24>.
>  
> +Optional properties:
> +- reset-active : <0>: reset pin is active low
> +                 <1>: reset pin is active high
> +                 omitted: reset pin not used
> +
>  Required sub-node:
>  - display-timings : Refer to binding doc display-timing.txt for details.

So maybe this is fine for the mxsfb if it doesn't support any kind of
panel drivers, and there's no plan to extend it. Otherwise the LCD_RESET
pin could perhaps be exposed as a GPIO for the panel drivers.

But even so, I think the definition of "reset" is a bit vague.

I know panels for which "reset" is a pulse, you assert it for a short
period. Other panels take reset more like a on/off switch. If I'm not
mistaken, this one is the latter kind.

I don't see any sleeps related to reset in the code. If reset is
asserted when the display is blanked, is it guaranteed that the reset
stays asserted long enough until the display is enabled again?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RESEND PATCH 1/2] devicetree: mxsfb: add reset-active property

[Måns Rullgård]

Tomi Valkeinen <tomi.valkeinen@ti.com> writes:

> On 12/01/16 12:22, Mans Rullgard wrote:
>> Some boards connect the LCD_RESET pin to a reset input on the
>> display panel.  On these boards, this pin must be set to the
>> proper level for the display to function.
>> 
>> This adds an optional "reset-active" property to the "display"
>> subnode such that devicetrees can specify the desired polarity
>> of the LCD_RESET pin.
>> 
>> Signed-off-by: Mans Rullgard <mans@mansr.com>
>> ---
>>  Documentation/devicetree/bindings/display/mxsfb.txt | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/display/mxsfb.txt b/Documentation/devicetree/bindings/display/mxsfb.txt
>> index 96ec5179c8a0..cb7212a6bdf2 100644
>> --- a/Documentation/devicetree/bindings/display/mxsfb.txt
>> +++ b/Documentation/devicetree/bindings/display/mxsfb.txt
>> @@ -13,6 +13,11 @@ Required properties:
>>  - bits-per-pixel : <16> for RGB565, <32> for RGB888/666.
>>  - bus-width : number of data lines.  Could be <8>, <16>, <18> or <24>.
>>  
>> +Optional properties:
>> +- reset-active : <0>: reset pin is active low
>> +                 <1>: reset pin is active high
>> +                 omitted: reset pin not used
>> +
>>  Required sub-node:
>>  - display-timings : Refer to binding doc display-timing.txt for details.
>
> So maybe this is fine for the mxsfb if it doesn't support any kind of
> panel drivers, and there's no plan to extend it. Otherwise the LCD_RESET
> pin could perhaps be exposed as a GPIO for the panel drivers.
>
> But even so, I think the definition of "reset" is a bit vague.
>
> I know panels for which "reset" is a pulse, you assert it for a short
> period. Other panels take reset more like a on/off switch. If I'm not
> mistaken, this one is the latter kind.
>
> I don't see any sleeps related to reset in the code. If reset is
> asserted when the display is blanked, is it guaranteed that the reset
> stays asserted long enough until the display is enabled again?

In the datasheet for the panel I'm dealing with, there's some vague
mention of 10 us (I missed it last time I looked), and my patch indeed
fails to ensure this is met.  Other panels will obviously have different
requirements.  To handle all cases properly, I suppose a configurable
delay after changing the reset pin should be added.

I could also take the lazy way out, pinmux this signal high and leave it
at that.

-- 
Måns Rullgård

Re: [RESEND PATCH 1/2] devicetree: mxsfb: add reset-active property

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]



On 12/01/16 15:10, Måns Rullgård wrote:

>> I don't see any sleeps related to reset in the code. If reset is
>> asserted when the display is blanked, is it guaranteed that the reset
>> stays asserted long enough until the display is enabled again?
> 
> In the datasheet for the panel I'm dealing with, there's some vague
> mention of 10 us (I missed it last time I looked), and my patch indeed
> fails to ensure this is met.  Other panels will obviously have different
> requirements.  To handle all cases properly, I suppose a configurable
> delay after changing the reset pin should be added.

Right. And then you need to ensure the powers are enabled in the right
order, the pixel clock is started at the right time (some need pix clock
before reset), and so on and so on =).

> I could also take the lazy way out, pinmux this signal high and leave it
> at that.

I've seen panels that require a reset after powers have been off, or
similar, but I hope those are minority.

So yes, probably just making sure the reset is not asserted is the most
generic and easy way forward.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]