[PATCH 34/57] fbdev: sh_mobile_lcdc: Store the format in struct sh_mobile_lcdc_chan

[PATCH 34/57] fbdev: sh_mobile_lcdc: Store the format in struct sh_mobile_lcdc_chan

[Laurent Pinchart]

Store the active format in the channel structure, and use it instead of
parsing info->var all over the place when the format is needed.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |   21 ++++++++++-----------
 drivers/video/sh_mobile_lcdcfb.h |    4 +++-
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index c6b6b9d..9829e01 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -726,20 +726,15 @@ static void __sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 
 	/* Setup geometry, format, frame buffer memory and operation mode. */
 	for (k = 0; k < ARRAY_SIZE(priv->ch); k++) {
-		const struct sh_mobile_lcdc_format_info *format;
-		u32 fourcc;
-
 		ch = &priv->ch[k];
 		if (!ch->enabled)
 			continue;
 
 		sh_mobile_lcdc_geometry(ch);
 
-		fourcc = sh_mobile_format_fourcc(&ch->info->var);
-		format = sh_mobile_format_info(fourcc);
-		tmp = format->lddfr;
+		tmp = ch->format->lddfr;
 
-		if (format->yuv) {
+		if (ch->format->yuv) {
 			switch (ch->info->var.colorspace) {
 			case V4L2_COLORSPACE_REC709:
 				tmp |= LDDFR_CF1;
@@ -753,7 +748,7 @@ static void __sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 		lcdc_write_chan(ch, LDDFR, tmp);
 		lcdc_write_chan(ch, LDMLSR, ch->pitch);
 		lcdc_write_chan(ch, LDSA1R, ch->base_addr_y);
-		if (format->yuv)
+		if (ch->format->yuv)
 			lcdc_write_chan(ch, LDSA2R, ch->base_addr_c);
 
 		/* When using deferred I/O mode, configure the LCDC for one-shot
@@ -770,7 +765,7 @@ static void __sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 	}
 
 	/* Word and long word swap. */
-	switch (sh_mobile_format_fourcc(&priv->ch[0].info->var)) {
+	switch (priv->ch[0].format->fourcc) {
 	case V4L2_PIX_FMT_RGB565:
 	case V4L2_PIX_FMT_NV21:
 	case V4L2_PIX_FMT_NV61:
@@ -857,7 +852,7 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 			ch->meram_enabled = 0;
 		}
 
-		switch (sh_mobile_format_fourcc(&ch->info->var)) {
+		switch (ch->format->fourcc) {
 		case V4L2_PIX_FMT_NV12:
 		case V4L2_PIX_FMT_NV21:
 		case V4L2_PIX_FMT_NV16:
@@ -1063,7 +1058,7 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
 			    + info->var.xres * info->var.yres_virtual
 			    + c_offset;
 		/* Set x offset */
-		if (sh_mobile_format_fourcc(&info->var) = V4L2_PIX_FMT_NV24)
+		if (ch->format->fourcc = V4L2_PIX_FMT_NV24)
 			base_addr_c += 2 * var->xoffset;
 		else
 			base_addr_c += var->xoffset;
@@ -1350,6 +1345,8 @@ static int sh_mobile_set_par(struct fb_info *info)
 		info->fix.line_length = info->var.xres
 				      * info->var.bits_per_pixel / 8;
 
+	ch->format = sh_mobile_format_info(sh_mobile_format_fourcc(&info->var));
+
 	ret = sh_mobile_lcdc_start(ch->lcdc);
 	if (ret < 0) {
 		dev_err(info->dev, "%s: unable to restart LCDC\n", __func__);
@@ -1714,6 +1711,8 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_priv *priv,
 		return -EINVAL;
 	}
 
+	ch->format = format;
+
 	/* Allocate the frame buffer device. */
 	ch->info = framebuffer_alloc(0, priv->dev);
 	if (!ch->info) {
diff --git a/drivers/video/sh_mobile_lcdcfb.h b/drivers/video/sh_mobile_lcdcfb.h
index 8e0d009..5ef7559 100644
--- a/drivers/video/sh_mobile_lcdcfb.h
+++ b/drivers/video/sh_mobile_lcdcfb.h
@@ -17,9 +17,10 @@ enum { LDDCKPAT1R, LDDCKPAT2R, LDMT1R, LDMT2R, LDMT3R, LDDFR, LDSM1R,
 struct backlight_device;
 struct fb_info;
 struct module;
+struct sh_mobile_lcdc_chan;
 struct sh_mobile_lcdc_entity;
+struct sh_mobile_lcdc_format_info;
 struct sh_mobile_lcdc_priv;
-struct sh_mobile_lcdc_chan;
 
 #define SH_MOBILE_LCDC_DISPLAY_DISCONNECTED	0
 #define SH_MOBILE_LCDC_DISPLAY_CONNECTED	1
@@ -70,6 +71,7 @@ struct sh_mobile_lcdc_chan {
 	wait_queue_head_t frame_end_wait;
 	struct completion vsync_completion;
 
+	const struct sh_mobile_lcdc_format_info *format;
 	unsigned long base_addr_y;
 	unsigned long base_addr_c;
 	unsigned int pitch;
-- 
1.7.3.4

Re: [PATCH 34/57] fbdev: sh_mobile_lcdc: Store the format in struct

[Guennadi Liakhovetski]

On Tue, 13 Dec 2011, Laurent Pinchart wrote:

> Store the active format in the channel structure, and use it instead of
> parsing info->var all over the place when the format is needed.

Right, this is what I was wondering about, while looking at the previous 
patch:-) But:

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/video/sh_mobile_lcdcfb.c |   21 ++++++++++-----------
>  drivers/video/sh_mobile_lcdcfb.h |    4 +++-
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
> index c6b6b9d..9829e01 100644
> --- a/drivers/video/sh_mobile_lcdcfb.c
> +++ b/drivers/video/sh_mobile_lcdcfb.c

[snip]

> @@ -1350,6 +1345,8 @@ static int sh_mobile_set_par(struct fb_info *info)
>  		info->fix.line_length = info->var.xres
>  				      * info->var.bits_per_pixel / 8;
>  
> +	ch->format = sh_mobile_format_info(sh_mobile_format_fourcc(&info->var));

Cannot this be NULL? As far as I could trace it back, I'm not sure with 
hotplug bits_per_pixel would be initialised correctly along the lines of 
sh_mobile_fb_reconfig().

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 34/57] fbdev: sh_mobile_lcdc: Store the format in struct sh_mobile_lcdc_chan

[Laurent Pinchart]

Hi Guennadi,

On Thursday 15 December 2011 23:29:29 Guennadi Liakhovetski wrote:
> On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > Store the active format in the channel structure, and use it instead of
> > parsing info->var all over the place when the format is needed.
> 
> Right, this is what I was wondering about, while looking at the previous
> patch:-)

So I'll consider your comment to patch 33/57 to be addressed :-)

> But:
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/video/sh_mobile_lcdcfb.c |   21 ++++++++++-----------
> >  drivers/video/sh_mobile_lcdcfb.h |    4 +++-
> >  2 files changed, 13 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > b/drivers/video/sh_mobile_lcdcfb.c index c6b6b9d..9829e01 100644
> > --- a/drivers/video/sh_mobile_lcdcfb.c
> > +++ b/drivers/video/sh_mobile_lcdcfb.c
> 
> [snip]
> 
> > @@ -1350,6 +1345,8 @@ static int sh_mobile_set_par(struct fb_info *info)
> > 
> >  		info->fix.line_length = info->var.xres
> >  		
> >  				      * info->var.bits_per_pixel / 8;
> > 
> > +	ch->format > > sh_mobile_format_info(sh_mobile_format_fourcc(&info->var));
> 
> Cannot this be NULL? As far as I could trace it back, I'm not sure with
> hotplug bits_per_pixel would be initialised correctly along the lines of
> sh_mobile_fb_reconfig().

You're right, sh_mobile_fb_reconfig() doesn't set the bits_per_pixel (and 
other format-related) field. I'll fix this by initializing those fields from 
the current var in sh_mobile_fb_reconfig() in patch 30/57, as that's the one 
that introduces the problem.

-- 
Regards,

Laurent Pinchart