Re: [PATCH v2 1/6] fb: add a line_length value to struct fb_info

[Alexander Aring <alex.aring@gmail.com>]

On Tue, Nov 05, 2013 at 09:48:07AM +0100, Andre Heider wrote:
> Hi Alexander,
> 
> On Tue, Nov 05, 2013 at 09:17:44AM +0100, Alexander Aring wrote:
> > Hi Sascha,
> > 
> > On Tue, Nov 05, 2013 at 09:00:57AM +0100, Sascha Hauer wrote:
> > > On Tue, Nov 05, 2013 at 01:06:22AM +0100, Alexander Aring wrote:
> > > > On Tue, Nov 05, 2013 at 12:00:59AM +0100, Andre Heider wrote:
> > > > > Add support for framebuffers with noncontiguous horizontal lines.
> > > > > 
> > > > > Video drivers can set this value if the hardware requires it.
> > > > > In case a driver does not set it, the current value of
> > > > > xres * (bpp / 8) is used instead.
> > > > > 
> > > > > Signed-off-by: Andre Heider <a.heider@gmail.com>
> > > > > ---
> > > > >  drivers/video/fb.c | 11 +++++++++--
> > > > >  include/fb.h       |  1 +
> > > > >  2 files changed, 10 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/video/fb.c b/drivers/video/fb.c
> > > > > index 420e4e3..0159994 100644
> > > > > --- a/drivers/video/fb.c
> > > > > +++ b/drivers/video/fb.c
> > > > > @@ -72,12 +72,16 @@ static int fb_setup_mode(struct device_d *dev, struct param_d *param,
> > > > >  
> > > > >  	info->xres = info->mode->xres;
> > > > >  	info->yres = info->mode->yres;
> > > > > +	info->line_length = 0;
> > > > >  
> > > > >  	ret = info->fbops->fb_activate_var(info);
> > > > >  
> > > > > +	if (!info->line_length)
> > > > > +		info->line_length = info->xres * (info->bits_per_pixel >> 3);
> > > > > +
> > > > Isn't this already set from the register_framebuffer function?
> > > > You wanna see that somebody changed this value in info->fbops->fb_activate_var(info)?
> > > > 
> > > > Then I would do nothing here.
> > > 
> > > It is set from register_framebuffer, but the mode setup invalidates it,
> > > So Andre sets it to 0 first and updates it after calling into the driver
> > > if necessary. The code looks correct. Maybe I misunderstand you?
> > > 
> > ok. Was a hard day and night for me ;-)
> > Maybe I was too tired and look all day into another code and other
> > things ;-).
> > 
> > I think I need a coffee or beer now.
> > 
> > 
> > I mean we have the same code in register_framebuffer:
> > 
> > if (!info->line_length)
> >         info->line_length = info->xres * (info->bits_per_pixel >> 3);
> > 
> > I don't know the exactly context to call fb_setup_mode but then we set
> > it twice and we don't need to check if the value has changed from
> > fb_activate_var function.
> 
> With this patch we have a new line_length value, but fb drivers don't
> have to set it, its optional. What you see in register_framebuffer()
> is a default value for the initial mode. The framework sets it in case
> the driver doesn't.
> 
> But we can also change modes, so if a fb driver doesn't update
> line_length in the fb_activate_var callback, we have a stale line_length
> value from the old mode.
> So I set it to zero before the callback and check afterwards if the
> driver set it. Without that there would be no way to distinguish if the
> value is valid for the new mode or a stale one from the prior mode.
> 
ok, thanks for the little explaination.

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox