2014-01-10 12:08:41

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH v4] video: add OpenCores VGA/LCD framebuffer driver

Hi,

On 2013-11-29 17:45, Stefan Kristiansson wrote:
> This adds support for the VGA/LCD core available from OpenCores:
> http://opencores.org/project,vga_lcd
>
> The driver have been tested together with both OpenRISC and
> ARM (socfpga) processors.
>
> Signed-off-by: Stefan Kristiansson <[email protected]>

If I'm not mistaken, this driver has been designed so that there can
only be one VGA/LCD core device on the system.

It would be better to get rid of those static structs, and have the
driver data per device as it should. That way the driver works even with
multiple VGA/LCD cores (although the video mode module option is a bit
odd in that case).

Tomi



Attachments:
signature.asc (901.00 B)
OpenPGP digital signature

2014-01-10 16:16:58

by Stefan Kristiansson

[permalink] [raw]
Subject: Re: [PATCH v4] video: add OpenCores VGA/LCD framebuffer driver

On Fri, Jan 10, 2014 at 02:08:27PM +0200, Tomi Valkeinen wrote:
> Hi,
>
> On 2013-11-29 17:45, Stefan Kristiansson wrote:
> > This adds support for the VGA/LCD core available from OpenCores:
> > http://opencores.org/project,vga_lcd
> >
> > The driver have been tested together with both OpenRISC and
> > ARM (socfpga) processors.
> >
> > Signed-off-by: Stefan Kristiansson <[email protected]>
>
> If I'm not mistaken, this driver has been designed so that there can
> only be one VGA/LCD core device on the system.
>
> It would be better to get rid of those static structs, and have the
> driver data per device as it should. That way the driver works even with
> multiple VGA/LCD cores (although the video mode module option is a bit
> odd in that case).
>

You're right, it's a completely realistic scenario with several
VGA/LCD cores in the system, so the one device only restriction is bad.
And looking closer at the static structs, I see that the ocfb_var and ocfb_fix
doesn't surve any purpose at all, so they can just be removed.
And the information in ocfb_par can easily be obtained directly from ocfb_dev
instead, so by setting info.par to point at that, the ocfb_par struct can also
be removed completely.
Thanks, that suggestion makes the implementation cleaner as well.

I'll post an updated patch with the changes.

Stefan