| Message ID | 1424898082-1522-2-git-send-email-arun.ramamurthy@broadcom.com |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
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
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).
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
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.
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
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 --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);