Re: [PATCH v2 1/5] [media] exynos-mscl: Add new driver for M-Scaler

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/19/2013 12:58 PM, Shaik Ameer Basha wrote:
This patch adds support for M-Scaler (M2M Scaler) device which is a
new device for scaling, blending, color fill  and color space
conversion on EXYNOS5 SoCs.

This device supports the followings as key feature.
     input image format
         - YCbCr420 2P(UV/VU), 3P
         - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P
         - YCbCr444 2P(UV,VU), 3P
         - RGB565, ARGB1555, ARGB4444, ARGB8888, RGBA8888
         - Pre-multiplexed ARGB8888, L8A8 and L8
     output image format
         - YCbCr420 2P(UV/VU), 3P
         - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P
         - YCbCr444 2P(UV,VU), 3P
         - RGB565, ARGB1555, ARGB4444, ARGB8888, RGBA8888
         - Pre-multiplexed ARGB8888
     input rotation
         - 0/90/180/270 degree, X/Y/XY Flip
     scale ratio
         - 1/4 scale down to 16 scale up
     color space conversion
         - RGB to YUV / YUV to RGB
     Size
         - Input : 16x16 to 8192x8192
         - Output:   4x4 to 8192x8192
     alpha blending, color fill

We don't have good support for alpha blending in v4l2. For example, the
s5p-g2d v4l2 driver only supports a subset of features of the device.
The G2D IP is probably better utilized through the exynos drm driver.

It might be a matter of designing proper controls though for such devices
that generate image by blending data from the output and capture buffers.

Signed-off-by: Shaik Ameer Basha<shaik.ameer@xxxxxxxxxxx>
---
  drivers/media/platform/exynos-mscl/mscl-regs.c |  318 ++++++++++++++++++++++++
  drivers/media/platform/exynos-mscl/mscl-regs.h |  282 +++++++++++++++++++++

Shouldn't we call this driver exynos5-scaler and this files would be
renamed to drivers/media/platform/exynos5-scaler/scaler-regs.[ch].

Or maybe put maybe put the SCALER, G-SCALER drivers into common exynos5-is
directory ? I don't have strong preference. But we planned gscaler to be
moved into the exynos5-is directory.

  2 files changed, 600 insertions(+)
  create mode 100644 drivers/media/platform/exynos-mscl/mscl-regs.c
  create mode 100644 drivers/media/platform/exynos-mscl/mscl-regs.h

diff --git a/drivers/media/platform/exynos-mscl/mscl-regs.c b/drivers/media/platform/exynos-mscl/mscl-regs.c
new file mode 100644
index 0000000..9354afc
--- /dev/null
+++ b/drivers/media/platform/exynos-mscl/mscl-regs.c
@@ -0,0 +1,318 @@
+/*
+ * Copyright (c) 2013 - 2014 Samsung Electronics Co., Ltd.

Just make it 2013 ? I think we can manage to merge this diver this year :)

+ *		http://www.samsung.com
+ *
+ * Samsung EXYNOS5 SoC series M-Scaler driver
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation, either version 2 of the License,
+ * or (at your option) any later version.

Are you sure you want to have this "or (at your option) any later version"
clause ?

+ */
+
+#include<linux/delay.h>
+#include<linux/platform_device.h>
+
+#include "mscl-core.h"

How about defining register access helpers like:

u32 mscl_read(struct mscl_dev *dev, u32 offset)
{
	readl(dev->regs + offset);
}

void mscl_write(struct mscl_dev *dev, u32 offset, u32 value)
{
	writel(value, dev->regs + offset);
}

perhaps as static inline in mscl-regs.h ?

