Subject: Re: [PATCH v3 09/11] powerpc/mpc5121: shared DIU framebuffer support

From: Anatolij Gustschin <[email-masked]>
Date: Sat, 27 Feb 2010 23:09:03 +0100

On Tue, 16 Feb 2010 11:06:22 -0700
Grant Likely <[email-masked]> wrote:

> [...]
> > diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
> > index 72d68b3..29c7f31 100644
> > --- a/drivers/video/fsl-diu-fb.c
> > +++ b/drivers/video/fsl-diu-fb.c
> > @@ -34,7 +34,7 @@
> >  #include <linux/of_platform.h>
> >
> >  #include <sysdev/fsl_soc.h>
> > -#include "fsl-diu-fb.h"
> > +#include <linux/fsl-diu-fb.h>
> >
> >  /*
> >  * These parameters give default parameters
> > @@ -178,6 +178,21 @@ static struct fb_videomode __devinitdata fsl_diu_mode_db[] = {
> >                .sync           = FB_SYNC_COMP_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
> >                .vmode          = FB_VMODE_NONINTERLACED
> >        },
> > +       {
> > +               .name           = "800x480-60",
> > +               .refresh        = 60,
> > +               .xres           = 800,
> > +               .yres           = 480,
> > +               .pixclock       = 31250,
> > +               .left_margin    = 86,
> > +               .right_margin   = 42,
> > +               .upper_margin   = 33,
> > +               .lower_margin   = 10,
> > +               .hsync_len      = 128,
> > +               .vsync_len      = 2,
> > +               .sync           = FB_SYNC_COMP_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
> > +               .vmode          = FB_VMODE_NONINTERLACED
> > +       },
> >  };
> 
> This hunk bothers me.  It looks like the type of data that belongs
> either in some common shared .c file, or encoded into the device tree.
>  It seems to be data about the display panel, instead of data about
> the framebuffer driver.  I know that the driver already uses this
> pattern, but before I merge this patch and further rely on that
> pattern, I think it is worth discussing.

In the updated DIU patches I don't add panel timing data to the
framebuffer driver. It is encoded in the device tree now. If this
is acceptable, I'll send another patch adding the documentation
for added bindings.

Thanks,
Anatolij