Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [RFC] Standardize YUV support in the fbdev API
From: Laurent Pinchart @ 2011-05-17 22:07 UTC (permalink / raw)
  To: linux-fbdev; +Cc: linux-media, dri-devel

Hi everybody,

I need to implement support for a YUV frame buffer in an fbdev driver. As the 
fbdev API doesn't support this out of the box, I've spent a couple of days 
reading fbdev (and KMS) code and thinking about how we could cleanly add YUV 
support to the API. I'd like to share my findings and thoughts, and hopefully 
receive some comments back.

The terms 'format', 'pixel format', 'frame buffer format' and 'data format' 
will be used interchangeably in this e-mail. They all refer to the way pixels 
are stored in memory, including both the representation of a pixel as integer 
values and the layout of those integer values in memory.


History and current situation
-----------------------------

The fbdev API predates YUV frame buffers. In those old days developers only 
cared (and probably thought) about RGB frame buffers, and they developed an 
API that could express all kind of weird RGB layout in memory (such as R-
GGGGGGGGGGG-BBBB for instance, I can't imagine hardware implementing that). 
This resulted in individual bit field description for the red, green, blue and 
alpha components:

struct fb_bitfield {
        __u32 offset;                  /* beginning of bitfield        */
        __u32 length;                  /* length of bitfield           */
        __u32 msb_right;               /* != 0 : Most significant bit is */
                                       /* right */
};

Grayscale formats were pretty common, so a grayscale field tells color formats 
(grayscale = 0) from grayscale formats (grayscale != 0).

People already realized that hardware developers were crazily inventive (the 
word to remember here is crazily), and that non-standard formats would be 
needed at some point. The fb_var_screeninfo ended up containing the following 
format-related fields.

struct fb_var_screeninfo {
        ...
        __u32 bits_per_pixel;          /* guess what                   */
        __u32 grayscale;               /* != 0 Graylevels instead of colors */

        struct fb_bitfield red;        /* bitfield in fb mem if true color, */
        struct fb_bitfield green;      /* else only length is significant */
        struct fb_bitfield blue;
        struct fb_bitfield transp;     /* transparency                 */

        __u32 nonstd;                  /* != 0 Non standard pixel format */
        ...
};

Two bits have been specified for the nonstd field:

#define FB_NONSTD_HAM           1  /* Hold-And-Modify (HAM)        */
#define FB_NONSTD_REV_PIX_IN_B  2  /* order of pixels in each byte is reversed 
*/

The FB_NONSTD_HAM bit is used by the video/amifb.c driver only to enable HAM 
mode[1]. I very much doubt that any new hardware will implement HAM mode (and 
I certainly hope none will).

The FB_NONSTD_REV_PIX_IN_B is used in video/fb_draw.h by the generic bitblit, 
fillrect and copy area implementations, not directly by drivers.

A couple of drivers hardcode the nonstd field to 1 for some reason. Those are 
video/arcfb.c (1bpp gray display), video/hecubafb.c (1bpp gray display) and 
video/metronomefb.c (8bpp gray display).

The following drivers use nonstd for various other (and sometimes weird) 
purposes:

video/arkfb.c
        Used in 4bpp mode only, to control fb_setcolreg operation
video/fsl-diu-fb.c
        Set var->nonstd bits into var->sync (why?)
video/pxafb.c
        Encode frame buffer xpos and ypos in the nonstd field
video/s3fb.c
        Used in 4bpp mode only, to control fb_setcolreg operation
video/vga16fb.c
        When panning in non-8bpp, non-text mode, decrement xoffset
        Do some other weird stuff when not 0
video/i810/i810_main.c
        Select direct color mode when set to 1 (truecolor otherwise)

Fast forward a couple of years, hardware provides support for YUV frame 
buffers. Several drivers had to provide YUV format selection to applications, 
without any standard API to do so. All of them used the nonstd field for that 
purpose:

media/video/ivtv/
        Enable YUV mode when set to 1
video/pxafb.c
        Encode pixel format in the nonstd field
video/sh_mobile_lcdfb.c
        If bpp = 12 and nonstd != 0, enable NV12 mode
        If bpp = 16 or bpp = 24, ?
video/omap/omapfb_main.c
        Select direct color mode when set to 1 (depend on bpp otherwise)
        Used as a pixel format identifier (YUV422, YUV420 or YUY422)
video/omap2/omapfb/omapfb-main.c
        Select direct color mode when set to 1 (depend on bpp otherwise)
        Used as a pixel format identifier (YUV422 or YUY422)

Those drivers use the nonstd field in different, incompatible ways.


Other related APIs
------------------

Beside the fbdev API, two other kernel/userspace APIs deal with video-related 
modes and formats.

- Kernel Mode Setting (KMS)

The KMS API is similar in purpose to XRandR. Its main purpose is to provide a 
kernel API to configure output video modes. As such, it doesn't care about 
frame buffer formats, as they are irrelevant at the CRTC output.

In addition to handling video modes, the KMS API also provides support for 
creating (and managing) frame buffer devices, as well as dumb scan-out 
buffers. In-memory data format is relevant there, but KMS only handles width, 
height, pitch, depth and bit-per-pixel information. The API doesn't care 
whether the frame buffer or the dumb scan-out buffer contains RGB or YUV data.

An RFC[2] has recently been posted to the dri-devel mailing list to "add 
overlays as first class KMS objects". The proposal includes explicit overlay 
format support, and discussions have so far pushed towards reusing V4L format 
codes for the KMS API.

- Video 4 Linux (V4L)

The V4L API version 2 (usually called V4L2) has since the beginning included 
explicit support for data format, referred to as pixel formats.

Pixel formats are identified by a 32-bit number in the form of a four 
characters code (4CC or FCC[3]). The list of FCCs defined by V4L2 is available 
in [4].

A pixel format uniquely defines the layout of pixel data in memory, including 
the format type (RGB, YUV, ...), number of bits per components, components 
order and subsampling. It doesn't define the range of acceptable values for 
pixel components (such as full-range YUV vs. BT.601[5]), which is carried 
through a separate colorspace identifier[6].


YUV support in the fbdev API
----------------------------

Experience with the V4L2 API, both for desktop and embedded devices, and the 
KMS API, suggests that recent hardware tend to standardize on a relatively 
small number of pixel formats that don't require bitfield-level descriptions. 
A pixel format definition based on identifiers should thus fullfill the 
hardware needs for the foreseeable future.

Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share data 
and buffers between those subsystems, and the planned use of V4L2 FCCs in the 
KMS overlay API, I believe using V4L2 FCCs to identify fbdev formats would be 
a wise decision.

To select a frame buffer YUV format, the fb_var_screeninfo structure will need 
to be extended with a format field. The fbdev API and ABI must not be broken, 
which prevents us from changing the current structure layout and replacing the 
existing format selection mechanism (through the red, green, blue and alpha 
bitfields) completely.

Several solutions exist to introduce a format field in the fb_var_screeninfo 
structure.

- Use the nonstd field as a format identifier. Existing drivers that use the 
nonstd field for other purposes wouldn't be able to implement the new API 
while keeping their existing API. This isn't a show stopper for drivers using 
the FB_NONSTD_HAM and FB_NONSTD_REV_PIX_IN_B bits, as they don't need to 
support any non-RGB format.

Existing drivers that support YUV formats through the nonstd field would have 
to select between the current and the new API, without being able to implement 
both.

- Use one of the reserved fields as a format identifier. Applications are 
supposed to zero the fb_var_screeninfo structure before passing it to the 
kernel, but experience showed that many applications forget to do so. Drivers 
would then be unable to find out whether applications request a particular 
format, or just forgot to initialize the field.

- Add a new FB_NONSTD_FORMAT bit to the nonstd field, and pass the format 
through a separate field. If an application sets the FB_NONSTD_FORMAT bit in 
the nonstd field, drivers will ignore the red, green, blue, transp and 
grayscale fields, and use the format identifier to configure the frame buffer 
format. The grayscale field would then be unneeded and could be reused as a 
format identifier. Alternatively, one of the reserved fields could be used for 
that purpose.

Existing drivers that support YUV formats through the nonstd field don't use 
the field's most significant bits. They could implement both their current API 
and the new API if the FB_NONSTD_FORMAT bit is stored in one of the nonstd 
MSBs.

- Other solutions are possible, such as adding new ioctls. Those solutions are 
more intrusive, and require larger changes to both userspace and kernelspace 
code.


The third solution has my preference. Comments and feedback will be 
appreciated. I will then work on a proof of concept and submit patches.


[1] http://en.wikipedia.org/wiki/Hold_And_Modify
[2] http://lwn.net/Articles/440192/
[3] http://www.fourcc.org/
[4] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L271
[5] http://en.wikipedia.org/wiki/Rec._601
[6] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L175

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [RFC] Standardize YUV support in the fbdev API
From: Felipe Contreras @ 2011-05-17 22:44 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel
In-Reply-To: <201105180007.21173.laurent.pinchart@ideasonboard.com>

On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> I need to implement support for a YUV frame buffer in an fbdev driver. As the
> fbdev API doesn't support this out of the box, I've spent a couple of days
> reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
> support to the API. I'd like to share my findings and thoughts, and hopefully
> receive some comments back.
>
> The terms 'format', 'pixel format', 'frame buffer format' and 'data format'
> will be used interchangeably in this e-mail. They all refer to the way pixels
> are stored in memory, including both the representation of a pixel as integer
> values and the layout of those integer values in memory.

This is a great proposal. It was about time!

> The third solution has my preference. Comments and feedback will be
> appreciated. I will then work on a proof of concept and submit patches.

I also would prefer the third solution. I don't think there's much
difference from the user-space point of view, and a new ioctl would be
cleaner. Also the v4l2 fourcc's should do.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [RFC] Standardize YUV support in the fbdev API
From: Andy Walls @ 2011-05-18  0:21 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel
In-Reply-To: <201105180007.21173.laurent.pinchart@ideasonboard.com>

On Wed, 2011-05-18 at 00:07 +0200, Laurent Pinchart wrote:
> Hi everybody,
> 
> I need to implement support for a YUV frame buffer in an fbdev driver. As the 
> fbdev API doesn't support this out of the box, I've spent a couple of days 
> reading fbdev (and KMS) code and thinking about how we could cleanly add YUV 
> support to the API. I'd like to share my findings and thoughts, and hopefully 
> receive some comments back.

I haven't looked at anything below, but I'll mention that the ivtv
driver presents an fbdev interface for the YUV output stream of the
CX23415.

It may be worth a look and asking Hans what are the short-comings.

-Andy


> The terms 'format', 'pixel format', 'frame buffer format' and 'data format' 
> will be used interchangeably in this e-mail. They all refer to the way pixels 
> are stored in memory, including both the representation of a pixel as integer 
> values and the layout of those integer values in memory.
> 
> 
> History and current situation
> -----------------------------
> 
> The fbdev API predates YUV frame buffers. In those old days developers only 
> cared (and probably thought) about RGB frame buffers, and they developed an 
> API that could express all kind of weird RGB layout in memory (such as R-
> GGGGGGGGGGG-BBBB for instance, I can't imagine hardware implementing that). 
> This resulted in individual bit field description for the red, green, blue and 
> alpha components:
> 
> struct fb_bitfield {
>         __u32 offset;                  /* beginning of bitfield        */
>         __u32 length;                  /* length of bitfield           */
>         __u32 msb_right;               /* != 0 : Most significant bit is */
>                                        /* right */
> };
> 
> Grayscale formats were pretty common, so a grayscale field tells color formats 
> (grayscale = 0) from grayscale formats (grayscale != 0).
> 
> People already realized that hardware developers were crazily inventive (the 
> word to remember here is crazily), and that non-standard formats would be 
> needed at some point. The fb_var_screeninfo ended up containing the following 
> format-related fields.
> 
> struct fb_var_screeninfo {
>         ...
>         __u32 bits_per_pixel;          /* guess what                   */
>         __u32 grayscale;               /* != 0 Graylevels instead of colors */
> 
>         struct fb_bitfield red;        /* bitfield in fb mem if true color, */
>         struct fb_bitfield green;      /* else only length is significant */
>         struct fb_bitfield blue;
>         struct fb_bitfield transp;     /* transparency                 */
> 
>         __u32 nonstd;                  /* != 0 Non standard pixel format */
>         ...
> };
> 
> Two bits have been specified for the nonstd field:
> 
> #define FB_NONSTD_HAM           1  /* Hold-And-Modify (HAM)        */
> #define FB_NONSTD_REV_PIX_IN_B  2  /* order of pixels in each byte is reversed 
> */
> 
> The FB_NONSTD_HAM bit is used by the video/amifb.c driver only to enable HAM 
> mode[1]. I very much doubt that any new hardware will implement HAM mode (and 
> I certainly hope none will).
> 
> The FB_NONSTD_REV_PIX_IN_B is used in video/fb_draw.h by the generic bitblit, 
> fillrect and copy area implementations, not directly by drivers.
> 
> A couple of drivers hardcode the nonstd field to 1 for some reason. Those are 
> video/arcfb.c (1bpp gray display), video/hecubafb.c (1bpp gray display) and 
> video/metronomefb.c (8bpp gray display).
> 
> The following drivers use nonstd for various other (and sometimes weird) 
> purposes:
> 
> video/arkfb.c
>         Used in 4bpp mode only, to control fb_setcolreg operation
> video/fsl-diu-fb.c
>         Set var->nonstd bits into var->sync (why?)
> video/pxafb.c
>         Encode frame buffer xpos and ypos in the nonstd field
> video/s3fb.c
>         Used in 4bpp mode only, to control fb_setcolreg operation
> video/vga16fb.c
>         When panning in non-8bpp, non-text mode, decrement xoffset
>         Do some other weird stuff when not 0
> video/i810/i810_main.c
>         Select direct color mode when set to 1 (truecolor otherwise)
> 
> Fast forward a couple of years, hardware provides support for YUV frame 
> buffers. Several drivers had to provide YUV format selection to applications, 
> without any standard API to do so. All of them used the nonstd field for that 
> purpose:
> 
> media/video/ivtv/
>         Enable YUV mode when set to 1
> video/pxafb.c
>         Encode pixel format in the nonstd field
> video/sh_mobile_lcdfb.c
>         If bpp = 12 and nonstd != 0, enable NV12 mode
>         If bpp = 16 or bpp = 24, ?
> video/omap/omapfb_main.c
>         Select direct color mode when set to 1 (depend on bpp otherwise)
>         Used as a pixel format identifier (YUV422, YUV420 or YUY422)
> video/omap2/omapfb/omapfb-main.c
>         Select direct color mode when set to 1 (depend on bpp otherwise)
>         Used as a pixel format identifier (YUV422 or YUY422)
> 
> Those drivers use the nonstd field in different, incompatible ways.
> 
> 
> Other related APIs
> ------------------
> 
> Beside the fbdev API, two other kernel/userspace APIs deal with video-related 
> modes and formats.
> 
> - Kernel Mode Setting (KMS)
> 
> The KMS API is similar in purpose to XRandR. Its main purpose is to provide a 
> kernel API to configure output video modes. As such, it doesn't care about 
> frame buffer formats, as they are irrelevant at the CRTC output.
> 
> In addition to handling video modes, the KMS API also provides support for 
> creating (and managing) frame buffer devices, as well as dumb scan-out 
> buffers. In-memory data format is relevant there, but KMS only handles width, 
> height, pitch, depth and bit-per-pixel information. The API doesn't care 
> whether the frame buffer or the dumb scan-out buffer contains RGB or YUV data.
> 
> An RFC[2] has recently been posted to the dri-devel mailing list to "add 
> overlays as first class KMS objects". The proposal includes explicit overlay 
> format support, and discussions have so far pushed towards reusing V4L format 
> codes for the KMS API.
> 
> - Video 4 Linux (V4L)
> 
> The V4L API version 2 (usually called V4L2) has since the beginning included 
> explicit support for data format, referred to as pixel formats.
> 
> Pixel formats are identified by a 32-bit number in the form of a four 
> characters code (4CC or FCC[3]). The list of FCCs defined by V4L2 is available 
> in [4].
> 
> A pixel format uniquely defines the layout of pixel data in memory, including 
> the format type (RGB, YUV, ...), number of bits per components, components 
> order and subsampling. It doesn't define the range of acceptable values for 
> pixel components (such as full-range YUV vs. BT.601[5]), which is carried 
> through a separate colorspace identifier[6].
> 
> 
> YUV support in the fbdev API
> ----------------------------
> 
> Experience with the V4L2 API, both for desktop and embedded devices, and the 
> KMS API, suggests that recent hardware tend to standardize on a relatively 
> small number of pixel formats that don't require bitfield-level descriptions. 
> A pixel format definition based on identifiers should thus fullfill the 
> hardware needs for the foreseeable future.
> 
> Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share data 
> and buffers between those subsystems, and the planned use of V4L2 FCCs in the 
> KMS overlay API, I believe using V4L2 FCCs to identify fbdev formats would be 
> a wise decision.
> 
> To select a frame buffer YUV format, the fb_var_screeninfo structure will need 
> to be extended with a format field. The fbdev API and ABI must not be broken, 
> which prevents us from changing the current structure layout and replacing the 
> existing format selection mechanism (through the red, green, blue and alpha 
> bitfields) completely.
> 
> Several solutions exist to introduce a format field in the fb_var_screeninfo 
> structure.
> 
> - Use the nonstd field as a format identifier. Existing drivers that use the 
> nonstd field for other purposes wouldn't be able to implement the new API 
> while keeping their existing API. This isn't a show stopper for drivers using 
> the FB_NONSTD_HAM and FB_NONSTD_REV_PIX_IN_B bits, as they don't need to 
> support any non-RGB format.
> 
> Existing drivers that support YUV formats through the nonstd field would have 
> to select between the current and the new API, without being able to implement 
> both.
> 
> - Use one of the reserved fields as a format identifier. Applications are 
> supposed to zero the fb_var_screeninfo structure before passing it to the 
> kernel, but experience showed that many applications forget to do so. Drivers 
> would then be unable to find out whether applications request a particular 
> format, or just forgot to initialize the field.
> 
> - Add a new FB_NONSTD_FORMAT bit to the nonstd field, and pass the format 
> through a separate field. If an application sets the FB_NONSTD_FORMAT bit in 
> the nonstd field, drivers will ignore the red, green, blue, transp and 
> grayscale fields, and use the format identifier to configure the frame buffer 
> format. The grayscale field would then be unneeded and could be reused as a 
> format identifier. Alternatively, one of the reserved fields could be used for 
> that purpose.
> 
> Existing drivers that support YUV formats through the nonstd field don't use 
> the field's most significant bits. They could implement both their current API 
> and the new API if the FB_NONSTD_FORMAT bit is stored in one of the nonstd 
> MSBs.
> 
> - Other solutions are possible, such as adding new ioctls. Those solutions are 
> more intrusive, and require larger changes to both userspace and kernelspace 
> code.
> 
> 
> The third solution has my preference. Comments and feedback will be 
> appreciated. I will then work on a proof of concept and submit patches.
> 
> 
> [1] http://en.wikipedia.org/wiki/Hold_And_Modify
> [2] http://lwn.net/Articles/440192/
> [3] http://www.fourcc.org/
> [4] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L271
> [5] http://en.wikipedia.org/wiki/Rec._601
> [6] http://lxr.linux.no/linux+v2.6.38/include/linux/videodev2.h#L175
> 



^ permalink raw reply

* Re: [RFC] Standardize YUV support in the fbdev API
From: Andy Walls @ 2011-05-18  1:09 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel
In-Reply-To: <1305678067.2419.1.camel@localhost>

Oops.  Nevermind, you already have looked at what ivtvfb does.

-Andy

^ permalink raw reply

* Re: [RFC] Standardize YUV support in the fbdev API
From: Hans Verkuil @ 2011-05-18  6:53 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Laurent Pinchart, linux-fbdev, linux-media, dri-devel
In-Reply-To: <BANLkTi=mRYkJL-R63K+pvZGvtetJo3oJaQ@mail.gmail.com>

On Wednesday, May 18, 2011 00:44:26 Felipe Contreras wrote:
> On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > I need to implement support for a YUV frame buffer in an fbdev driver. As the
> > fbdev API doesn't support this out of the box, I've spent a couple of days
> > reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
> > support to the API. I'd like to share my findings and thoughts, and hopefully
> > receive some comments back.
> >
> > The terms 'format', 'pixel format', 'frame buffer format' and 'data format'
> > will be used interchangeably in this e-mail. They all refer to the way pixels
> > are stored in memory, including both the representation of a pixel as integer
> > values and the layout of those integer values in memory.
> 
> This is a great proposal. It was about time!
> 
> > The third solution has my preference. Comments and feedback will be
> > appreciated. I will then work on a proof of concept and submit patches.
> 
> I also would prefer the third solution. I don't think there's much
> difference from the user-space point of view, and a new ioctl would be
> cleaner. Also the v4l2 fourcc's should do.

I agree with this.

We might want to take the opportunity to fix this section of the V4L2 Spec:

http://www.xs4all.nl/~hverkuil/spec/media.html#pixfmt-rgb

There are two tables, 2.6 and 2.7. But 2.6 is almost certainly wrong and should
be removed. I suspect many if not all V4L2 drivers are badly broken for
big-endian systems and report the wrong pixel formats.

Officially the pixel formats reflect the contents of the memory. But everything
is swapped on a big endian system, so you are supposed to report a different
pix format. I can't remember seeing any driver do that. Some have built-in
swapping, though, and turn that on if needed.

I really need to run some tests, but I've been telling myself this for years
now :-(

Regards,

	Hans

^ permalink raw reply

* Re: [PATCH 2/2] trivial: Changed the printk loglevel when not able
From: Catalin Marinas @ 2011-05-18 10:15 UTC (permalink / raw)
  To: anish singh; +Cc: linux, lethal, linux-fbdev, linux-kernel
In-Reply-To: <BANLkTi=ngJiMyVt4AMqcWUSUM2EVXtjRQQ@mail.gmail.com>

On Wed, 2011-05-18 at 04:19 +0100, anish singh wrote:
> On Tue, May 17, 2011 at 11:13 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>         On 17 May 2011 18:29, anish <anish198519851985@gmail.com> wrote:
>         > When not able to allocate memory we were using KERN_INFO as
>         > log level in printk so changed to KERN_ERR
>         >  Signed-off-by: anish kumar<anish198519851985@gmail.com>
>         
>         
>         Maybe KERN_WARN?
> Then shouldn't we change below case also?
> 467         ret = amba_request_regions(dev, NULL);
> 468         if (ret) {
> 469                 printk(KERN_ERR "CLCD: unable to reserve regs region\n");
> 470                 goto out;
> 
> If yes,then i will resend the patch for this also.

I think the register reserving is less likely to fail because of memory
allocations but more because of overlapping regions, in which case it
could be a programming error.

Allocating a big framebuffer is likely to fail in some memory constraint
systems but I don't consider this a kernel error. That's why I suggested
warning.

-- 
Catalin



^ permalink raw reply

* Re: [PATCH 2/2] trivial: Changed the printk loglevel when not able
From: Catalin Marinas @ 2011-05-18 11:01 UTC (permalink / raw)
  To: anish singh; +Cc: linux, lethal, linux-fbdev, linux-kernel
In-Reply-To: <BANLkTinZ6d1=58Y0SCL72Jh3F6R8yfqE-Q@mail.gmail.com>

On Wed, 2011-05-18 at 11:57 +0100, anish singh wrote:
> On Wed, May 18, 2011 at 3:45 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>         
>         On Wed, 2011-05-18 at 04:19 +0100, anish singh wrote:
>         > On Tue, May 17, 2011 at 11:13 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>         >         On 17 May 2011 18:29, anish <anish198519851985@gmail.com> wrote:
>         >         > When not able to allocate memory we were using KERN_INFO as
>         >         > log level in printk so changed to KERN_ERR
>         >         >  Signed-off-by: anish kumar<anish198519851985@gmail.com>
>         >
>         >
>         >         Maybe KERN_WARN?
>         > Then shouldn't we change below case also?
>         > 467         ret = amba_request_regions(dev, NULL);
>         > 468         if (ret) {
>         > 469                 printk(KERN_ERR "CLCD: unable to reserve regs region\n");
>         > 470                 goto out;
>         >
>         > If yes,then i will resend the patch for this also.
>         
>         
>         I think the register reserving is less likely to fail because of memory
>         allocations but more because of overlapping regions, in which case it
>         could be a programming error.
>         
>         Allocating a big framebuffer is likely to fail in some memory constraint
>         systems but I don't consider this a kernel error. That's why I suggested
>         warning.
>  
> Got it.So should i change both as KERN_WARN and resend?

No, just the one KERN_INFO one. Anyway, Russell King is the maintainer
of this driver so it's up to him to ack the patch (and don't forget to
cc him).

-- 
Catalin



^ permalink raw reply

* Re: [PATCH 2/2] trivial: Changed the printk loglevel when not able
From: Russell King - ARM Linux @ 2011-05-18 11:24 UTC (permalink / raw)
  To: anish singh; +Cc: Catalin Marinas, lethal, linux-fbdev, linux-kernel
In-Reply-To: <BANLkTinZ6d1=58Y0SCL72Jh3F6R8yfqE-Q@mail.gmail.com>

On Wed, May 18, 2011 at 04:27:47PM +0530, anish singh wrote:
> Got it.So should i change both as KERN_WARN and resend?

Much better would be to change it to pr_warn() or even
dev_warn(&dev->dev, ...).

^ permalink raw reply

* Re: [PATCH 2/2] trivial: Changed the printk loglevel when not able
From: Russell King - ARM Linux @ 2011-05-18 11:26 UTC (permalink / raw)
  To: anish singh; +Cc: Catalin Marinas, lethal, linux-fbdev, linux-kernel
In-Reply-To: <20110518112436.GH5913@n2100.arm.linux.org.uk>

On Wed, May 18, 2011 at 12:24:36PM +0100, Russell King - ARM Linux wrote:
> On Wed, May 18, 2011 at 04:27:47PM +0530, anish singh wrote:
> > Got it.So should i change both as KERN_WARN and resend?
> 
> Much better would be to change it to pr_warn() or even
> dev_warn(&dev->dev, ...).

Actually, make that pr_err() or dev_err().  Both printks in clcdfb_probe()
are errors and so should be reported at error severity - they cause things
to stop working and so they're not just a warning.

^ permalink raw reply

* [patch 1/2] trivial: amba-clcd.c preferred form of passing size of
From: anish @ 2011-05-18 18:19 UTC (permalink / raw)
  To: linux, lethal; +Cc: linux-fbdev, linux-kernel

The preferred form for passing a size of a struct is the following:
         p = kmalloc(sizeof(*p), ...);
  Please refer Documentation/Codingstyle chapter 14

 Signed-off-by: anish kumar <anish198519851985@gmail.com>

---
 drivers/video/amba-clcd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c
index 5fc983c..cb7ec86 100644
--- a/drivers/video/amba-clcd.c
+++ b/drivers/video/amba-clcd.c
@@ -549,7 +549,7 @@ static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id)
 		goto out;
 	}
 
-	fb = kzalloc(sizeof(struct clcd_fb), GFP_KERNEL);
+	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
 	if (!fb) {
 		printk(KERN_INFO "CLCD: could not allocate new clcd_fb struct\n");
 		ret = -ENOMEM;
-- 
1.7.0.4





^ permalink raw reply related

* [patch 2/2] trivial: amba-clcd.c changed the printk to pr_err
From: anish @ 2011-05-18 18:19 UTC (permalink / raw)
  To: linux-fbdev

As suggested by russell,printk in the fail scenario
 is an error and should be reported at error severity.They
 cause things to stop working and so they are not just a
 warning so changed to pr_err.

Signed-off-by: anish kumar <anish198519851985@gmail.com>

---
 drivers/video/amba-clcd.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c
index 5fc983c..9f27c71 100644
--- a/drivers/video/amba-clcd.c
+++ b/drivers/video/amba-clcd.c
@@ -545,13 +545,13 @@ static int clcdfb_probe(struct amba_device *dev, const struct amba_id *id)
 
 	ret = amba_request_regions(dev, NULL);
 	if (ret) {
-		printk(KERN_ERR "CLCD: unable to reserve regs region\n");
+		pr_err("CLCD: unable to reserve regs region\n");
 		goto out;
 	}
 
 	fb = kzalloc(sizeof(struct clcd_fb), GFP_KERNEL);
 	if (!fb) {
-		printk(KERN_INFO "CLCD: could not allocate new clcd_fb struct\n");
+		pr_err("CLCD: could not allocate new clcd_fb struct\n");
 		ret = -ENOMEM;
 		goto free_region;
 	}
-- 
1.7.0.4






^ permalink raw reply related

* Re: [REGRESSION] [2.6.39-rc3] Wrong resolution in framebuffer and X Window
From: Maciej Rutecki @ 2011-05-19 19:01 UTC (permalink / raw)
  To: linux-fbdev, chris
  Cc: linux-kernel, mbroemme, airlied, dri-devel, Rafael J. Wysocki
In-Reply-To: <201104211922.05779.maciej.rutecki@gmail.com>

On czwartek, 21 kwietnia 2011 o 19:22:05 Maciej Rutecki wrote:
> (add LKML)
> 
> On niedziela, 17 kwietnia 2011 o 18:04:04 Maciej Rutecki wrote:
> > Hi
> > 
> > Last known good: 2.6.38
> > Failing kernel: 2.6.39-rc3
> > Subsystem: Intel graphics driver.
> > 
> > Description:
> > PC should work with 1440x900 resolution. But console (and after) X Window
> > start work with 1024x768.
> > 
> > I attach dmesg and Xorg.0.log with drm.debug\x14 log_buf_len\x16M options:
> > http://unixy.pl/maciek/download/kernel/2.6.39-rc1/zlom/kms/
> > 
> > seems that driver cannot detect resolution higher than 1024x768.
> > 
> > Also I boot kernel replace "i915.modeset=1" with "nomodeset" option:
> > http://unixy.pl/maciek/download/kernel/2.6.39-rc1/zlom/nomodeset/
> > 
> > But then X Window fails to start and got message: "(EE) No devices
> > detected."
> > 
> > Config for 2.6.39-rc3:
> > http://unixy.pl/maciek/download/kernel/2.6.39-rc1/zlom/config-2.6.39-rc3
> > 
> > Best regards
> 
> Device:
> 00:02.0 VGA compatible controller: Intel Corporation 82G33/G31 Express
> Integrated Graphics Controller (rev 02)

Reported on:
https://bugzilla.kernel.org/show_bug.cgi?id4002

2 weeks ago. Problem STILL exists on 2.6.39. That any of the graphic 
developers might be interested in the problem?


-- 
Maciej Rutecki
http://www.maciek.unixy.pl

^ permalink raw reply

* Re: [REGRESSION] [2.6.39-rc3] Wrong resolution in framebuffer and
From: Mark Lord @ 2011-05-19 20:23 UTC (permalink / raw)
  To: maciej.rutecki
  Cc: linux-fbdev, chris, linux-kernel, mbroemme, airlied, dri-devel,
	Rafael J. Wysocki
In-Reply-To: <201105192101.50000.maciej.rutecki@gmail.com>

On 11-05-19 03:01 PM, Maciej Rutecki wrote:
> On czwartek, 21 kwietnia 2011 o 19:22:05 Maciej Rutecki wrote:
>> (add LKML)
>>
>> On niedziela, 17 kwietnia 2011 o 18:04:04 Maciej Rutecki wrote:
>>> Hi
>>>
>>> Last known good: 2.6.38
>>> Failing kernel: 2.6.39-rc3
>>> Subsystem: Intel graphics driver.
>>>
>>> Description:
>>> PC should work with 1440x900 resolution. But console (and after) X Window
>>> start work with 1024x768.
>>>
>>> I attach dmesg and Xorg.0.log with drm.debug\x14 log_buf_len\x16M options:
>>> http://unixy.pl/maciek/download/kernel/2.6.39-rc1/zlom/kms/
>>>
>>> seems that driver cannot detect resolution higher than 1024x768.
>>>
>>> Also I boot kernel replace "i915.modeset=1" with "nomodeset" option:
>>> http://unixy.pl/maciek/download/kernel/2.6.39-rc1/zlom/nomodeset/
>>>
>>> But then X Window fails to start and got message: "(EE) No devices
>>> detected."
>>>
>>> Config for 2.6.39-rc3:
>>> http://unixy.pl/maciek/download/kernel/2.6.39-rc1/zlom/config-2.6.39-rc3
>>>
>>> Best regards
>>
>> Device:
>> 00:02.0 VGA compatible controller: Intel Corporation 82G33/G31 Express
>> Integrated Graphics Controller (rev 02)
>
> Reported on:
> https://bugzilla.kernel.org/show_bug.cgi?id4002
>
> 2 weeks ago. Problem STILL exists on 2.6.39. That any of the graphic
> developers might be interested in the problem?

I had a problem similar to this, beginning with 2.6.37 (2.6.36 was okay).
It turned out to be that newer kernels were detecting the LVDS (LCD panel)
interface from the chipset, event though nothing was attached to that interface.
So GNOME would come up at 1024x768 (max the LVDS supported, I guess).

I initially worked around it by going into the GNOME -> Preferences -> Monitors,
and disabling the LVDS device there, in favour of the 1920x1200 "VGA" device I use.

Later, I hacked my kernel to just ignore the LVDS completely:


--- linux/drivers/gpu/drm/i915/intel_lvds.c.orig	2011-05-09 22:33:54.000000000 -0400
+++ linux/drivers/gpu/drm/i915/intel_lvds.c	2011-05-17 23:00:40.829773525 -0400
@@ -841,6 +841,7 @@
  */
 bool intel_lvds_init(struct drm_device *dev)
 {
+    if (0) {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_lvds *intel_lvds;
 	struct intel_encoder *intel_encoder;
@@ -1041,5 +1042,6 @@
 	drm_encoder_cleanup(encoder);
 	kfree(intel_lvds);
 	kfree(intel_connector);
+    }
 	return false;
 }

^ permalink raw reply

* linux-next: build failure after merge of the final tree
From: Stephen Rothwell @ 2011-05-20  6:32 UTC (permalink / raw)
  To: Linus; +Cc: linux-next, linux-kernel, Paul Mundt, linux-fbdev

Hi all,

After merging the final tree, today's linux-next build (powerpc
allyesconfig) failed like this:

drivers/video/udlfb.c: In function 'dlfb_compress_hline':
drivers/video/udlfb.c:421: error: implicit declaration of function 'prefetch_range'

Presumably caused by commit e66eed651fd1 ("list: remove prefetching from
regular list iterators").  We need to include preempt.h explictly, now.

I have included this patch for today:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 20 May 2011 16:29:01 +1000
Subject: [PATCH] udlfb: include prefetch.h explicitly

Commit e66eed651fd1 ("list: remove prefetching from
regular list iterators") removed the include of prefetch.h from list.h,
so we need to include it explicitly, now.

fixes this build error on powerpc:

drivers/video/udlfb.c: In function 'dlfb_compress_hline':
drivers/video/udlfb.c:421: error: implicit declaration of function 'prefetch_range'

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/video/udlfb.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c
index 68041d9..4c299cc 100644
--- a/drivers/video/udlfb.c
+++ b/drivers/video/udlfb.c
@@ -28,6 +28,7 @@
 #include <linux/vmalloc.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
+#include <linux/prefetch.h>
 #include <video/udlfb.h>
 #include "edid.h"
 
-- 
1.7.5.1

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

^ permalink raw reply related

* Equity Business Investment
From: PROF.MAURICE IWU @ 2011-05-20 14:55 UTC (permalink / raw)
  To: linux-fbdev


Equity Business Investment
From Prof. Maurice Iwu 

Dear Friend,

I am Prof. Maurice M. Iwu, Former Chairman Independent Electoral Commission 
(INEC).Currently, I am on compulsory terminal leave presently in UK , 
leading up to my final retirement from public service.

I am seeking your partnership in a mutual benefit project involving 
investing into your company and secondly on properties and to be precise Estate Development or any other profitable venture of your great idea for our mutual benefit of Equity Market Investment.,

Sincerely,
Prof. Maurice M.Iwu.
{INEC.}

^ permalink raw reply

* Re: [RFC] Standardize YUV support in the fbdev API
From: Florian Tobias Schandinat @ 2011-05-20 22:33 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, linux-media, dri-devel
In-Reply-To: <201105180007.21173.laurent.pinchart@ideasonboard.com>

Hi Laurent,

On 05/17/2011 10:07 PM, Laurent Pinchart wrote:
> Hi everybody,
>
> I need to implement support for a YUV frame buffer in an fbdev driver. As the
> fbdev API doesn't support this out of the box, I've spent a couple of days
> reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
> support to the API. I'd like to share my findings and thoughts, and hopefully
> receive some comments back.

Thanks. I think you did already a good job, hope we can get it done this time.

> Given the overlap between the KMS, V4L2 and fbdev APIs, the need to share data
> and buffers between those subsystems, and the planned use of V4L2 FCCs in the
> KMS overlay API, I believe using V4L2 FCCs to identify fbdev formats would be
> a wise decision.

I agree.

> To select a frame buffer YUV format, the fb_var_screeninfo structure will need
> to be extended with a format field. The fbdev API and ABI must not be broken,
> which prevents us from changing the current structure layout and replacing the
> existing format selection mechanism (through the red, green, blue and alpha
> bitfields) completely.

I agree.

> - Other solutions are possible, such as adding new ioctls. Those solutions are
> more intrusive, and require larger changes to both userspace and kernelspace
> code.

I'm against (ab)using the nonstd field (probably the only sane thing we can do 
with it is declare it non-standard which interpretation is completely dependent 
on the specific driver) or requiring previously unused fields to have a special 
value so I'd like to suggest a different method:

I remembered an earlier discussion:
[ http://marc.info/?l=linux-fbdev&m\x129896686208130&w=2 ]

On 03/01/2011 08:07 AM, Geert Uytterhoeven wrote:
 > On Tue, Mar 1, 2011 at 04:13, Damian<dhobsong@igel.co.jp>  wrote:
 >> On 2011/02/24 15:05, Geert Uytterhoeven wrote:
 >>> For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new
 >>> FB_VISUAL_*
 >>> type instead, which indicates the fb_var_screeninfo.{red,green,blue}
 >>> fields are
 >>> YCbCr instead of RGB.
 >>> Depending on the frame buffer organization, you also need new
 >>> FB_TYPE_*/FB_AUX_*
 >>> types.
 >>
 >> I just wanted to clarify here.  Is your comment about these new flags in
 >> specific reference to this patch or to Magnus' "going forward" comment?  It
 >
 > About new flags.
 >
 >> seems like the beginnings of a method to standardize YCbCr support in fbdev
 >> across all platforms.
 >> Also, do I understand correctly that FB_VISUAL_ would specify the colorspace
 >
 > FB_VISUAL_* specifies how pixel values (which may be tuples) are mapped to
 > colors on the screen, so to me it looks like the sensible way to set up YCbCr.
 >
 >> (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. planar,
 >> semiplanar, interleaved, etc)?  I'm not really sure what you are referring
 >> to with the FB_AUX_* however.
 >
 > Yep, FB_TYPE_* specifies how pixel values/tuples are laid out in frame buffer
 > memory.
 >
 > FB_AUX_* is only used if a specific value of FB_TYPE_* needs an additional
 > parameter (e.g. the interleave value for interleaved bitplanes).

Adding new standard values for these fb_fix_screeninfo fields would solve the 
issue for framebuffers which only support a single format. If you have the need 
to switch I guess it would be a good idea to add a new flag to the vmode 
bitfield in fb_var_screeninfo which looks like a general purpose modifier for 
the videomode. You could than reuse any RGB-specific field you like to pass more 
information.
Maybe we should also use this chance to declare one of the fix_screeninfo 
reserved fields to be used for capability flags or an API version as we can 
assume that those are 0 (at least in sane drivers).


Good luck,

Florian Tobias Schandinat

^ permalink raw reply

* [PATCH 1/3] fbdev/amifb: Correct check for video memory size
From: Geert Uytterhoeven @ 2011-05-21 19:42 UTC (permalink / raw)
  To: Paul Mundt; +Cc: linux-fbdev, linux-m68k, Geert Uytterhoeven

The check should compare the available memory size with the highest allocation
value (VIDEOMEMSIZE_*_2M), not with the lowest value (VIDEOMEMSIZE_*_1M).

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/video/amifb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/amifb.c b/drivers/video/amifb.c
index e5d6b56..603f84f 100644
--- a/drivers/video/amifb.c
+++ b/drivers/video/amifb.c
@@ -2295,7 +2295,7 @@ default_chipset:
 			    defmode = amiga_vblank = 50 ? DEFMODE_PAL
 							 : DEFMODE_NTSC;
 			if (amiga_chip_avail()-CHIPRAM_SAFETY_LIMIT >
-			    VIDEOMEMSIZE_ECS_1M)
+			    VIDEOMEMSIZE_ECS_2M)
 				fb_info.fix.smem_len = VIDEOMEMSIZE_ECS_2M;
 			else
 				fb_info.fix.smem_len = VIDEOMEMSIZE_ECS_1M;
@@ -2312,7 +2312,7 @@ default_chipset:
 			maxfmode = TAG_FMODE_4;
 			defmode = DEFMODE_AGA;
 			if (amiga_chip_avail()-CHIPRAM_SAFETY_LIMIT >
-			    VIDEOMEMSIZE_AGA_1M)
+			    VIDEOMEMSIZE_AGA_2M)
 				fb_info.fix.smem_len = VIDEOMEMSIZE_AGA_2M;
 			else
 				fb_info.fix.smem_len = VIDEOMEMSIZE_AGA_1M;
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/3] fbdev/amifb: Do not call panic() if there's not enough Chip RAM
From: Geert Uytterhoeven @ 2011-05-21 19:42 UTC (permalink / raw)
  To: Paul Mundt; +Cc: linux-fbdev, linux-m68k, Geert Uytterhoeven
In-Reply-To: <1306006976-32117-1-git-send-email-geert@linux-m68k.org>

Fail gracefully instead.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/video/amifb.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/video/amifb.c b/drivers/video/amifb.c
index 603f84f..1b0185c 100644
--- a/drivers/video/amifb.c
+++ b/drivers/video/amifb.c
@@ -2230,8 +2230,10 @@ static inline u_long __init chipalloc(u_long size)
 {
 	size += PAGE_SIZE-1;
 	if (!(unaligned_chipptr = (u_long)amiga_chip_alloc(size,
-							   "amifb [RAM]")))
-		panic("No Chip RAM for frame buffer");
+							   "amifb [RAM]"))) {
+		pr_err("amifb: No Chip RAM for frame buffer");
+		return 0;
+	}
 	memset((void *)unaligned_chipptr, 0, size);
 	return PAGE_ALIGN(unaligned_chipptr);
 }