+void mscl_hw_set_sw_reset(struct mscl_dev *dev)
+{
+	u32 cfg;
+
+	cfg = readl(dev->regs + MSCL_CFG);
+	cfg |= MSCL_CFG_SOFT_RESET;
+
+	writel(cfg, dev->regs + MSCL_CFG);
+}
+
+int mscl_wait_reset(struct mscl_dev *dev)
+{
+	unsigned long end = jiffies + msecs_to_jiffies(50);

Don't you want a #define for this timeout ?

+	u32 cfg, reset_done = 0;
+
+	while (time_before(jiffies, end)) {
+		cfg = readl(dev->regs + MSCL_CFG);
+		if (!(cfg&  MSCL_CFG_SOFT_RESET)) {
+			reset_done = 1;
+			break;
+		}
+		usleep_range(10, 20);
+	}
+
+	/* write any value to r/w reg and read it back */
+	while (reset_done) {
+
+		/* [TBD] need to define number of tries before returning
+		 * -EBUSY to the caller
+		 */

CodingStyle: wrong multi-line comment style

+		writel(MSCL_CFG_SOFT_RESET_CHECK_VAL,
+				dev->regs + MSCL_CFG_SOFT_RESET_CHECK_REG);
+		if (MSCL_CFG_SOFT_RESET_CHECK_VAL ==
+			readl(dev->regs + MSCL_CFG_SOFT_RESET_CHECK_REG))
+			return 0;
+	}
+
+	return -EBUSY;
+}
+
+void mscl_hw_set_irq_mask(struct mscl_dev *dev, int interrupt, bool mask)
+{
+	u32 cfg;
+
+	switch (interrupt) {
+	case MSCL_INT_TIMEOUT:
+	case MSCL_INT_ILLEGAL_BLEND:
+	case MSCL_INT_ILLEGAL_RATIO:
+	case MSCL_INT_ILLEGAL_DST_HEIGHT:
+	case MSCL_INT_ILLEGAL_DST_WIDTH:
+	case MSCL_INT_ILLEGAL_DST_V_POS:
+	case MSCL_INT_ILLEGAL_DST_H_POS:
+	case MSCL_INT_ILLEGAL_DST_C_SPAN:
+	case MSCL_INT_ILLEGAL_DST_Y_SPAN:
+	case MSCL_INT_ILLEGAL_DST_CR_BASE:
+	case MSCL_INT_ILLEGAL_DST_CB_BASE:
+	case MSCL_INT_ILLEGAL_DST_Y_BASE:
+	case MSCL_INT_ILLEGAL_DST_COLOR:
+	case MSCL_INT_ILLEGAL_SRC_HEIGHT:
+	case MSCL_INT_ILLEGAL_SRC_WIDTH:
+	case MSCL_INT_ILLEGAL_SRC_CV_POS:
+	case MSCL_INT_ILLEGAL_SRC_CH_POS:
+	case MSCL_INT_ILLEGAL_SRC_YV_POS:
+	case MSCL_INT_ILLEGAL_SRC_YH_POS:
+	case MSCL_INT_ILLEGAL_SRC_C_SPAN:
+	case MSCL_INT_ILLEGAL_SRC_Y_SPAN:
+	case MSCL_INT_ILLEGAL_SRC_CR_BASE:
+	case MSCL_INT_ILLEGAL_SRC_CB_BASE:
+	case MSCL_INT_ILLEGAL_SRC_Y_BASE:
+	case MSCL_INT_ILLEGAL_SRC_COLOR:
+	case MSCL_INT_FRAME_END:

What if more than one of those flag is set in 'interrupt' argument ?
You probably want to add a single macro definition for all the above
bits.

+		break;
+	default:
+		return;
+	}
+	cfg = readl(dev->regs + MSCL_INT_EN);
+	if (mask)
+		cfg |= interrupt;
+	else
+		cfg&= ~interrupt;
+	writel(cfg, dev->regs + MSCL_INT_EN);
+}
+
+void mscl_hw_set_input_addr(struct mscl_dev *dev, struct mscl_addr *addr)
+{
+	dev_dbg(&dev->pdev->dev, "src_buf: 0x%X, cb: 0x%X, cr: 0x%X",
+				addr->y, addr->cb, addr->cr);

Hmm, perhaps add a macro to simply these dev_dbg() calls, e.g.

#define mscl_dbg(_dev, fmt, args...) dev_dbg(&_dev->pdev->dev, fmt, ##args)

?
+	writel(addr->y, dev->regs + MSCL_SRC_Y_BASE);
+	writel(addr->cb, dev->regs + MSCL_SRC_CB_BASE);
+	writel(addr->cr, dev->regs + MSCL_SRC_CR_BASE);
+}
+
+void mscl_hw_set_output_addr(struct mscl_dev *dev,
+			     struct mscl_addr *addr)
+{
+	dev_dbg(&dev->pdev->dev, "dst_buf: 0x%X, cb: 0x%X, cr: 0x%X",
+				addr->y, addr->cb, addr->cr);
+	writel(addr->y, dev->regs + MSCL_DST_Y_BASE);
+	writel(addr->cb, dev->regs + MSCL_DST_CB_BASE);
+	writel(addr->cr, dev->regs + MSCL_DST_CR_BASE);
+}
+
+void mscl_hw_set_in_size(struct mscl_ctx *ctx)
+{
+	struct mscl_dev *dev = ctx->mscl_dev;
+	struct mscl_frame *frame =&ctx->s_frame;
+	u32 cfg;
+
+	/* set input pixel offset */
+	cfg = MSCL_SRC_YH_POS(frame->crop.left);
+	cfg |= MSCL_SRC_YV_POS(frame->crop.top);
+	writel(cfg, dev->regs + MSCL_SRC_Y_POS);
+
+	/* [TBD] calculate 'C' plane h/v offset using 'Y' plane h/v offset */
+
+	/* set input span */
+	cfg = MSCL_SRC_Y_SPAN(frame->f_width);
+	if (is_yuv420_2p(frame->fmt))
+		cfg |= MSCL_SRC_C_SPAN(frame->f_width);
+	else
+		cfg |= MSCL_SRC_C_SPAN(frame->f_width); /* [TBD] Verify */

s/[TBD]/TODO: ?

+	writel(cfg, dev->regs + MSCL_SRC_SPAN);
+
+	/* Set input cropped size */
+	cfg = MSCL_SRC_WIDTH(frame->crop.width);
+	cfg |= MSCL_SRC_HEIGHT(frame->crop.height);
+	writel(cfg, dev->regs + MSCL_SRC_WH);
+
+	dev_dbg(&dev->pdev->dev,
+		"src: posx: %d, posY: %d, spanY: %d, spanC: %d, "
+		"cropX: %d, cropY: %d\n",
+		frame->crop.left, frame->crop.top, frame->f_width,
+		frame->f_width, frame->crop.width, frame->crop.height);
+}
+
+void mscl_hw_set_in_image_format(struct mscl_ctx *ctx)
+{
+	struct mscl_dev *dev = ctx->mscl_dev;
+	struct mscl_frame *frame =&ctx->s_frame;
+	u32 cfg;
+
+	cfg = readl(dev->regs + MSCL_SRC_CFG);
+	cfg&= ~MSCL_SRC_COLOR_FORMAT_MASK;
+	cfg |= MSCL_SRC_COLOR_FORMAT(frame->fmt->mscl_color);
+
+	/* setting tile/linear format */

s/tile/tiled ?

+	if (frame->fmt->is_tiled)
+		cfg |= MSCL_SRC_TILE_EN;
+	else
+		cfg&= ~MSCL_SRC_TILE_EN;
+
+	writel(cfg, dev->regs + MSCL_SRC_CFG);
+}
+
+void mscl_hw_set_out_size(struct mscl_ctx *ctx)
+{
+	struct mscl_dev *dev = ctx->mscl_dev;
+	struct mscl_frame *frame =&ctx->d_frame;
+	u32 cfg;
+
+	/* set output pixel offset */
+	cfg = MSCL_DST_H_POS(frame->crop.left);
+	cfg |= MSCL_DST_V_POS(frame->crop.top);

I personally find this kind of macros rather confusing - it's not clear
whether they prepare a value to be applied to a register or extract some
property from a value read from the register. I would suggest open coding
this with proper shifts and only #defined masks. But it's up to you,
I personally find such a coding style a bit disturbing :)

