linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] rcar-vin: Add support for RGB formats with alpha component
@ 2019-06-13  0:04 Niklas Söderlund
  2019-06-13  0:04 ` [PATCH v2 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB Niklas Söderlund
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Niklas Söderlund @ 2019-06-13  0:04 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund

Hi,

This small series adds support for two new pixel formats for the
rcar-vin driver; V4L2_PIX_FMT_ARGB555 and V4L2_PIX_FMT_ABGR32. Both
formats have an alpha component so a new standard control is also added
to control its value, V4L2_CID_ALPHA_COMPONENT.

The series is based on the latest media-tree and is tested on both
Renesas Gen2 and Gen3 hardware without any regressions found.

Patch 1/3 fixes a badly named register name, 2/3 adds the new control
and finally 3/3 adds the two new pixel formats.

Niklas Söderlund (3):
  rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB
  rcar-vin: Add control for alpha component
  rcar-vin: Add support for RGB formats with alpha component

 drivers/media/platform/rcar-vin/rcar-core.c | 53 ++++++++++++++++++++-
 drivers/media/platform/rcar-vin/rcar-dma.c  | 39 ++++++++++++++-
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  8 ++++
 drivers/media/platform/rcar-vin/rcar-vin.h  |  5 ++
 4 files changed, 101 insertions(+), 4 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB
  2019-06-13  0:04 [PATCH v2 0/3] rcar-vin: Add support for RGB formats with alpha component Niklas Söderlund
@ 2019-06-13  0:04 ` Niklas Söderlund
  2019-06-13  0:04 ` [PATCH v2 2/3] rcar-vin: Add control for alpha component Niklas Söderlund
  2019-06-13  0:04 ` [PATCH v2 3/3] rcar-vin: Add support for RGB formats with " Niklas Söderlund
  2 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2019-06-13  0:04 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media
  Cc: linux-renesas-soc, Niklas Söderlund, Ulrich Hecht

The value have nothing to do with ARGB1555, it controls if the alpha
component should be filled in for ARGB1555 or ARGB888. Rename it to
reflect this.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 91ab064404a185af..2d146ecf93d66ad5 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -114,7 +114,7 @@
 #define VNDMR_EXRGB		(1 << 8)
 #define VNDMR_BPSM		(1 << 4)
 #define VNDMR_DTMD_YCSEP	(1 << 1)
-#define VNDMR_DTMD_ARGB1555	(1 << 0)
+#define VNDMR_DTMD_ARGB		(1 << 0)
 
 /* Video n Data Mode Register 2 bits */
 #define VNDMR2_VPS		(1 << 30)
@@ -721,7 +721,7 @@ static int rvin_setup(struct rvin_dev *vin)
 		output_is_yuv = true;
 		break;
 	case V4L2_PIX_FMT_XRGB555:
-		dmr = VNDMR_DTMD_ARGB1555;
+		dmr = VNDMR_DTMD_ARGB;
 		break;
 	case V4L2_PIX_FMT_RGB565:
 		dmr = 0;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] rcar-vin: Add control for alpha component
  2019-06-13  0:04 [PATCH v2 0/3] rcar-vin: Add support for RGB formats with alpha component Niklas Söderlund
  2019-06-13  0:04 ` [PATCH v2 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB Niklas Söderlund
@ 2019-06-13  0:04 ` Niklas Söderlund
  2019-06-13  0:04 ` [PATCH v2 3/3] rcar-vin: Add support for RGB formats with " Niklas Söderlund
  2 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2019-06-13  0:04 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund

In preparation to adding support for RGB pixel formats with an alpha
component add a control to allow the user to control which alpha value
should be used.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 53 ++++++++++++++++++++-
 drivers/media/platform/rcar-vin/rcar-dma.c  |  5 ++
 drivers/media/platform/rcar-vin/rcar-vin.h  |  5 ++
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 64f9cf790445d14e..ee6e6cb39c749675 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -389,6 +389,28 @@ static void rvin_group_put(struct rvin_dev *vin)
 	kref_put(&group->refcount, rvin_group_release);
 }
 
+/* -----------------------------------------------------------------------------
+ * Controls
+ */
+
+static int rvin_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct rvin_dev *vin =
+		container_of(ctrl->handler, struct rvin_dev, ctrl_handler);
+
+	switch (ctrl->id) {
+	case V4L2_CID_ALPHA_COMPONENT:
+		rvin_set_alpha(vin, ctrl->val);
+		break;
+	}
+
+	return 0;
+}
+
+static const struct v4l2_ctrl_ops rvin_ctrl_ops = {
+	.s_ctrl = rvin_s_ctrl,
+};
+
 /* -----------------------------------------------------------------------------
  * Async notifier
  */
@@ -478,6 +500,15 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
 	if (ret < 0)
 		return ret;
 
+	v4l2_ctrl_new_std(&vin->ctrl_handler, &rvin_ctrl_ops,
+			  V4L2_CID_ALPHA_COMPONENT, 0, 255, 1, 255);
+
+	if (vin->ctrl_handler.error) {
+		ret = vin->ctrl_handler.error;
+		v4l2_ctrl_handler_free(&vin->ctrl_handler);
+		return ret;
+	}
+
 	ret = v4l2_ctrl_add_handler(&vin->ctrl_handler, subdev->ctrl_handler,
 				    NULL, true);
 	if (ret < 0) {
@@ -870,6 +901,21 @@ static int rvin_mc_init(struct rvin_dev *vin)
 	if (ret)
 		rvin_group_put(vin);
 
+	ret = v4l2_ctrl_handler_init(&vin->ctrl_handler, 1);
+	if (ret < 0)
+		return ret;
+
+	v4l2_ctrl_new_std(&vin->ctrl_handler, &rvin_ctrl_ops,
+			  V4L2_CID_ALPHA_COMPONENT, 0, 255, 1, 255);
+
+	if (vin->ctrl_handler.error) {
+		ret = vin->ctrl_handler.error;
+		v4l2_ctrl_handler_free(&vin->ctrl_handler);
+		return ret;
+	}
+
+	vin->vdev.ctrl_handler = &vin->ctrl_handler;
+
 	return ret;
 }
 
@@ -1245,6 +1291,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
 
 	vin->dev = &pdev->dev;
 	vin->info = of_device_get_match_data(&pdev->dev);
+	vin->alpha = 0xff;
 
 	/*
 	 * Special care is needed on r8a7795 ES1.x since it
@@ -1288,6 +1335,8 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	return 0;
 
 error_group_unregister:
+	v4l2_ctrl_handler_free(&vin->ctrl_handler);
+
 	if (vin->info->use_mc) {
 		mutex_lock(&vin->group->lock);
 		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
@@ -1323,10 +1372,10 @@ static int rcar_vin_remove(struct platform_device *pdev)
 		}
 		mutex_unlock(&vin->group->lock);
 		rvin_group_put(vin);
-	} else {
-		v4l2_ctrl_handler_free(&vin->ctrl_handler);
 	}
 
+	v4l2_ctrl_handler_free(&vin->ctrl_handler);
+
 	rvin_dma_unregister(vin);
 
 	return 0;
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 2d146ecf93d66ad5..4e991cce5fb56a90 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -1343,3 +1343,8 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
 
 	return 0;
 }
+
+void rvin_set_alpha(struct rvin_dev *vin, unsigned int alpha)
+{
+	vin->alpha = alpha;
+}
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 0b13b34d03e3dce4..365dfde06ec25add 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -178,6 +178,8 @@ struct rvin_info {
  * @compose:		active composing
  * @source:		active size of the video source
  * @std:		active video standard of the video source
+ *
+ * @alpha:		Alpha component to fill in for supported pixel formats
  */
 struct rvin_dev {
 	struct device *dev;
@@ -215,6 +217,8 @@ struct rvin_dev {
 	struct v4l2_rect compose;
 	struct v4l2_rect source;
 	v4l2_std_id std;
+
+	unsigned int alpha;
 };
 
 #define vin_to_source(vin)		((vin)->parallel->subdev)
@@ -266,5 +270,6 @@ const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat);
 void rvin_crop_scale_comp(struct rvin_dev *vin);
 
 int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel);
+void rvin_set_alpha(struct rvin_dev *vin, unsigned int alpha);
 
 #endif
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] rcar-vin: Add support for RGB formats with alpha component
  2019-06-13  0:04 [PATCH v2 0/3] rcar-vin: Add support for RGB formats with alpha component Niklas Söderlund
  2019-06-13  0:04 ` [PATCH v2 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB Niklas Söderlund
  2019-06-13  0:04 ` [PATCH v2 2/3] rcar-vin: Add control for alpha component Niklas Söderlund
@ 2019-06-13  0:04 ` Niklas Söderlund
  2019-06-17 14:33   ` Laurent Pinchart
  2 siblings, 1 reply; 6+ messages in thread
From: Niklas Söderlund @ 2019-06-13  0:04 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund

The R-Car VIN module supports V4L2_PIX_FMT_ARGB555 and
V4L2_PIX_FMT_ABGR32 pixel formats. Add the hardware register setup and
allow the alpha component to be changed while streaming using the
V4L2_CID_ALPHA_COMPONENT control.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-dma.c  | 30 +++++++++++++++++++++
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  8 ++++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 4e991cce5fb56a90..5c0ed27c5d05dd45 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -111,8 +111,11 @@
 #define VNIE_EFE		(1 << 1)
 
 /* Video n Data Mode Register bits */
+#define VNDMR_A8BIT(n)		((n & 0xff) << 24)
+#define VNDMR_A8BIT_MASK	(0xff << 24)
 #define VNDMR_EXRGB		(1 << 8)
 #define VNDMR_BPSM		(1 << 4)
+#define VNDMR_ABIT		(1 << 2)
 #define VNDMR_DTMD_YCSEP	(1 << 1)
 #define VNDMR_DTMD_ARGB		(1 << 0)
 
@@ -730,6 +733,12 @@ static int rvin_setup(struct rvin_dev *vin)
 		/* Note: not supported on M1 */
 		dmr = VNDMR_EXRGB;
 		break;
+	case V4L2_PIX_FMT_ARGB555:
+		dmr = (vin->alpha ? VNDMR_ABIT : 0) | VNDMR_DTMD_ARGB;
+		break;
+	case V4L2_PIX_FMT_ABGR32:
+		dmr = VNDMR_A8BIT(vin->alpha) | VNDMR_EXRGB | VNDMR_DTMD_ARGB;
+		break;
 	default:
 		vin_err(vin, "Invalid pixelformat (0x%x)\n",
 			vin->format.pixelformat);
@@ -1346,5 +1355,26 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
 
 void rvin_set_alpha(struct rvin_dev *vin, unsigned int alpha)
 {
+	u32 dmr;
+
 	vin->alpha = alpha;
+
+	if (vin->state == STOPPED)
+		return;
+
+	switch (vin->format.pixelformat) {
+	case V4L2_PIX_FMT_ARGB555:
+		dmr = rvin_read(vin, VNDMR_REG) & ~VNDMR_ABIT;
+		if (vin->alpha)
+			dmr |= VNDMR_ABIT;
+		break;
+	case V4L2_PIX_FMT_ABGR32:
+		dmr = rvin_read(vin, VNDMR_REG) & ~VNDMR_A8BIT_MASK;
+		dmr |= VNDMR_A8BIT(vin->alpha);
+		break;
+	default:
+		return;
+	}
+
+	rvin_write(vin, dmr,  VNDMR_REG);
 }
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 7cbdcbf9b090c638..bb2900f5d000f9a6 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -54,6 +54,14 @@ static const struct rvin_video_format rvin_formats[] = {
 		.fourcc			= V4L2_PIX_FMT_XBGR32,
 		.bpp			= 4,
 	},
+	{
+		.fourcc			= V4L2_PIX_FMT_ARGB555,
+		.bpp			= 2,
+	},
+	{
+		.fourcc			= V4L2_PIX_FMT_ABGR32,
+		.bpp			= 4,
+	},
 };
 
 const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat)
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] rcar-vin: Add support for RGB formats with alpha component
  2019-06-13  0:04 ` [PATCH v2 3/3] rcar-vin: Add support for RGB formats with " Niklas Söderlund
@ 2019-06-17 14:33   ` Laurent Pinchart
  2019-06-17 14:40     ` Niklas Söderlund
  0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2019-06-17 14:33 UTC (permalink / raw)
  To: Niklas Söderlund; +Cc: linux-media, linux-renesas-soc

Hi Niklas,

Thank you for the patch.

On Thu, Jun 13, 2019 at 02:04:39AM +0200, Niklas Söderlund wrote:
> The R-Car VIN module supports V4L2_PIX_FMT_ARGB555 and
> V4L2_PIX_FMT_ABGR32 pixel formats. Add the hardware register setup and
> allow the alpha component to be changed while streaming using the
> V4L2_CID_ALPHA_COMPONENT control.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c  | 30 +++++++++++++++++++++
>  drivers/media/platform/rcar-vin/rcar-v4l2.c |  8 ++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index 4e991cce5fb56a90..5c0ed27c5d05dd45 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -111,8 +111,11 @@
>  #define VNIE_EFE		(1 << 1)
>  
>  /* Video n Data Mode Register bits */
> +#define VNDMR_A8BIT(n)		((n & 0xff) << 24)
> +#define VNDMR_A8BIT_MASK	(0xff << 24)
>  #define VNDMR_EXRGB		(1 << 8)
>  #define VNDMR_BPSM		(1 << 4)
> +#define VNDMR_ABIT		(1 << 2)
>  #define VNDMR_DTMD_YCSEP	(1 << 1)
>  #define VNDMR_DTMD_ARGB		(1 << 0)
>  
> @@ -730,6 +733,12 @@ static int rvin_setup(struct rvin_dev *vin)
>  		/* Note: not supported on M1 */
>  		dmr = VNDMR_EXRGB;
>  		break;
> +	case V4L2_PIX_FMT_ARGB555:
> +		dmr = (vin->alpha ? VNDMR_ABIT : 0) | VNDMR_DTMD_ARGB;
> +		break;
> +	case V4L2_PIX_FMT_ABGR32:
> +		dmr = VNDMR_A8BIT(vin->alpha) | VNDMR_EXRGB | VNDMR_DTMD_ARGB;
> +		break;
>  	default:
>  		vin_err(vin, "Invalid pixelformat (0x%x)\n",
>  			vin->format.pixelformat);
> @@ -1346,5 +1355,26 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
>  
>  void rvin_set_alpha(struct rvin_dev *vin, unsigned int alpha)
>  {
> +	u32 dmr;
> +
>  	vin->alpha = alpha;
> +
> +	if (vin->state == STOPPED)

The state is protected by the vin->qlock spinlock. Is it safe to check
it here without holding the spinlock ? The answer may be yes if you can
guarantee that no code patch will race except for the IRQ handler, and
guarantee that the race with the IRQ handler isn't an issue.

Additionally, what happens if the control is set and streaming is then
started ? I don't see in call to v4l2_ctrl_handler_setup() in 2/3 or
3/3.

> +		return;
> +
> +	switch (vin->format.pixelformat) {
> +	case V4L2_PIX_FMT_ARGB555:
> +		dmr = rvin_read(vin, VNDMR_REG) & ~VNDMR_ABIT;
> +		if (vin->alpha)
> +			dmr |= VNDMR_ABIT;
> +		break;
> +	case V4L2_PIX_FMT_ABGR32:
> +		dmr = rvin_read(vin, VNDMR_REG) & ~VNDMR_A8BIT_MASK;
> +		dmr |= VNDMR_A8BIT(vin->alpha);
> +		break;
> +	default:
> +		return;
> +	}
> +
> +	rvin_write(vin, dmr,  VNDMR_REG);
>  }
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 7cbdcbf9b090c638..bb2900f5d000f9a6 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -54,6 +54,14 @@ static const struct rvin_video_format rvin_formats[] = {
>  		.fourcc			= V4L2_PIX_FMT_XBGR32,
>  		.bpp			= 4,
>  	},
> +	{
> +		.fourcc			= V4L2_PIX_FMT_ARGB555,
> +		.bpp			= 2,
> +	},
> +	{
> +		.fourcc			= V4L2_PIX_FMT_ABGR32,
> +		.bpp			= 4,
> +	},
>  };
>  
>  const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] rcar-vin: Add support for RGB formats with alpha component
  2019-06-17 14:33   ` Laurent Pinchart
@ 2019-06-17 14:40     ` Niklas Söderlund
  0 siblings, 0 replies; 6+ messages in thread
From: Niklas Söderlund @ 2019-06-17 14:40 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc

Hi Laurent,

Thanks for your feedback.

On 2019-06-17 17:33:41 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Thu, Jun 13, 2019 at 02:04:39AM +0200, Niklas Söderlund wrote:
> > The R-Car VIN module supports V4L2_PIX_FMT_ARGB555 and
> > V4L2_PIX_FMT_ABGR32 pixel formats. Add the hardware register setup and
> > allow the alpha component to be changed while streaming using the
> > V4L2_CID_ALPHA_COMPONENT control.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-dma.c  | 30 +++++++++++++++++++++
> >  drivers/media/platform/rcar-vin/rcar-v4l2.c |  8 ++++++
> >  2 files changed, 38 insertions(+)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index 4e991cce5fb56a90..5c0ed27c5d05dd45 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -111,8 +111,11 @@
> >  #define VNIE_EFE		(1 << 1)
> >  
> >  /* Video n Data Mode Register bits */
> > +#define VNDMR_A8BIT(n)		((n & 0xff) << 24)
> > +#define VNDMR_A8BIT_MASK	(0xff << 24)
> >  #define VNDMR_EXRGB		(1 << 8)
> >  #define VNDMR_BPSM		(1 << 4)
> > +#define VNDMR_ABIT		(1 << 2)
> >  #define VNDMR_DTMD_YCSEP	(1 << 1)
> >  #define VNDMR_DTMD_ARGB		(1 << 0)
> >  
> > @@ -730,6 +733,12 @@ static int rvin_setup(struct rvin_dev *vin)
> >  		/* Note: not supported on M1 */
> >  		dmr = VNDMR_EXRGB;
> >  		break;
> > +	case V4L2_PIX_FMT_ARGB555:
> > +		dmr = (vin->alpha ? VNDMR_ABIT : 0) | VNDMR_DTMD_ARGB;
> > +		break;
> > +	case V4L2_PIX_FMT_ABGR32:
> > +		dmr = VNDMR_A8BIT(vin->alpha) | VNDMR_EXRGB | VNDMR_DTMD_ARGB;
> > +		break;
> >  	default:
> >  		vin_err(vin, "Invalid pixelformat (0x%x)\n",
> >  			vin->format.pixelformat);
> > @@ -1346,5 +1355,26 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
> >  
> >  void rvin_set_alpha(struct rvin_dev *vin, unsigned int alpha)
> >  {
> > +	u32 dmr;
> > +
> >  	vin->alpha = alpha;
> > +
> > +	if (vin->state == STOPPED)
> 
> The state is protected by the vin->qlock spinlock. Is it safe to check
> it here without holding the spinlock ? The answer may be yes if you can
> guarantee that no code patch will race except for the IRQ handler, and
> guarantee that the race with the IRQ handler isn't an issue.

This is just a optimization to not try and write to the hardware if it's 
stopped and switched off. I assume this could race and a lock of 
vin->qlock could be added, if races worst case it writes the alpha value 
to HW when it don't need to. I will add the lock in the next version.

> 
> Additionally, what happens if the control is set and streaming is then
> started ? I don't see in call to v4l2_ctrl_handler_setup() in 2/3 or
> 3/3.

This is a good point, I have recently reworked part of the driver for 
gen2 which already had controls without considering gen3 will gain 
controls with this series. I will fix this and send a new version.

> 
> > +		return;
> > +
> > +	switch (vin->format.pixelformat) {
> > +	case V4L2_PIX_FMT_ARGB555:
> > +		dmr = rvin_read(vin, VNDMR_REG) & ~VNDMR_ABIT;
> > +		if (vin->alpha)
> > +			dmr |= VNDMR_ABIT;
> > +		break;
> > +	case V4L2_PIX_FMT_ABGR32:
> > +		dmr = rvin_read(vin, VNDMR_REG) & ~VNDMR_A8BIT_MASK;
> > +		dmr |= VNDMR_A8BIT(vin->alpha);
> > +		break;
> > +	default:
> > +		return;
> > +	}
> > +
> > +	rvin_write(vin, dmr,  VNDMR_REG);
> >  }
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > index 7cbdcbf9b090c638..bb2900f5d000f9a6 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > @@ -54,6 +54,14 @@ static const struct rvin_video_format rvin_formats[] = {
> >  		.fourcc			= V4L2_PIX_FMT_XBGR32,
> >  		.bpp			= 4,
> >  	},
> > +	{
> > +		.fourcc			= V4L2_PIX_FMT_ARGB555,
> > +		.bpp			= 2,
> > +	},
> > +	{
> > +		.fourcc			= V4L2_PIX_FMT_ABGR32,
> > +		.bpp			= 4,
> > +	},
> >  };
> >  
> >  const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat)
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-17 14:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-13  0:04 [PATCH v2 0/3] rcar-vin: Add support for RGB formats with alpha component Niklas Söderlund
2019-06-13  0:04 ` [PATCH v2 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB Niklas Söderlund
2019-06-13  0:04 ` [PATCH v2 2/3] rcar-vin: Add control for alpha component Niklas Söderlund
2019-06-13  0:04 ` [PATCH v2 3/3] rcar-vin: Add support for RGB formats with " Niklas Söderlund
2019-06-17 14:33   ` Laurent Pinchart
2019-06-17 14:40     ` Niklas Söderlund

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).