@@ -2385,6 +2387,10 @@ default_chipset:
 	                    DUMMYSPRITEMEMSIZE+
 	                    COPINITSIZE+
 	                    4*COPLISTSIZE);
+	if (!chipptr) {
+		err = -ENOMEM;
+		goto amifb_error;
+	}
 
 	assignchunk(videomemory, u_long, chipptr, fb_info.fix.smem_len);
 	assignchunk(spritememory, u_long, chipptr, SPRITEMEMSIZE);
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 3/3] fbdev/amifb: Remove superfluous alignment of frame buffer memory
From: Geert Uytterhoeven @ 2011-05-21 19:42 UTC (permalink / raw)
  To: Paul Mundt; +Cc: linux-fbdev, linux-m68k, Geert Uytterhoeven
In-Reply-To: <1306006976-32117-1-git-send-email-geert@linux-m68k.org>

amiga_chip_alloc() already aligns to the PAGE_SIZE

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/video/amifb.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/video/amifb.c b/drivers/video/amifb.c
index 1b0185c..5ea6596 100644
--- a/drivers/video/amifb.c
+++ b/drivers/video/amifb.c
@@ -2224,24 +2224,23 @@ static int amifb_ioctl(struct fb_info *info,
 	 * Allocate, Clear and Align a Block of Chip Memory
 	 */
 
-static u_long unaligned_chipptr = 0;
+static void *aligned_chipptr;
 
 static inline u_long __init chipalloc(u_long size)
 {
-	size += PAGE_SIZE-1;
-	if (!(unaligned_chipptr = (u_long)amiga_chip_alloc(size,
-							   "amifb [RAM]"))) {
+	aligned_chipptr = amiga_chip_alloc(size, "amifb [RAM]");
+	if (!aligned_chipptr) {
 		pr_err("amifb: No Chip RAM for frame buffer");
 		return 0;
 	}
-	memset((void *)unaligned_chipptr, 0, size);
-	return PAGE_ALIGN(unaligned_chipptr);
+	memset(aligned_chipptr, 0, size);
+	return (u_long)aligned_chipptr;
 }
 
 static inline void chipfree(void)
 {
-	if (unaligned_chipptr)
-		amiga_chip_free((void *)unaligned_chipptr);
+	if (aligned_chipptr)
+		amiga_chip_free(aligned_chipptr);
 }
 
 
-- 
1.7.0.4


^ permalink raw reply related

* [GIT PULL] viafb updates
From: Florian Tobias Schandinat @ 2011-05-22 18:59 UTC (permalink / raw)
  To: torvalds; +Cc: Paul Mundt, LKML, linux-fbdev

Hi Linus,

as Paul Mundt, the fbdev maintainer, has been inactive for several weeks now, I 
ask you to pull these viafb updates for 2.6.40 from

   git://github.com/schandinat/linux-2.6.git viafb-next

They contain major cleanups, a better PLL/clock management and some notable 
changes for users like write-combining speeding userspace apps up to a factor 
around 6 (on newer hardware) and other things making viafb more likely to work. 
Also some important changes to make it work on OLPC XO 1.5 (again).
This stuff has been in linux-next for some time.


Thanks,

Florian Tobias Schandinat


Daniel Drake (1):
       viafb: Automatic OLPC XO-1.5 configuration

Florian Tobias Schandinat (26):
       viafb: move initialization code
       viafb: no need to write CRTC values twice
       viafb: kill crt_setting_information
       viafb: allow some pll calculations
       viafb: remove unused max_hres/vres
       viafb: call viafb_get_clk_value only in viafb_set_vclock
       viafb: prepare for PLL separation
       viafb: add clock source selection and PLL power management support
       viafb: add primary/secondary clock on/off switches
       viafb: split clock and PLL code to an extra file
       viafb: add VIA slapping capability
       viafb: add engine clock support
       viafb: gather common good, old VGA initialization in one place
       viafb: some small cleanup for global variables
       viafb: replace custom return values
       viafb: delete clock and PLL initialization
       viafb: fix OLPC DCON refresh rate
       viafb: fix OLPC XO 1.5 device connection
       viafb: reduce OLPC refresh a bit
       viafb: add X server compatibility mode
       Merge branch 'viafb-olpc' into viafb-next
       Merge branch 'viafb-cleanup' into viafb-next
       Merge branch 'viafb-pll' into viafb-next
       viafb: use write combining for video ram
       viafb: try to map less memory in case of failure
       viafb: remove unused CEA mode

  drivers/video/Kconfig         |   11 +
  drivers/video/via/Makefile    |    2 +-
  drivers/video/via/chip.h      |    6 -
  drivers/video/via/dvi.c       |  160 +----------
  drivers/video/via/dvi.h       |    2 +-
  drivers/video/via/global.c    |    4 -
  drivers/video/via/global.h    |    2 -
  drivers/video/via/hw.c        |  630 ++++++++++++++---------------------------
  drivers/video/via/hw.h        |   15 +-
  drivers/video/via/lcd.c       |   23 +-
  drivers/video/via/lcd.h       |    2 +-
  drivers/video/via/share.h     |   17 +-
  drivers/video/via/via-core.c  |    9 +-
  drivers/video/via/via_clock.c |  349 +++++++++++++++++++++++
  drivers/video/via/via_clock.h |   76 +++++
  drivers/video/via/viafbdev.c  |   62 +++--
  drivers/video/via/viafbdev.h  |    4 -
  drivers/video/via/viamode.c   |   46 +---
  drivers/video/via/viamode.h   |    9 -
  19 files changed, 723 insertions(+), 706 deletions(-)
  create mode 100644 drivers/video/via/via_clock.c
  create mode 100644 drivers/video/via/via_clock.h

^ permalink raw reply

* Re: [patch 1/2] trivial: amba-clcd.c preferred form of passing size of structure
From: Paul Mundt @ 2011-05-23  7:13 UTC (permalink / raw)
  To: anish; +Cc: linux, linux-fbdev, linux-kernel
In-Reply-To: <1305742026.8771.26.camel@anish-desktop>

On Wed, May 18, 2011 at 11:37:06PM +0530, anish wrote:
> The preferred form for passing a size of a struct is the following:
>          p = kmalloc(sizeof(*p), ...);
>   Please refer Documentation/Codingstyle chapter 14
> 
>  Signed-off-by: anish kumar <anish198519851985@gmail.com>
> 
That's pretty subjective, and ultimately depends on the author. I
personally prefer to have it written out, but then I don't randomly
change my structure types around either, so the "bug" alluded to isn't
really an issue. I'm not sure I buy the impaired readability argument,
either..

^ permalink raw reply

* [GIT PULL] OMAP DSS updates for 2.6.40
From: Tomi Valkeinen @ 2011-05-23 11:43 UTC (permalink / raw)
  To: Paul Mundt; +Cc: lo-ml, lfbdev-ml

Hi Paul,

Here are the OMAP Display Subsystem updates for 2.6.40.

Most of the patches are OMAP4 related, either fixing the old code to
allow us to implement new OMAP4 features, or implementing those new
features. Other bigger changes are ULPS power management feature and
moving the DSS header file from arch/arm/plat-omap/include to
include/video.

 Tomi

The following changes since commit 693d92a1bbc9e42681c42ed190bd42b636ca876f:

  Linux 2.6.39-rc7 (2011-05-09 19:33:54 -0700)

are available in the git repository at:
  git://gitorious.org/linux-omap-dss2/linux.git for-paul

Amber Jain (5):
      OMAP: DSS2: Add new color formats for OMAP4
      OMAP: DSS2: Ensure non-zero FIR values are configured
      OMAP: DSS2: Use for loop where ever possible in SR(), RR()
      OMAP: DSS2: Add new registers for NV12 support
      OMAP: DSS2: Add support for NV12 format

Archit Taneja (23):
      OMAP: DSS2: Fix: Return correct lcd clock source for OMAP2/3
      OMAP4: DSS2: Register configuration changes for DSI
      OMAP: DSS2: DSI: Introduce sync_vc functions
      OMAP2PLUS: DSS2: Change enum "dss_clk_source" to "omap_dss_clk_source"
      OMAP2PLUS: DSS2: Add clock sources to dss device clock configuration
      OMAP: DSS2: HDMI: Use dss_device clock configuration for HDMI PLL parameters
      OMAP2PLUS: DSS2: Remove hack config "CONFIG_OMAP2_DSS_USE_DSI_PLL"
      OMAP2PLUS: DSS2: Clean up omap_display_init()
      OMAP: DSS2: DSI: enable scp clock in dsi_dump_regs()
      OMAP: DSS2: Clean up DISPC overlay register definitions
      OMAP: DSS2: Clean up DISPC overlay manager register definitions
      OMAP: DSS2: Remove usage of struct dispc_reg
      OMAP: DSS2: DSI: Add extra omap_dss_device argument in functions exported by dsi
      OMAP: DSS2: Remove omap_dss_device argument from dsi_pll_init()
      OMAP: DSS2: Pass platform_device as an argument in dsi functions
      OMAP: DSS2: DSI: Use platform_device pointer to get dsi data
      OMAP: DSS2: DSI: Pass pointer to struct to packet_sent_handler isrs
      OMAP4: DSS2: DSI: Changes for DSI2 on OMAP4
      OMAP: DSS2: Taal: Use device name in backlight_device_register
      OMAP: DSS2: DSI: Remove DISPC pixel clock related info in dsi_dump_clocks()
      OMAP: DSS2: DSI: Use system workqueue for delayed work instead of a private workqueue
      OMAP: DSS2: DSI: Get number of DSI data lanes using DSI_GNQ register
      OMAP: DSS2: DSI: Get line buffer size from DSI_GNQ register

Enric Balletbo i Serra (2):
      OMAP: DSS2: Support for Seiko 70WVW1TZ3
      OMAP: DSS2: Support for Powertip PH480272T

Jani Nikula (2):
      OMAP: DSS2: Add method for querying display dimensions from DSS drivers
      OMAP: DSS2: OMAPFB: Remove implicit display update on unblank

Mike Frysinger (1):
      OMAP2: avoid descending into disabled framebuffer dirs

Niels de Vos (1):
      OMAP: DSS2: OMAPFB: make DBG() more resistant in if-else constructions

Ricardo Neri (5):
      OMAP4: DSS2: Create a DSS features structure for OMAP4430 ES1.0
      OMAP4: DSS2: HDMI: Add DSS feature for CTS calculation
      OMAP4: DSS2: HDMI: Add enums and structures for audio
      OMAP4: DSS2: HDMI: Add functionality for audio configuration
      OMAP4: DSS2: HDMI: Implement ASoC Codec driver for HDMI audio

Tomi Valkeinen (48):
      OMAP: DSS2: Move display.h to include/video/
      OMAP: DSS2: Move panel-generic-dpi.h to include/video/
      OMAP: DSS2: Move nokia-dsi-panel.h to include/video/
      OMAP: DSS2: DSI: fix use_sys_clk & highfreq
      OMAP: DSS2: DSI: fix dsi_dump_clocks()
      OMAP: DSS2: DSI: Fix DSI PLL power bug
      OMAP: DSS2: fix panel Kconfig dependencies
      OMAP: DSS2: move dss device clock configuration
      OMAP: DSS2: remove non-working msleep(40) workaround
      OMAP: DSS2: make 50ms bug-fix sleep optional
      OMAP: DSS2: VENC: make 20ms venc bug-fix sleep optional
      OMAP: DSS2: VENC: Remove sleeps at venc enable/disable
      OMAP: DSS2: OMAPFB: Handle errors when initializing panel
      OMAP: DSS2: VENC: Add missing start/stop_device calls
      OMAP: DSS2: make omap_dss_(un)register_device static
      OMAP: DSS2: use __exit for selected panel drivers
      OMAP: DSS2: Convert simple/strict_strto* to kstrto*
      OMAP: DSS2: improve clock debugfs output
      OMAP: DSS2: DSI: Add lane override functions
      OMAP: DSS2: DSI: Remove CIO LDO status check
      OMAP: DSS2: DSI: implement ULPS enter and exit
      OMAP: DSS2: DSI: add option to leave DSI lanes powered on
      OMAP: DSS2: DSI: rename complexio related functions
      OMAP: DSS2: Add FEAT_DSI_REVERSE_TXCLKESC
      OMAP: DSS2: DSI: fix _dsi_print_reset_status
      OMAP: DSS2: DSI: implement enable/disable SCP clk
      OMAP: DSS2: DSI: fix CIO init and uninit
      OMAP: DSS2: DSI: wait for TXCLKESC domain to come out of reset
      OMAP: DSS2: DSI: add parameter to enter ulps on disable
      OMAP: DSS2: DSI: Add DSI pad muxing support
      OMAP: DSS2: DSI: ensure VDDS_DSI is disabled on exit
      OMAP: DSS2: Taal: Implement configurable ESD interval
      OMAP: DSS2: Taal: Clean up ESD queueing
      OMAP: DSS2: Taal: Add sysfs file for ESD interval
      OMAP: DSS2: Taal: Separate panel reset
      OMAP: DSS2: Taal: Rename esd_wq to workqueue
      OMAP: DSS2: Taal: Implement ULPS functionality
      OMAP: DSS2: DSI: Add OMAP4 CIO irqs
      OMAP: DSS2: FEATURES: Add missing consts
      OMAP: DSS2: Remove DSS_IRQSTATUS
      OMAP: DSS2: Add missing dummy functions
      OMAPFB: fix wrong clock aliases and device name
      OMAP: DSS2: RFBI: add rfbi_bus_lock
      OMAP: DSS2: RFBI: clock enable/disable changes
      OMAP: DSS2: RFBI: add omap_rfbi_configure
      OMAP: DSS2: RFBI: cleanup
      OMAP: DSS2: OMAPFB: remove dead code
      OMAP: DSS2: OMAPFB: Reduce stack usage

Vikram Pandita (1):
      OMAP: DSS2: DSI: enable interface for omap4

 arch/arm/mach-omap2/board-3430sdp.c                |    4 +-
 arch/arm/mach-omap2/board-4430sdp.c                |   11 +-
 arch/arm/mach-omap2/board-am3517evm.c              |    4 +-
 arch/arm/mach-omap2/board-cm-t35.c                 |    4 +-
 arch/arm/mach-omap2/board-devkit8000.c             |    4 +-
 arch/arm/mach-omap2/board-igep0020.c               |    4 +-
 arch/arm/mach-omap2/board-omap3beagle.c            |    4 +-
 arch/arm/mach-omap2/board-omap3evm.c               |    4 +-
 arch/arm/mach-omap2/board-omap3pandora.c           |    2 +-
 arch/arm/mach-omap2/board-omap3stalker.c           |    4 +-
 arch/arm/mach-omap2/board-omap4panda.c             |    4 +-
 arch/arm/mach-omap2/board-overo.c                  |    4 +-
 arch/arm/mach-omap2/board-rx51-video.c             |    2 +-
 arch/arm/mach-omap2/board-zoom-display.c           |    2 +-
 arch/arm/mach-omap2/display.c                      |   77 +-
 arch/arm/mach-omap2/include/mach/board-zoom.h      |    2 +-
 drivers/media/video/omap/omap_vout.c               |    2 +-
 drivers/media/video/omap/omap_voutdef.h            |    2 +-
 drivers/video/omap/dispc.c                         |    4 +-
 drivers/video/omap/omapfb_main.c                   |    2 +-
 drivers/video/omap/rfbi.c                          |    2 +-
 drivers/video/omap2/Makefile                       |    4 +-
 drivers/video/omap2/displays/Kconfig               |    9 +-
 drivers/video/omap2/displays/panel-acx565akm.c     |    2 +-
 drivers/video/omap2/displays/panel-generic-dpi.c   |   57 +-
 .../omap2/displays/panel-lgphilips-lb035q02.c      |    2 +-
 .../omap2/displays/panel-nec-nl8048hl11-01b.c      |    2 +-
 .../video/omap2/displays/panel-sharp-ls037v7dw01.c |    6 +-
 drivers/video/omap2/displays/panel-taal.c          |  536 ++++-
 .../video/omap2/displays/panel-tpo-td043mtea1.c    |   10 +-
 drivers/video/omap2/dss/Kconfig                    |   33 +-
 drivers/video/omap2/dss/core.c                     |   15 +-
 drivers/video/omap2/dss/dispc.c                    | 1552 +++++++------
 drivers/video/omap2/dss/dispc.h                    |  691 ++++++
 drivers/video/omap2/dss/display.c                  |   46 +-
 drivers/video/omap2/dss/dpi.c                      |  113 +-
 drivers/video/omap2/dss/dsi.c                      | 2367 +++++++++++++-------
 drivers/video/omap2/dss/dss.c                      |  118 +-
 drivers/video/omap2/dss/dss.h                      |   98 +-
 drivers/video/omap2/dss/dss_features.c             |  105 +-
 drivers/video/omap2/dss/dss_features.h             |   39 +-
 drivers/video/omap2/dss/hdmi.c                     |  461 ++++-
 drivers/video/omap2/dss/hdmi.h                     |  222 ++-
 drivers/video/omap2/dss/hdmi_omap4_panel.c         |    2 +-
 drivers/video/omap2/dss/manager.c                  |   14 +-
 drivers/video/omap2/dss/overlay.c                  |   43 +-
 drivers/video/omap2/dss/rfbi.c                     |  176 +--
 drivers/video/omap2/dss/sdi.c                      |    2 +-
 drivers/video/omap2/dss/venc.c                     |   23 +-
 drivers/video/omap2/omapfb/omapfb-ioctl.c          |   14 +-
 drivers/video/omap2/omapfb/omapfb-main.c           |  231 ++-
 drivers/video/omap2/omapfb/omapfb-sysfs.c          |   23 +-
 drivers/video/omap2/omapfb/omapfb.h                |    8 +-
 .../video/omap-panel-generic-dpi.h                 |    8 +-
 .../video/omap-panel-nokia-dsi.h                   |   14 +-
 .../plat/display.h => include/video/omapdss.h      |  114 +-
 56 files changed, 5154 insertions(+), 2154 deletions(-)
 create mode 100644 drivers/video/omap2/dss/dispc.h
 rename arch/arm/plat-omap/include/plat/panel-generic-dpi.h => include/video/omap-panel-generic-dpi.h (86%)
 rename arch/arm/plat-omap/include/plat/nokia-dsi-panel.h => include/video/omap-panel-nokia-dsi.h (65%)
 rename arch/arm/plat-omap/include/plat/display.h => include/video/omapdss.h (85%)



^ permalink raw reply

* RE: [RFC] Standardize YUV support in the fbdev API
From: Marek Szyprowski @ 2011-05-23 11:55 UTC (permalink / raw)
  To: 'Hans Verkuil', 'Felipe Contreras'
  Cc: 'Laurent Pinchart', linux-fbdev, linux-media, dri-devel
In-Reply-To: <201105180853.33850.hverkuil@xs4all.nl>

Hello,

On Wednesday, May 18, 2011 8:54 AM Hans Verkuil wrote:

> On Wednesday, May 18, 2011 00:44:26 Felipe Contreras wrote:
> > On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> > > I need to implement support for a YUV frame buffer in an fbdev driver.
> As the
> > > fbdev API doesn't support this out of the box, I've spent a couple of
> days
> > > reading fbdev (and KMS) code and thinking about how we could cleanly
> add YUV
> > > support to the API. I'd like to share my findings and thoughts, and
> hopefully
> > > receive some comments back.
> > >
> > > The terms 'format', 'pixel format', 'frame buffer format' and 'data
> format'
> > > will be used interchangeably in this e-mail. They all refer to the way
> pixels
> > > are stored in memory, including both the representation of a pixel as
> integer
> > > values and the layout of those integer values in memory.
> >
> > This is a great proposal. It was about time!
> >
> > > The third solution has my preference. Comments and feedback will be
> > > appreciated. I will then work on a proof of concept and submit patches.
> >
> > I also would prefer the third solution. I don't think there's much
> > difference from the user-space point of view, and a new ioctl would be
> > cleaner. Also the v4l2 fourcc's should do.
> 
> I agree with this.
> 
> We might want to take the opportunity to fix this section of the V4L2 Spec:
> 
> http://www.xs4all.nl/~hverkuil/spec/media.html#pixfmt-rgb
> 
> There are two tables, 2.6 and 2.7. But 2.6 is almost certainly wrong and
> should be removed.

That's definitely true. I was confused at the beginning when I saw 2
different tables describing the same pixel formats.

 I suspect many if not all V4L2 drivers are badly broken for
> big-endian systems and report the wrong pixel formats.
> 
> Officially the pixel formats reflect the contents of the memory. But
> everything is swapped on a big endian system, so you are supposed to 
> report a different pix format.

I always thought that pix_format describes the layout of video data in
memory on byte level, which is exactly the same on both little- and big-
endian systems. You can notice swapped data only if you access memory
by units larger than byte, like 16bit or 32bit integers. BTW - I would
really like to avoid little- and big- endian flame, but your statement
about 'everything is swapped on a big endian system' is completely
wrong. It is rather the characteristic of little-endian system not big
endian one if you display the content of the same memory first using
byte access and then using word/long access.

> I can't remember seeing any driver do that. Some have built-in swapping,
> though, and turn that on if needed.

The drivers shouldn't do ANY byte swapping at all. Only tools that
extract pixel data with some 'accelerated' methods (like 32bit integer
casting and bit-level shifting) should be aware of endianess.

> I really need to run some tests, but I've been telling myself this for
> years now :-(

I've checked the BTTV board in my PowerMac/G4 and the display was
correct with xawtv. It is just a matter of selecting correct pix format
basing on the information returned by xsever. 

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center



^ permalink raw reply

* Re: [RFC] Standardize YUV support in the fbdev API
From: Hans Verkuil @ 2011-05-23 12:09 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: 'Felipe Contreras', 'Laurent Pinchart',
	linux-fbdev, linux-media, dri-devel
In-Reply-To: <000601cc1940$4b2e2b20$e18a8160$%szyprowski@samsung.com>

On Monday, May 23, 2011 13:55:21 Marek Szyprowski wrote:
> Hello,
> 
> On Wednesday, May 18, 2011 8:54 AM Hans Verkuil wrote:
> 
> > On Wednesday, May 18, 2011 00:44:26 Felipe Contreras wrote:
> > > On Wed, May 18, 2011 at 1:07 AM, Laurent Pinchart
> > > <laurent.pinchart@ideasonboard.com> wrote:
> > > > I need to implement support for a YUV frame buffer in an fbdev driver.
> > As the
> > > > fbdev API doesn't support this out of the box, I've spent a couple of
> > days
> > > > reading fbdev (and KMS) code and thinking about how we could cleanly
> > add YUV
> > > > support to the API. I'd like to share my findings and thoughts, and
> > hopefully
> > > > receive some comments back.
> > > >
> > > > The terms 'format', 'pixel format', 'frame buffer format' and 'data
> > format'
> > > > will be used interchangeably in this e-mail. They all refer to the way
> > pixels
> > > > are stored in memory, including both the representation of a pixel as
> > integer
> > > > values and the layout of those integer values in memory.
> > >
> > > This is a great proposal. It was about time!
> > >
> > > > The third solution has my preference. Comments and feedback will be
> > > > appreciated. I will then work on a proof of concept and submit patches.
> > >
> > > I also would prefer the third solution. I don't think there's much
> > > difference from the user-space point of view, and a new ioctl would be
> > > cleaner. Also the v4l2 fourcc's should do.
> > 
> > I agree with this.
> > 
> > We might want to take the opportunity to fix this section of the V4L2 Spec:
> > 
> > http://www.xs4all.nl/~hverkuil/spec/media.html#pixfmt-rgb
> > 
> > There are two tables, 2.6 and 2.7. But 2.6 is almost certainly wrong and
> > should be removed.
> 
> That's definitely true. I was confused at the beginning when I saw 2
> different tables describing the same pixel formats.
> 
>  I suspect many if not all V4L2 drivers are badly broken for
> > big-endian systems and report the wrong pixel formats.
> > 
> > Officially the pixel formats reflect the contents of the memory. But
> > everything is swapped on a big endian system, so you are supposed to 
> > report a different pix format.
> 
> I always thought that pix_format describes the layout of video data in
> memory on byte level, which is exactly the same on both little- and big-
> endian systems.

Correct.

> You can notice swapped data only if you access memory
> by units larger than byte, like 16bit or 32bit integers. BTW - I would
> really like to avoid little- and big- endian flame, but your statement
> about 'everything is swapped on a big endian system' is completely
> wrong. It is rather the characteristic of little-endian system not big
> endian one if you display the content of the same memory first using
> byte access and then using word/long access.

You are correct, I wasn't thinking it through.
 
> > I can't remember seeing any driver do that. Some have built-in swapping,
> > though, and turn that on if needed.
> 
> The drivers shouldn't do ANY byte swapping at all. Only tools that
> extract pixel data with some 'accelerated' methods (like 32bit integer
> casting and bit-level shifting) should be aware of endianess.
> 
> > I really need to run some tests, but I've been telling myself this for
> > years now :-(
> 
> I've checked the BTTV board in my PowerMac/G4 and the display was
> correct with xawtv. It is just a matter of selecting correct pix format
> basing on the information returned by xsever. 
> 
> Best regards
> 

Just forget my post (except for the part of cleaning up the tables :-) ).

Regards,

	Hans

^ permalink raw reply

* [RFC] Which fb_var_screeninfo fields should FBIOPAN_DISPLAY use ?
From: Laurent Pinchart @ 2011-05-23 20:11 UTC (permalink / raw)
  To: linux-fbdev

Hi everybody,

I've recently received a patch for my fbdev test application to fix a panning-
related issue. The application naively assumed that only the xoffset and 
yoffset were used, and the patch commit message explained that several drivers 
relied on other fields being filled with current values.

I got curious about this. As there's no FBIOPAN_DISPLAY documentation that 
clearly states which fields are used and which fields must be ignored by 
drivers, I checked the in-tree fbdev drivers and here's what I found.

First of all, the FBIOPAN_DISPLAY implementation (drivers/video/fbmem.c) uses 
the xoffset, yoffset and vmode fields. That was expected, there's nothing too 
weird there. Several drivers also use the activate field, which seems like a 
valid use to me.

Then, two drivers (video/msm and staging/msm) use the reserved fields. As 
usage of the reserved fields is by definition not standard, I decided to 
ignore this.

Finally, here's a list of the drivers using other fields, along with the 
fields that are being used.

    media/video/ivtv - xres_virtual, bits_per_pixel
    video/68328fb - xres, yres
    video/acornfb - yres, yres_virtual
    video/arkfb - bits_per_pixel, xres_virtual
    video/atmel_lcdfb - bits_per_pixel, xres_virtual, xres
    video/aty/radeon - xres, yres, xres_virtual, yres_virtual, bits_per_pixel
    video/da8xx-fb - bits_per_pixel
    video/fb-puv3 - xres, yres
    video/g364fb - xres, yres, yres_virtual
    video/gxt4500 - xres, yres, xres_virtual, yres_virtual
    video/hgafb - xres, yres
    video/imsttfb - bits_per_pixel
    video/intelfb - xres, yres, xres_virtual, yres_virtual, bits_per_pixel
    video/mb862xx - xres_virtual, yres_virtual
    video/mx3fb - yres, xres_virtual, bits_per_pixel
    video/neofb - xres_virtual, bits_per_pixel
    video/pm2fb - bits_per_pixel, xres
    video/pm3fb - xres, bits_per_pixel
    video/s3c-fb - yres
    video/s3fb - bits_per_pixel, xres_virtual
    video/savage - xres_virtual, bits_per_pixel
    video/sh_mobile_lcdcfb - nonstd, xres, yres_virtual
    video/sis - xres, yres, xres_virtual, yres_virtual, bits_per_pixel
    video/sm501fb - xres_virtual, yres_virtual, bits_per_pixel
    video/tridentfb - xres_virtual, bits_per_pixel
    video/vfb - xres, yres
    video/vga16fb - bits_per_pixel
    video/via - xres_virtual, bits_per_pixel
    video/vt8500lcdfb - xres, xres_virtual
    video/vt8623fb - bits_per_pixel, xres_virtual
    video/xgifb - xres_virtual, yres_virtual, xres, yres, bits_per_pixel

These fields are used to validate the xoffset and yoffset parameters against 
the active resolution, as well as to compute offset in vram.

This clearly looks like bugs to me, especially given that many other drivers 
compute the same parameters using info->var instead of var. For instance, if 
drivers/video/aty/radeon_base.c implements the pan display operation as

        if ((var->xoffset + var->xres > var->xres_virtual)
            || (var->yoffset + var->yres > var->yres_virtual))
               return -EINVAL;

        ...

        OUTREG(CRTC_OFFSET, ((var->yoffset * var->xres_virtual + var->xoffset)
                             * var->bits_per_pixel / 8) & ~7);

If var isn't a copy of the current fb parameters, the code will clearly lead 
to a wrong behaviour. Even worse, some drivers use var to validate parameters 
and then info->var to compute offsets, or the other way around. This could 
lead to security issues.

I believe all those drivers should be fixed to use info->var instead of var to 
access the current xres, yres, xres_virtual, yres_virtual and bits_per_pixel 
fields in their pan display operation handler. However, this could break 
applications that rely on the current buggy behaviour. Would that be 
acceptable ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox