* [PATCH 0/3] rcar-vin: Add support for RGB formats with alpha component @ 2019-05-16 0:47 Niklas Söderlund 2019-05-16 0:47 ` [PATCH 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB Niklas Söderlund ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Niklas Söderlund @ 2019-05-16 0:47 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] 10+ messages in thread
* [PATCH 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB 2019-05-16 0:47 [PATCH 0/3] rcar-vin: Add support for RGB formats with alpha component Niklas Söderlund @ 2019-05-16 0:47 ` Niklas Söderlund 2019-05-16 9:57 ` Ulrich Hecht 2019-05-16 9:58 ` Laurent Pinchart 2019-05-16 0:47 ` [PATCH 2/3] rcar-vin: Add control for alpha component Niklas Söderlund 2019-05-16 0:47 ` [PATCH 3/3] rcar-vin: Add support for RGB formats with " Niklas Söderlund 2 siblings, 2 replies; 10+ messages in thread From: Niklas Söderlund @ 2019-05-16 0:47 UTC (permalink / raw) To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund 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> --- 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] 10+ messages in thread
* Re: [PATCH 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB 2019-05-16 0:47 ` [PATCH 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB Niklas Söderlund @ 2019-05-16 9:57 ` Ulrich Hecht 2019-05-16 9:58 ` Laurent Pinchart 1 sibling, 0 replies; 10+ messages in thread From: Ulrich Hecht @ 2019-05-16 9:57 UTC (permalink / raw) To: Niklas Söderlund, Laurent Pinchart, linux-media; +Cc: linux-renesas-soc > On May 16, 2019 at 2:47 AM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > > > 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> > --- > 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 > Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu> CU Uli ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB 2019-05-16 0:47 ` [PATCH 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB Niklas Söderlund 2019-05-16 9:57 ` Ulrich Hecht @ 2019-05-16 9:58 ` Laurent Pinchart 1 sibling, 0 replies; 10+ messages in thread From: Laurent Pinchart @ 2019-05-16 9:58 UTC (permalink / raw) To: Niklas Söderlund; +Cc: linux-media, linux-renesas-soc Hi Niklas, Thank you for the patch. On Thu, May 16, 2019 at 02:47:44AM +0200, Niklas Söderlund wrote: > 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: 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; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] rcar-vin: Add control for alpha component 2019-05-16 0:47 [PATCH 0/3] rcar-vin: Add support for RGB formats with alpha component Niklas Söderlund 2019-05-16 0:47 ` [PATCH 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB Niklas Söderlund @ 2019-05-16 0:47 ` Niklas Söderlund 2019-05-16 10:01 ` Laurent Pinchart 2019-05-16 0:47 ` [PATCH 3/3] rcar-vin: Add support for RGB formats with " Niklas Söderlund 2 siblings, 1 reply; 10+ messages in thread From: Niklas Söderlund @ 2019-05-16 0:47 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> --- 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] 10+ messages in thread
* Re: [PATCH 2/3] rcar-vin: Add control for alpha component 2019-05-16 0:47 ` [PATCH 2/3] rcar-vin: Add control for alpha component Niklas Söderlund @ 2019-05-16 10:01 ` Laurent Pinchart 2019-05-16 13:04 ` Niklas Söderlund 0 siblings, 1 reply; 10+ messages in thread From: Laurent Pinchart @ 2019-05-16 10:01 UTC (permalink / raw) To: Niklas Söderlund; +Cc: linux-media, linux-renesas-soc Hi Niklas, Thank you for the patch. On Thu, May 16, 2019 at 02:47:45AM +0200, Niklas Söderlund wrote: > 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> > --- > 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); You can set vin->alpha here directly, no need for a rvin_set_alpha() function. > + 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 > + * Do you need a blank line here ? Apart from that the patch looks good to me. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * @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 -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] rcar-vin: Add control for alpha component 2019-05-16 10:01 ` Laurent Pinchart @ 2019-05-16 13:04 ` Niklas Söderlund 0 siblings, 0 replies; 10+ messages in thread From: Niklas Söderlund @ 2019-05-16 13:04 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc Hi Laurent, Thanks for your feedback. On 2019-05-16 13:01:38 +0300, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Thu, May 16, 2019 at 02:47:45AM +0200, Niklas Söderlund wrote: > > 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> > > --- > > 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); > > You can set vin->alpha here directly, no need for a rvin_set_alpha() > function. I think you discovers the reason for this in 3/3. I could set vin->alpha directly here and introduce rvin_set_alpha() in 3/3. I opted for this approach as the reason form not margin 2/3 and 3/3 into a single patch is that I only want the register writes in 3/3. > > > + 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 > > + * > > Do you need a blank line here ? I don't need it, but it make sens in my head on how I group variables in sturct rvin_info. There already exists blank lines in the structure for this grouping of variables thinking. If you are OK with it I would prefer to keep it, but I do not feel strongly about it. > > Apart from that the patch looks good to me. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks! > > > + * @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 > > -- > Regards, > > Laurent Pinchart -- Regards, Niklas Söderlund ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] rcar-vin: Add support for RGB formats with alpha component 2019-05-16 0:47 [PATCH 0/3] rcar-vin: Add support for RGB formats with alpha component Niklas Söderlund 2019-05-16 0:47 ` [PATCH 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB Niklas Söderlund 2019-05-16 0:47 ` [PATCH 2/3] rcar-vin: Add control for alpha component Niklas Söderlund @ 2019-05-16 0:47 ` Niklas Söderlund 2019-05-16 10:08 ` Laurent Pinchart 2 siblings, 1 reply; 10+ messages in thread From: Niklas Söderlund @ 2019-05-16 0:47 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] 10+ messages in thread
* Re: [PATCH 3/3] rcar-vin: Add support for RGB formats with alpha component 2019-05-16 0:47 ` [PATCH 3/3] rcar-vin: Add support for RGB formats with " Niklas Söderlund @ 2019-05-16 10:08 ` Laurent Pinchart 2019-05-16 13:00 ` Niklas Söderlund 0 siblings, 1 reply; 10+ messages in thread From: Laurent Pinchart @ 2019-05-16 10:08 UTC (permalink / raw) To: Niklas Söderlund; +Cc: linux-media, linux-renesas-soc Hi Niklas, Thank you for the patch. On Thu, May 16, 2019 at 02:47:46AM +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) OK, I now see why you added a rvin_set_alpha() function. It makes sense, but I think you need to protect this with a lock to avoid races between stream start and control set. Or are we already protected by a lock the serialises all V4L2 ioctls for the VIN video node ? > { > + 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; Should you cache the DNDMR valid to avoid a hardware read ? > + 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] 10+ messages in thread
* Re: [PATCH 3/3] rcar-vin: Add support for RGB formats with alpha component 2019-05-16 10:08 ` Laurent Pinchart @ 2019-05-16 13:00 ` Niklas Söderlund 0 siblings, 0 replies; 10+ messages in thread From: Niklas Söderlund @ 2019-05-16 13:00 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc Hi Laurent, Thanks for your feedback. On 2019-05-16 13:08:22 +0300, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Thu, May 16, 2019 at 02:47:46AM +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) > > OK, I now see why you added a rvin_set_alpha() function. It makes sense, > but I think you need to protect this with a lock to avoid races between > stream start and control set. Or are we already protected by a lock the > serialises all V4L2 ioctls for the VIN video node ? Yes we are protected by the ioctls lock in __video_do_ioctl(), at least that is my interpretation of the code I have not tried to force a race. > > > { > > + 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; > > Should you cache the DNDMR valid to avoid a hardware read ? It is one possibility, as VNDMR is written to at other locations I would feel better to to this at a later point in time. I'm currently trying to clean up the rcar-vin driver with regard to how it handles formats a bit different in the devnode and media centric code paths. Once that work is complete a generic way to cache register values could be added on top. I suspect there are other registers then VNDMR which could benefit from such a solution. > > > + 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] 10+ messages in thread
end of thread, other threads:[~2019-05-16 13:04 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-16 0:47 [PATCH 0/3] rcar-vin: Add support for RGB formats with alpha component Niklas Söderlund 2019-05-16 0:47 ` [PATCH 1/3] rcar-vin: Rename VNDMR_DTMD_ARGB1555 to VNDMR_DTMD_ARGB Niklas Söderlund 2019-05-16 9:57 ` Ulrich Hecht 2019-05-16 9:58 ` Laurent Pinchart 2019-05-16 0:47 ` [PATCH 2/3] rcar-vin: Add control for alpha component Niklas Söderlund 2019-05-16 10:01 ` Laurent Pinchart 2019-05-16 13:04 ` Niklas Söderlund 2019-05-16 0:47 ` [PATCH 3/3] rcar-vin: Add support for RGB formats with " Niklas Söderlund 2019-05-16 10:08 ` Laurent Pinchart 2019-05-16 13:00 ` 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).