+	writel(cfg, dev->regs + MSCL_DST_POS);
+
+	/* set output span */
+	cfg = MSCL_DST_Y_SPAN(frame->f_width);
+	if (is_yuv420_2p(frame->fmt))
+		cfg |= MSCL_DST_C_SPAN(frame->f_width/2);
+	else
+		cfg |= MSCL_DST_C_SPAN(frame->f_width);
+	writel(cfg, dev->regs + MSCL_DST_SPAN);
+
+	/* set output scaled size */
+	cfg = MSCL_DST_WIDTH(frame->crop.width);
+	cfg |= MSCL_DST_HEIGHT(frame->crop.height);
+	writel(cfg, dev->regs + MSCL_DST_WH);
+
+	dev_dbg(&dev->pdev->dev,
+		"dst: posx: %d, posY: %d, spanY: %d, spanC: %d, "
+		"cropX: %d, cropY: %d\n",

		"DST: pos X: %d, pos Y: %d, span Y: %d, span C: %d, "
		"crop X: %d, crop Y: %d\n",

?
+		frame->crop.left, frame->crop.top, frame->f_width,
+		frame->f_width, frame->crop.width, frame->crop.height);
+}
+
+void mscl_hw_set_out_image_format(struct mscl_ctx *ctx)
+{
+	struct mscl_dev *dev = ctx->mscl_dev;
+	struct mscl_frame *frame =&ctx->d_frame;
+	u32 cfg;
+
+	cfg = readl(dev->regs + MSCL_DST_CFG);
+	cfg&= ~MSCL_DST_COLOR_FORMAT_MASK;
+	cfg |= MSCL_DST_COLOR_FORMAT(frame->fmt->mscl_color);

This kind of macros really obfuscate code, this could be simply written as:

	cfg |= (frame->fmt->mscl_color & MSCL_DST_COLOR_FORMAT_MASK);

And these macros are usually used only in single place and IMHO they
aren't horribly helpful.

+
+	writel(cfg, dev->regs + MSCL_DST_CFG);
+}
+
+void mscl_hw_set_scaler_ratio(struct mscl_ctx *ctx)
+{
+	struct mscl_dev *dev = ctx->mscl_dev;
+	struct mscl_scaler *sc =&ctx->scaler;
+	u32 cfg;
+
+	cfg = MSCL_H_RATIO_VALUE(sc->hratio);
+	writel(cfg, dev->regs + MSCL_H_RATIO);
+
+	cfg = MSCL_V_RATIO_VALUE(sc->vratio);
+	writel(cfg, dev->regs + MSCL_V_RATIO);
+}
+
+void mscl_hw_set_rotation(struct mscl_ctx *ctx)
+{
+	struct mscl_dev *dev = ctx->mscl_dev;
+	u32 cfg = 0;
+
+	cfg = MSCL_ROTMODE(ctx->ctrls_mscl.rotate->val/90);

nit: missing spaces around '/'

+	if (ctx->ctrls_mscl.hflip->val)
+		cfg |= MSCL_FLIP_X_EN;
+
+	if (ctx->ctrls_mscl.vflip->val)
+		cfg |= MSCL_FLIP_Y_EN;
+
+	writel(cfg, dev->regs + MSCL_ROT_CFG);
+}
+
+void mscl_hw_address_queue_reset(struct mscl_ctx *ctx)
+{
+	struct mscl_dev *dev = ctx->mscl_dev;
+
+	writel(MSCL_ADDR_QUEUE_RST, dev->regs + MSCL_ADDR_QUEUE_CONFIG);

With mscl_write() (or scaler_write()) this would become a one-liner
and could be moved to the .h file.

+}
+
+void mscl_hw_set_csc_coeff(struct mscl_ctx *ctx)
+{
+	struct mscl_dev *dev = ctx->mscl_dev;
+	enum mscl_csc_coeff type;
+	u32 cfg = 0;
+	int i, j;
+	static const u32 csc_coeff[MSCL_CSC_COEFF_MAX][3][3] = {
+		{ /* YCbCr to RGB */
+			{0x200, 0x000, 0x2be},
+			{0x200, 0xeac, 0x165},
+			{0x200, 0x377, 0x000}
+		},
+		{ /* YCbCr to RGB with -16 offset */
+			{0x254, 0x000, 0x331},
+			{0x254, 0xec8, 0xFA0},

nit: please use lower case for hexadecimal numbers consistently.

+			{0x254, 0x409, 0x000}
+		},
+		{ /* RGB to YCbCr */
+			{0x099, 0x12d, 0x03a},
+			{0xe58, 0xeae, 0x106},
+			{0x106, 0xedb, 0xe2a}
+		},
+		{ /* RGB to YCbCr with -16 offset */
+			{0x084, 0x102, 0x032},
+			{0xe4c, 0xe95, 0x0e1},
+			{0x0e1, 0xebc, 0xe24}
+		} };
+
+	if (is_rgb(ctx->s_frame.fmt) == is_rgb(ctx->d_frame.fmt))
+		type = MSCL_CSC_COEFF_NONE;
+	else if (is_rgb(ctx->d_frame.fmt))
+		type = MSCL_CSC_COEFF_YCBCR_TO_RGB_OFF16;
+	else
+		type = MSCL_CSC_COEFF_RGB_TO_YCBCR_OFF16;
+
+	if ((type == ctx->mscl_dev->coeff_type) || (type>= MSCL_CSC_COEFF_MAX))

nit: superfluous braces

+		return;
+
+	for (i = 0; i<  3; i++) {
+		for (j = 0; j<  3; j++) {
+			cfg = csc_coeff[type][i][j];
+			writel(cfg, dev->regs + MSCL_CSC_COEF(i, j));
+		}
+	}
+
+	switch (type) {
+	case MSCL_CSC_COEFF_YCBCR_TO_RGB:
+		mscl_hw_src_y_offset_en(ctx->mscl_dev, false);
+		break;
+	case MSCL_CSC_COEFF_YCBCR_TO_RGB_OFF16:
+		mscl_hw_src_y_offset_en(ctx->mscl_dev, true);
+		break;
+	case MSCL_CSC_COEFF_RGB_TO_YCBCR:
+		mscl_hw_src_y_offset_en(ctx->mscl_dev, false);
+		break;
+	case MSCL_CSC_COEFF_RGB_TO_YCBCR_OFF16:
+		mscl_hw_src_y_offset_en(ctx->mscl_dev, true);
+		break;
+	default:
+		return;
+	}
+
+	ctx->mscl_dev->coeff_type = type;
+	return;

This could be removed.

+}
diff --git a/drivers/media/platform/exynos-mscl/mscl-regs.h b/drivers/media/platform/exynos-mscl/mscl-regs.h
new file mode 100644
index 0000000..02e2294d
--- /dev/null
+++ b/drivers/media/platform/exynos-mscl/mscl-regs.h
@@ -0,0 +1,282 @@
+/*
+ * Copyright (c) 2013 - 2014 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Register definition file for Samsung M-Scaler driver
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef REGS_MSCL_H_
+#define REGS_MSCL_H_
+
+/* m2m-scaler status */

nit: perhaps use M-Scaler (or SCALER) name consistently, or omit m2m-scaler
since all these registers are for the SCALER only ?

+#define MSCL_STATUS				0x00
+#define MSCL_STATUS_RUNNING			(1<<  1)
+#define MSCL_STATUS_READY_CLK_DOWN		(1<<  0)
+
+/* m2m-scaler config */
+#define MSCL_CFG				0x04
+#define MSCL_CFG_FILL_EN			(1<<  24)
+#define MSCL_CFG_BLEND_CLR_DIV_ALPHA_EN		(1<<  17)
+#define MSCL_CFG_BLEND_EN			(1<<  16)
+#define MSCL_CFG_CSC_Y_OFFSET_SRC_EN		(1<<  10)
+#define MSCL_CFG_CSC_Y_OFFSET_DST_EN		(1<<  9)
+#define MSCL_CFG_16_BURST_MODE			(1<<  8)
+#define MSCL_CFG_SOFT_RESET			(1<<  1)
+#define MSCL_CFG_START_CMD			(1<<  0)
+
+/* m2m-scaler interrupt enable */
+#define MSCL_INT_EN				0x08
+#define MSCL_INT_EN_DEFAULT			0x81ffffff
+#define MSCL_INT_EN_TIMEOUT			(1<<  31)
+#define MSCL_INT_EN_ILLEGAL_BLEND		(1<<  24)
+#define MSCL_INT_EN_ILLEGAL_RATIO		(1<<  23)
+#define MSCL_INT_EN_ILLEGAL_DST_HEIGHT		(1<<  22)
+#define MSCL_INT_EN_ILLEGAL_DST_WIDTH		(1<<  21)
+#define MSCL_INT_EN_ILLEGAL_DST_V_POS		(1<<  20)
+#define MSCL_INT_EN_ILLEGAL_DST_H_POS		(1<<  19)
+#define MSCL_INT_EN_ILLEGAL_DST_C_SPAN		(1<<  18)
+#define MSCL_INT_EN_ILLEGAL_DST_Y_SPAN		(1<<  17)
+#define MSCL_INT_EN_ILLEGAL_DST_CR_BASE		(1<<  16)
+#define MSCL_INT_EN_ILLEGAL_DST_CB_BASE		(1<<  15)
+#define MSCL_INT_EN_ILLEGAL_DST_Y_BASE		(1<<  14)
+#define MSCL_INT_EN_ILLEGAL_DST_COLOR		(1<<  13)
+#define MSCL_INT_EN_ILLEGAL_SRC_HEIGHT		(1<<  12)
+#define MSCL_INT_EN_ILLEGAL_SRC_WIDTH		(1<<  11)
+#define MSCL_INT_EN_ILLEGAL_SRC_CV_POS		(1<<  10)
+#define MSCL_INT_EN_ILLEGAL_SRC_CH_POS		(1<<  9)
+#define MSCL_INT_EN_ILLEGAL_SRC_YV_POS		(1<<  8)
+#define MSCL_INT_EN_ILLEGAL_SRC_YH_POS		(1<<  7)
+#define MSCL_INT_EN_ILLEGAL_SRC_C_SPAN		(1<<  6)
+#define MSCL_INT_EN_ILLEGAL_SRC_Y_SPAN		(1<<  5)
+#define MSCL_INT_EN_ILLEGAL_SRC_CR_BASE		(1<<  4)
+#define MSCL_INT_EN_ILLEGAL_SRC_CB_BASE		(1<<  3)
+#define MSCL_INT_EN_ILLEGAL_SRC_Y_BASE		(1<<  2)
+#define MSCL_INT_EN_ILLEGAL_SRC_COLOR		(1<<  1)
+#define MSCL_INT_EN_FRAME_END			(1<<  0)
+
+/* m2m-scaler interrupt status */
+#define MSCL_INT_STATUS				0x0c

+#define MSCL_INT_STATUS_CLEAR			(0xffffffff)
+#define MSCL_INT_STATUS_ERROR			(0x81fffffe)

nit: Superfluous braces

+#define MSCL_INT_STATUS_TIMEOUT			(1<<  31)
+#define MSCL_INT_STATUS_ILLEGAL_BLEND		(1<<  24)
+#define MSCL_INT_STATUS_ILLEGAL_RATIO		(1<<  23)
+#define MSCL_INT_STATUS_ILLEGAL_DST_HEIGHT	(1<<  22)
+#define MSCL_INT_STATUS_ILLEGAL_DST_WIDTH	(1<<  21)
+#define MSCL_INT_STATUS_ILLEGAL_DST_V_POS	(1<<  20)
+#define MSCL_INT_STATUS_ILLEGAL_DST_H_POS	(1<<  19)
+#define MSCL_INT_STATUS_ILLEGAL_DST_C_SPAN	(1<<  18)
+#define MSCL_INT_STATUS_ILLEGAL_DST_Y_SPAN	(1<<  17)
+#define MSCL_INT_STATUS_ILLEGAL_DST_CR_BASE	(1<<  16)
+#define MSCL_INT_STATUS_ILLEGAL_DST_CB_BASE	(1<<  15)
+#define MSCL_INT_STATUS_ILLEGAL_DST_Y_BASE	(1<<  14)
+#define MSCL_INT_STATUS_ILLEGAL_DST_COLOR	(1<<  13)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_HEIGHT	(1<<  12)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_WIDTH	(1<<  11)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_CV_POS	(1<<  10)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_CH_POS	(1<<  9)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_YV_POS	(1<<  8)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_YH_POS	(1<<  7)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_C_SPAN	(1<<  6)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_Y_SPAN	(1<<  5)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_CR_BASE	(1<<  4)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_CB_BASE	(1<<  3)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_Y_BASE	(1<<  2)
+#define MSCL_INT_STATUS_ILLEGAL_SRC_COLOR	(1<<  1)
+#define MSCL_INT_STATUS_FRAME_END		(1<<  0)
+
+/* m2m-scaler source format configuration */
+#define MSCL_SRC_CFG				0x10
+#define MSCL_SRC_TILE_EN			(0x1<<  10)
+#define MSCL_SRC_BYTE_SWAP_MASK			(0x3<<  5)
+#define MSCL_SRC_BYTE_SWAP(x)			(((x)&  0x3)<<  5)
+#define MSCL_SRC_COLOR_FORMAT_MASK		(0xf<<  0)
+#define MSCL_SRC_COLOR_FORMAT(x)		(((x)&  0xf)<<  0)

((x) & 0xf) ?

+
+/* m2m-scaler source y-base */
+#define MSCL_SRC_Y_BASE				0x14
+
+/* m2m-scaler source cb-base */
+#define MSCL_SRC_CB_BASE			0x18
+
+/* m2m-scaler source cr-base */
+#define MSCL_SRC_CR_BASE			0x294
+
+/* m2m-scaler source span */
+#define MSCL_SRC_SPAN				0x1c
+#define MSCL_SRC_C_SPAN_MASK			(0x3fff<<  16)
+#define MSCL_SRC_C_SPAN(x)			(((x)&  0x3fff)<<  16)
+#define MSCL_SRC_Y_SPAN_MASK			(0x3fff<<  0)
+#define MSCL_SRC_Y_SPAN(x)			(((x)&  0x3fff)<<  0)

((x) & 0x3fff) ? And there is couple more occurences like this.

+/* m2m-scaler source y-position */
+#define MSCL_SRC_Y_POS				0x20
+#define MSCL_SRC_YH_POS_MASK			(0xffff<<  (16 + 2))
+#define MSCL_SRC_YH_POS(x)			(((x)&  0xffff)<<  (16 + 2))
+#define MSCL_SRC_YV_POS_MASK			(0xffff<<  (0 + 2))
+#define MSCL_SRC_YV_POS(x)			(((x)&  0xffff)<<  (0 + 2))
+
+/* m2m-scaler source width/height */
+#define MSCL_SRC_WH				0x24
+#define MSCL_SRC_WIDTH_MASK			(0x3fff<<  16)
+#define MSCL_SRC_WIDTH(x)			(((x)&  0x3fff)<<  16)
+#define MSCL_SRC_HEIGHT_MASK			(0x3fff<<  0)
+#define MSCL_SRC_HEIGHT(x)			(((x)&  0x3fff)<<  0)

+/* m2m-scaler source c-position */
+#define MSCL_SRC_C_POS				0x28
+#define MSCL_SRC_CH_POS_MASK			(0xffff<<  (16 + 2))
+#define MSCL_SRC_CH_POS(x)			(((x)&  0xffff)<<  (16 + 2))
+#define MSCL_SRC_CV_POS_MASK			(0xffff<<  (0 + 2))
+#define MSCL_SRC_CV_POS(x)			(((x)&  0xffff)<<  (0 + 2))
+
+/* m2m-scaler destination format configuration */
+#define MSCL_DST_CFG				0x30
+#define MSCL_DST_BYTE_SWAP_MASK			(0x3<<  5)
+#define MSCL_DST_BYTE_SWAP(x)			(((x)&  0x3)<<  5)
+#define MSCL_DST_COLOR_FORMAT_MASK		(0xf<<  0)

Is just 0xf, using zero shits like this for me just makes it harder
to read.

