diff mbox

video: ARM CLCD: Added support for FBIOPAN_DISPLAY and virtual y resolution

Message ID 1424898082-1522-2-git-send-email-arun.ramamurthy@broadcom.com
State New, archived
Headers show

Commit Message

Arun Ramamurthy Feb. 25, 2015, 9:01 p.m. UTC
Added code to support FBIOPAN_DISPLAY. Also added yres_virtual
parameter to device tree to set the virtual y resolution

Reviewed-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
---
 .../devicetree/bindings/video/arm,pl11x.txt        |  4 +++
 drivers/video/fbdev/amba-clcd.c                    | 31 +++++++++++++++++++---
 2 files changed, 31 insertions(+), 4 deletions(-)

Comments

Pawel Moll March 2, 2015, 4:08 p.m. UTC | #1
On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote:
> Added code to support FBIOPAN_DISPLAY. Also added yres_virtual
> parameter to device tree to set the virtual y resolution
> 
> Reviewed-by: Ray Jui <rjui@broadcom.com>
> Reviewed-by: Scott Branden <sbranden@broadcom.com>
> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
> ---
>  .../devicetree/bindings/video/arm,pl11x.txt        |  4 +++
>  drivers/video/fbdev/amba-clcd.c                    | 31 +++++++++++++++++++---
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt
> index 14d6f87..2262cdb 100644
> --- a/Documentation/devicetree/bindings/video/arm,pl11x.txt
> +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt
> @@ -50,6 +50,10 @@ Optional properties:
>  	display mode, data is driven onto the LCD data lines at the
>  	programmed edge of CLCP when CLAC is in its active state.
>  
> +- yres_virtual: Virtual Y resolution,
> +	It can be used to configure a virtual y resolution. It
> +	must be a value larger than the actual y resolution.
> +

I'm not sure about this... The word "virtual" never works well with
device tree nodes defined as "hardware description".

I understand what you're doing, but adding this property to the display
controller's node doesn't sound right. How does this describe hardware?
If anywhere, it's more like a job for the panel node?

Pawel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux March 2, 2015, 4:11 p.m. UTC | #2
On Mon, Mar 02, 2015 at 04:08:29PM +0000, Pawel Moll wrote:
> I'm not sure about this... The word "virtual" never works well with
> device tree nodes defined as "hardware description".
> 
> I understand what you're doing, but adding this property to the display
> controller's node doesn't sound right. How does this describe hardware?
> If anywhere, it's more like a job for the panel node?

A better description (and implementation) would be to describe the size
of the RAM available for video purposes.  The driver can then use the
requested virtual X resolution to limit (and/or compute) the virtual Y
resolution to allow Y panning/wrapping of the display.

This would match some hardware where the video RAM is indeed a separate
physical set of RAM (such as the IM-PD/1).
Arun Ramamurthy March 2, 2015, 7:09 p.m. UTC | #3
On 15-03-02 08:11 AM, Russell King - ARM Linux wrote:
> On Mon, Mar 02, 2015 at 04:08:29PM +0000, Pawel Moll wrote:
>> I'm not sure about this... The word "virtual" never works well with
>> device tree nodes defined as "hardware description".
>>
>> I understand what you're doing, but adding this property to the display
>> controller's node doesn't sound right. How does this describe hardware?
>> If anywhere, it's more like a job for the panel node?
>
I see what you are saying Pawel, I can follow Russell's recommendation 
of adding a RAM size node called max-memory-available or something similar
> A better description (and implementation) would be to describe the size
> of the RAM available for video purposes.  The driver can then use the
> requested virtual X resolution to limit (and/or compute) the virtual Y
> resolution to allow Y panning/wrapping of the display.
>

In this scenario, where would I specify the virtual X resolution? I am 
assuming it would be in the panel-timing node as Pawel suggested?

> This would match some hardware where the video RAM is indeed a separate
> physical set of RAM (such as the IM-PD/1).
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux March 2, 2015, 7:12 p.m. UTC | #4
On Mon, Mar 02, 2015 at 11:09:51AM -0800, Arun Ramamurthy wrote:
> 
> 
> On 15-03-02 08:11 AM, Russell King - ARM Linux wrote:
> >On Mon, Mar 02, 2015 at 04:08:29PM +0000, Pawel Moll wrote:
> >>I'm not sure about this... The word "virtual" never works well with
> >>device tree nodes defined as "hardware description".
> >>
> >>I understand what you're doing, but adding this property to the display
> >>controller's node doesn't sound right. How does this describe hardware?
> >>If anywhere, it's more like a job for the panel node?
> >
> I see what you are saying Pawel, I can follow Russell's recommendation of
> adding a RAM size node called max-memory-available or something similar
> >A better description (and implementation) would be to describe the size
> >of the RAM available for video purposes.  The driver can then use the
> >requested virtual X resolution to limit (and/or compute) the virtual Y
> >resolution to allow Y panning/wrapping of the display.
> >
> 
> In this scenario, where would I specify the virtual X resolution? I am
> assuming it would be in the panel-timing node as Pawel suggested?

virtual X should default to real X if it's not set, otherwise it
should come from the selected mode.

Drivers which get this right are things like acornfb, where we've
used ywrap scolling since it was merged.
Rob Herring March 2, 2015, 11:22 p.m. UTC | #5
On Mon, Mar 2, 2015 at 1:09 PM, Arun Ramamurthy
<arun.ramamurthy@broadcom.com> wrote:
>
>
> On 15-03-02 08:11 AM, Russell King - ARM Linux wrote:
>>
>> On Mon, Mar 02, 2015 at 04:08:29PM +0000, Pawel Moll wrote:
>>>
>>> I'm not sure about this... The word "virtual" never works well with
>>> device tree nodes defined as "hardware description".
>>>
>>> I understand what you're doing, but adding this property to the display
>>> controller's node doesn't sound right. How does this describe hardware?
>>> If anywhere, it's more like a job for the panel node?
>>
>>
> I see what you are saying Pawel, I can follow Russell's recommendation of
> adding a RAM size node called max-memory-available or something similar

We've already got a binding for reserved memory regions for this
purpose. And there is also simplefb binding.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arun Ramamurthy March 4, 2015, 12:31 a.m. UTC | #6
On 15-03-02 03:22 PM, Rob Herring wrote:
> On Mon, Mar 2, 2015 at 1:09 PM, Arun Ramamurthy
> <arun.ramamurthy@broadcom.com> wrote:
>>
>>
>> On 15-03-02 08:11 AM, Russell King - ARM Linux wrote:
>>>
>>> On Mon, Mar 02, 2015 at 04:08:29PM +0000, Pawel Moll wrote:
>>>>
>>>> I'm not sure about this... The word "virtual" never works well with
>>>> device tree nodes defined as "hardware description".
>>>>
>>>> I understand what you're doing, but adding this property to the display
>>>> controller's node doesn't sound right. How does this describe hardware?
>>>> If anywhere, it's more like a job for the panel node?
>>>
>>>
>> I see what you are saying Pawel, I can follow Russell's recommendation of
>> adding a RAM size node called max-memory-available or something similar
>
> We've already got a binding for reserved memory regions for this
> purpose. And there is also simplefb binding.
>
Rob, I am not sure what binding you are referring to, could you be more 
specific? Also how does the simplefb bindings apply to me considering 
that is a separate driver? Thanks
> Rob
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt
index 14d6f87..2262cdb 100644
--- a/Documentation/devicetree/bindings/video/arm,pl11x.txt
+++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt
@@ -50,6 +50,10 @@  Optional properties:
 	display mode, data is driven onto the LCD data lines at the
 	programmed edge of CLCP when CLAC is in its active state.
 
+- yres_virtual: Virtual Y resolution,
+	It can be used to configure a virtual y resolution. It
+	must be a value larger than the actual y resolution.
+
 Required sub-nodes:
 
 - port: describes LCD panel signals, following the common binding
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 4e4e50f..3bc09ad 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -454,6 +454,18 @@  static int clcdfb_mmap(struct fb_info *info,
 	return ret;
 }
 
+static int clcdfb_pan_display(struct fb_var_screeninfo *var,
+			      struct fb_info *info)
+{
+	struct clcd_fb *fb;
+
+	info->var = *var;
+	fb = to_clcd(info);
+	clcdfb_set_start(fb);
+
+	return 0;
+}
+
 static struct fb_ops clcdfb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_check_var	= clcdfb_check_var,
@@ -464,6 +476,7 @@  static struct fb_ops clcdfb_ops = {
 	.fb_copyarea	= cfb_copyarea,
 	.fb_imageblit	= cfb_imageblit,
 	.fb_mmap	= clcdfb_mmap,
+	.fb_pan_display	= clcdfb_pan_display,
 };
 
 static int clcdfb_register(struct clcd_fb *fb)
@@ -517,14 +530,16 @@  static int clcdfb_register(struct clcd_fb *fb)
 	fb->fb.fix.type		= FB_TYPE_PACKED_PIXELS;
 	fb->fb.fix.type_aux	= 0;
 	fb->fb.fix.xpanstep	= 0;
-	fb->fb.fix.ypanstep	= 0;
+	if (fb->fb.var.yres_virtual > fb->panel->mode.yres)
+		fb->fb.fix.ypanstep = 1;
+	else
+		fb->fb.fix.ypanstep = 0;
 	fb->fb.fix.ywrapstep	= 0;
 	fb->fb.fix.accel	= FB_ACCEL_NONE;
 
 	fb->fb.var.xres		= fb->panel->mode.xres;
 	fb->fb.var.yres		= fb->panel->mode.yres;
 	fb->fb.var.xres_virtual	= fb->panel->mode.xres;
-	fb->fb.var.yres_virtual	= fb->panel->mode.yres;
 	fb->fb.var.bits_per_pixel = fb->panel->bpp;
 	fb->fb.var.grayscale	= fb->panel->grayscale;
 	fb->fb.var.pixclock	= fb->panel->mode.pixclock;
@@ -690,7 +705,7 @@  static int clcdfb_of_init_display(struct clcd_fb *fb)
 	struct device_node *endpoint;
 	int err;
 	unsigned int bpp;
-	u32 max_bandwidth;
+	u32 max_bandwidth, yres_virtual;
 	u32 tft_r0b0g0[3];
 
 	fb->panel = devm_kzalloc(&fb->dev->dev, sizeof(*fb->panel), GFP_KERNEL);
@@ -730,6 +745,14 @@  static int clcdfb_of_init_display(struct clcd_fb *fb)
 	fb->panel->width = -1;
 	fb->panel->height = -1;
 
+	/* if yres_virtual property is not specified in device tree,
+	 * set it as the actual y resolution */
+	if (of_property_read_u32(fb->dev->dev.of_node,
+				"yres_virtual", &yres_virtual))
+		fb->fb.var.yres_virtual = fb->panel->mode.yres;
+	else
+		fb->fb.var.yres_virtual = yres_virtual;
+
 	if (of_property_read_u32_array(endpoint,
 			"arm,pl11x,tft-r0g0b0-pads",
 			tft_r0b0g0, ARRAY_SIZE(tft_r0b0g0)) == 0)
@@ -797,7 +820,7 @@  static int clcdfb_of_dma_setup(struct clcd_fb *fb)
 	if (err)
 		return err;
 
-	framesize = fb->panel->mode.xres * fb->panel->mode.yres *
+	framesize = fb->panel->mode.xres * fb->fb.var.yres_virtual *
 			fb->panel->bpp / 8;
 	fb->fb.screen_base = dma_alloc_coherent(&fb->dev->dev, framesize,
 			&dma, GFP_KERNEL);