Hello,

-----Original Message-----
From: linux-samsung-soc-owner at vger.kernel.org [mailto:linux-samsung-
soc-owner at vger.kernel.org] On Behalf Of Maurus Cuelenaere
Sent: Thursday, July 15, 2010 12:32 PM
To: Sylwester Nawrocki
Cc: linux-samsung-soc at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; p.osciak at samsung.com;
m.szyprowski at samsung.com; kyungmin.park at samsung.com;
kgene.kim at samsung.com; ben-linux at fluff.org; linux-fbdev at vger.kernel.org
Subject: Re: [PATCH 05/10 v2] s3c-fb: Add v4l2 subdevice to support
framebuffer local fifo input path

 Op 15-07-10 11:10, Sylwester Nawrocki schreef:

quoted

Selected multimedia devices in Samsung S3C/S5P SoC series are capable

of

quoted

transferring data directly between each other, bypassing the main

system

quoted

bus. Such a datapath exists between the camera interface/video
postprocessor and the lcd controller. To control the data flow from

the

quoted

fimc driver level v4l2-subdevice driver is added to the framebuffer.
It enables to configure the lcd controller into FIFO or DMA input

mode.

quoted

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/plat-samsung/include/plat/fb.h |    6 +
 drivers/video/s3c-fb.c                  |  487

+++++++++++++++++++++++++++++--

quoted

 2 files changed, 472 insertions(+), 21 deletions(-)

<snip>

quoted

 /**
+ * s3c_fb_enable_local() - switch window between input DMA and fifo

modes

quoted

+ *
+ * @fb_sd: window subdevice for fifo input
+ * @en:    1 - switch from input DMA to fifo mode and apply
+ *	       window size and position set by the window's subdevice
+ *         0 - restore from fifo to DMA mode
+ */
+static int s3c_fb_enable_local_in(struct s3c_fb_win_sd *fb_sd, int

en)

quoted

+{
+	struct s3c_fb_win *win = fb_sd->win;
+	struct s3c_fb *sfb = win->parent;
+	static u32 wincon;
+	u32 reg, data;
+	int ret = 0, bpp = 32;
+
+	/* disable video output and the window logic */
+	reg = readl(sfb->regs + WINCON(win->index));
+	writel(reg & ~WINCONx_ENWIN, sfb->regs + WINCON(win->index));
+
+	shadow_protect_win(win, 1);
+
+	if (en == 1) {
+		if (fb_sd->streaming)
+			return 0;

Shouldn't you do shadow_protect_win(win, 0) before returning here?

Yes, indeed the error path is wrong and also WINCONx_ENWIN
needs to be restored.
Thank you for pointing this out. 

quoted

+
+		wincon = reg;
+
+		switch (fb_sd->fmt.fmt.pix.pixelformat) {
+		case V4L2_PIX_FMT_YUYV: /* YCbCr 4:4:4 */
+			reg |= WINCONx_YCbCr | WINCONx_ENLOCAL;
+			bpp = 16;
+			break;
+
+		case V4L2_PIX_FMT_RGB24:
+		default:
+			reg &= ~(WINCONx_YCbCr | WINCONx_WSWP |

WINCONx_HAWSWP |

quoted

+				 WINCONx_BYTSWP | WINCONx_BITSWP |
+				 WINCON0_BPPMODE_MASK | WINCONx_BURSTLEN_MASK);
+
+			reg |=	WINCON0_BPPMODE_24BPP_888 |
+				WINCONx_BURSTLEN_4WORD;
+			bpp = 24;
+			break;
+		}
+
+		fb_sd->streaming = 1;
+		writel(reg, sfb->regs + WINCON(win->index));
+
+		s3c_fb_set_osd(fb_sd->win, &fb_sd->curr_osd_win, bpp);
+
+		writel(reg | WINCONx_ENLOCAL, sfb->regs + WINCON(win-
index));
+
+		shadow_protect_win(win, 0);
+
+		reg = readl(sfb->regs + WINCON(win->index));
+		writel(reg | WINCONx_ENWIN, sfb->regs + WINCON(win-
index));
+
+		if (sfb->variant.has_shadowcon)	{
+			data = readl(sfb->regs + SHADOWCON);
+			data |=	SHADOWCON_CHx_LOCAL_ENABLE(win->index);
+			writel(data, sfb->regs + SHADOWCON);
+		}
+
+	} else if (en == 0) {
+		if (!fb_sd->streaming)
+			return 0;

Same comment

quoted

+
+		fb_sd->streaming = 0;
+
+		/* need to be aligned with VSYNC interrupt */
+		writel(wincon & ~WINCONx_ENLOCAL,
+		       sfb->regs + WINCON(win->index));
+
+		/* restore OSD values from before we enabled local mode */
+		bpp = win->fbinfo->var.bits_per_pixel;
+		s3c_fb_set_osd(fb_sd->win, &fb_sd->default_osd_win, bpp);
+
+		shadow_protect_win(win, 0);
+
+		if (sfb->variant.has_shadowcon)	{
+			data = readl(sfb->regs + SHADOWCON);
+			data &=	~SHADOWCON_CHx_LOCAL_ENABLE(win->index);
+			writel(data, sfb->regs + SHADOWCON);
+		}
+
+		reg = readl(sfb->regs + WINCON(win->index));
+		writel(reg | WINCONx_ENWIN, sfb->regs + WINCON(win-
index));
+	} else {
+		return -EINVAL;
+	}
+
+	return ret;
+}
+

--
Maurus Cuelenaere

Regards,
--
Sylwester Nawrocki

--
To unsubscribe from this list: send the line "unsubscribe linux-
samsung-soc" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html