+#define MSCL_DST_COLOR_FORMAT(x)		(((x)&  0xf)<<  0)
+
+/* m2m-scaler destination y-base */
+#define MSCL_DST_Y_BASE				0x34
+
+/* m2m-scaler destination cb-base */
+#define MSCL_DST_CB_BASE			0x38
+
+/* m2m-scaler destination cr-base */
+#define MSCL_DST_CR_BASE			0x298
+
+/* m2m-scaler destination span */
+#define MSCL_DST_SPAN				0x3c
+#define MSCL_DST_C_SPAN_MASK			(0x3fff<<  16)
+#define MSCL_DST_C_SPAN(x)			(((x)&  0x3fff)<<  16)
+#define MSCL_DST_Y_SPAN_MASK			(0x3fff<<  0)
+#define MSCL_DST_Y_SPAN(x)			(((x)&  0x3fff)<<  0)
+
+/* m2m-scaler destination width/height */
+#define MSCL_DST_WH				0x40
+#define MSCL_DST_WIDTH_MASK			(0x3fff<<  16)
+#define MSCL_DST_WIDTH(x)			(((x)&  0x3fff)<<  16)
+#define MSCL_DST_HEIGHT_MASK			(0x3fff<<  0)
+#define MSCL_DST_HEIGHT(x)			(((x)&  0x3fff)<<  0)
+
+/* m2m-scaler destination position */
+#define MSCL_DST_POS				0x44
+#define MSCL_DST_H_POS_MASK			(0x3fff<<  16)
+#define MSCL_DST_H_POS(x)			(((x)&  0x3fff)<<  16)
+#define MSCL_DST_V_POS_MASK			(0x3fff<<  0)
+#define MSCL_DST_V_POS(x)			(((x)&  0x3fff)<<  0)
+
+/* m2m-scaler horizontal scale ratio */
+#define MSCL_H_RATIO				0x50
+#define MSCL_H_RATIO_VALUE(x)			(((x)&  0x7ffff)<<  0)
+
+/* m2m-scaler vertical scale ratio */
+#define MSCL_V_RATIO				0x54
+#define MSCL_V_RATIO_VALUE(x)			(((x)&  0x7ffff)<<  0)
+
+/* m2m-scaler rotation config */
+#define MSCL_ROT_CFG				0x58
+#define MSCL_FLIP_X_EN				(1<<  3)
+#define MSCL_FLIP_Y_EN				(1<<  2)
+#define MSCL_ROTMODE_MASK			(0x3<<  0)
+#define MSCL_ROTMODE(x)				(((x)&  0x3)<<  0)
+
+/* m2m-scaler csc coefficients */

+#define MSCL_CSC_COEF_00			0x220
+#define MSCL_CSC_COEF_10			0x224
+#define MSCL_CSC_COEF_20			0x228
+#define MSCL_CSC_COEF_01			0x22C
+#define MSCL_CSC_COEF_11			0x230
+#define MSCL_CSC_COEF_21			0x234
+#define MSCL_CSC_COEF_02			0x238
+#define MSCL_CSC_COEF_12			0x23C
+#define MSCL_CSC_COEF_22			0x240

Why are these needed when below there is a common macro ?

+#define MSCL_CSC_COEF(x, y)			(0x220 + ((x * 12) + (y * 4)))

+/* m2m-scaler dither config */
+#define MSCL_DITH_CFG				0x250
+#define MSCL_DITHER_R_TYPE_MASK			(0x7<<  6)
+#define MSCL_DITHER_R_TYPE(x)			(((x)&  0x7)<<  6)
+#define MSCL_DITHER_G_TYPE_MASK			(0x7<<  3)
+#define MSCL_DITHER_G_TYPE(x)			(((x)&  0x7)<<  3)
+#define MSCL_DITHER_B_TYPE_MASK			(0x7<<  0)
+#define MSCL_DITHER_B_TYPE(x)			(((x)&  0x7)<<  0)
+
+/* m2m-scaler src blend color */
+#define MSCL_SRC_BLEND_COLOR			0x280
+#define MSCL_SRC_COLOR_SEL_INV			(1<<  31)
+#define MSCL_SRC_COLOR_SEL_MASK			(0x3<<  29)
+#define MSCL_SRC_COLOR_SEL(x)			(((x)&  0x3)<<  29)
+#define MSCL_SRC_COLOR_OP_SEL_INV		(1<<  28)
+#define MSCL_SRC_COLOR_OP_SEL_MASK		(0xf<<  24)
+#define MSCL_SRC_COLOR_OP_SEL(x)		(((x)&  0xf)<<  24)
+#define MSCL_SRC_GLOBAL_COLOR0_MASK		(0xff<<  16)
+#define MSCL_SRC_GLOBAL_COLOR0(x)		(((x)&  0xff)<<  16)
+#define MSCL_SRC_GLOBAL_COLOR1_MASK		(0xff<<  8)
+#define MSCL_SRC_GLOBAL_COLOR1(x)		(((x)&  0xff)<<  8)
+#define MSCL_SRC_GLOBAL_COLOR2_MASK		(0xff<<  0)
+#define MSCL_SRC_GLOBAL_COLOR2(x)		(((x)&  0xff)<<  0)
+
+/* m2m-scaler src blend alpha */
+#define MSCL_SRC_BLEND_ALPHA			0x284
+#define MSCL_SRC_ALPHA_SEL_INV			(1<<  31)
+#define MSCL_SRC_ALPHA_SEL_MASK			(0x3<<  29)
+#define MSCL_SRC_ALPHA_SEL(x)			(((x)&  0x3)<<  29)
+#define MSCL_SRC_ALPHA_OP_SEL_INV		(1<<  28)
+#define MSCL_SRC_ALPHA_OP_SEL_MASK		(0xf<<  24)
+#define MSCL_SRC_ALPHA_OP_SEL(x)		(((x)&  0xf)<<  24)
+#define MSCL_SRC_GLOBAL_ALPHA_MASK		(0xff<<  0)
+#define MSCL_SRC_GLOBAL_ALPHA(x)		(((x)&  0xff)<<  0)
+
+/* m2m-scaler dst blend color */
+#define MSCL_DST_BLEND_COLOR			0x288
+#define MSCL_DST_COLOR_SEL_INV			(1<<  31)
+#define MSCL_DST_COLOR_SEL_MASK			(0x3<<  29)
+#define MSCL_DST_COLOR_SEL(x)			(((x)&  0x3)<<  29)
+#define MSCL_DST_COLOR_OP_SEL_INV		(1<<  28)
+#define MSCL_DST_COLOR_OP_SEL_MASK		(0xf<<  24)
+#define MSCL_DST_COLOR_OP_SEL(x)		(((x)&  0xf)<<  24)
+#define MSCL_DST_GLOBAL_COLOR0_MASK		(0xff<<  16)
+#define MSCL_DST_GLOBAL_COLOR0(x)		(((x)&  0xff)<<  16)
+#define MSCL_DST_GLOBAL_COLOR1_MASK		(0xff<<  8)
+#define MSCL_DST_GLOBAL_COLOR1(x)		(((x)&  0xff)<<  8)
+#define MSCL_DST_GLOBAL_COLOR2_MASK		(0xff<<  0)
+#define MSCL_DST_GLOBAL_COLOR2(x)		(((x)&  0xff)<<  0)
+
+/* m2m-scaler dst blend alpha */
+#define MSCL_DST_BLEND_ALPHA			0x28C
+#define MSCL_DST_ALPHA_SEL_INV			(1<<  31)
+#define MSCL_DST_ALPHA_SEL_MASK			(0x3<<  29)
+#define MSCL_DST_ALPHA_SEL(x)			(((x)&  0x3)<<  29)
+#define MSCL_DST_ALPHA_OP_SEL_INV		(1<<  28)
+#define MSCL_DST_ALPHA_OP_SEL_MASK		(0xf<<  24)
+#define MSCL_DST_ALPHA_OP_SEL(x)		(((x)&  0xf)<<  24)
+#define MSCL_DST_GLOBAL_ALPHA_MASK		(0xff<<  0)
+#define MSCL_DST_GLOBAL_ALPHA(x)		(((x)&  0xff)<<  0)
+
+/* m2m-scaler fill color */
+#define MSCL_FILL_COLOR				0x290
+#define MSCL_FILL_ALPHA_MASK			(0xff<<  24)
+#define MSCL_FILL_ALPHA(x)			(((x)&  0xff)<<  24)
+#define MSCL_FILL_COLOR0_MASK			(0xff<<  16)
+#define MSCL_FILL_COLOR0(x)			(((x)&  0xff)<<  16)
+#define MSCL_FILL_COLOR1_MASK			(0xff<<  8)
+#define MSCL_FILL_COLOR1(x)			(((x)&  0xff)<<  8)
+#define MSCL_FILL_COLOR2_MASK			(0xff<<  0)
+#define MSCL_FILL_COLOR2(x)			(((x)&  0xff)<<  0)

Seems like there is no user interface exposed for the blending control
in this driver yet.

+/* m2m-scaler address queue config */
+#define MSCL_ADDR_QUEUE_CONFIG			0x2a0
+#define MSCL_ADDR_QUEUE_RST			(1<<  0)
+
+/* arbitrary r/w register and reg-value to check soft reset is success */

How about:

/* Arbitrary R/W register address and value to check if soft reset succeeded */

?
+#define MSCL_CFG_SOFT_RESET_CHECK_REG		MSCL_SRC_CFG
+#define MSCL_CFG_SOFT_RESET_CHECK_VAL		0x3
+
+#endif /* REGS_MSCL_H_ */

--
Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux