* [PATCH v2 0/2] media: rcar-vin: Mask access to VNCSI_IFMD register
@ 2020-11-16 11:04 Jacopo Mondi
2020-11-16 11:04 ` [PATCH v2 1/2] media: rcar-vin: Remove unused macro Jacopo Mondi
2020-11-16 11:04 ` [PATCH v2 2/2] media: rcar-vin: Mask VNCSI_IFMD register Jacopo Mondi
0 siblings, 2 replies; 5+ messages in thread
From: Jacopo Mondi @ 2020-11-16 11:04 UTC (permalink / raw)
To: koji.matsuoka.xm, niklas.soderlund+renesas, laurent.pinchart
Cc: Jacopo Mondi, linux-media, linux-renesas-soc
As reported in patch 2/2 commit message the the VNCSI_IFMD register
has the following limitations according to chip manual revision 2.20
- V3M, V3H and E3 do not support the DES1 field has they do not feature
a CSI20 receiver.
- D3 only supports parallel input, and the whole register shall always
be written as 0.
This patch upports the BSP change commit f54697394457
("media: rcar-vin: Fix VnCSI_IFMD register access for r8a77990") from
Koji Matsuoka
Tested on r-car E3 Ebisu.
v1 -> v2:
- Inspect the channel routing table to deduce the availability of DES1/DES0
bits as suggested by Niklas.
Thanks
j
Jacopo Mondi (2):
media: rcar-vin: Remove unused macro
media: rcar-vin: Mask VNCSI_IFMD register
drivers/media/platform/rcar-vin/rcar-dma.c | 27 ++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
--
2.29.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] media: rcar-vin: Remove unused macro
2020-11-16 11:04 [PATCH v2 0/2] media: rcar-vin: Mask access to VNCSI_IFMD register Jacopo Mondi
@ 2020-11-16 11:04 ` Jacopo Mondi
2020-11-16 11:04 ` [PATCH v2 2/2] media: rcar-vin: Mask VNCSI_IFMD register Jacopo Mondi
1 sibling, 0 replies; 5+ messages in thread
From: Jacopo Mondi @ 2020-11-16 11:04 UTC (permalink / raw)
To: koji.matsuoka.xm, niklas.soderlund+renesas, laurent.pinchart
Cc: Jacopo Mondi, linux-media, linux-renesas-soc
The VNCSI_IFMD_CSI_CHSEL_MASK is not used: remove it.
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
drivers/media/platform/rcar-vin/rcar-dma.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index f6e84fa29bce..378514a75bc2 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -137,7 +137,6 @@
#define VNCSI_IFMD_DES1 (1 << 26)
#define VNCSI_IFMD_DES0 (1 << 25)
#define VNCSI_IFMD_CSI_CHSEL(n) (((n) & 0xf) << 0)
-#define VNCSI_IFMD_CSI_CHSEL_MASK 0xf
/* Video n scaling control register (Gen3) */
#define VNUDS_CTRL_AMD (1 << 30)
--
2.29.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] media: rcar-vin: Mask VNCSI_IFMD register
2020-11-16 11:04 [PATCH v2 0/2] media: rcar-vin: Mask access to VNCSI_IFMD register Jacopo Mondi
2020-11-16 11:04 ` [PATCH v2 1/2] media: rcar-vin: Remove unused macro Jacopo Mondi
@ 2020-11-16 11:04 ` Jacopo Mondi
2020-11-23 15:31 ` Niklas Söderlund
1 sibling, 1 reply; 5+ messages in thread
From: Jacopo Mondi @ 2020-11-16 11:04 UTC (permalink / raw)
To: koji.matsuoka.xm, niklas.soderlund+renesas, laurent.pinchart
Cc: Jacopo Mondi, linux-media, linux-renesas-soc
The VNCSI_IFMD register controls the data expansion mode and the
channel routing between the CSI-2 receivers and VIN instances.
According to the chip manual revision 2.20 not all fields are available
for all the SoCs:
- V3M, V3H and E3 do not support the DES1 field has they do not feature
a CSI20 receiver.
- D3 only supports parallel input, and the whole register shall always
be written as 0.
Inspect the per-SoC channel routing table where the available CSI-2
instances are reported and configure VNCSI_IFMD accordingly.
This patch upports the BSP change commit f54697394457
("media: rcar-vin: Fix VnCSI_IFMD register access for r8a77990")
Suggested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
drivers/media/platform/rcar-vin/rcar-dma.c | 26 +++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 378514a75bc2..ab6818b34e5a 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -1570,7 +1570,9 @@ int rvin_dma_register(struct rvin_dev *vin, int irq)
*/
int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
{
- u32 ifmd, vnmc;
+ const struct rvin_group_route *route;
+ u32 ifmd = 0;
+ u32 vnmc;
int ret;
ret = pm_runtime_get_sync(vin->dev);
@@ -1583,9 +1585,27 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
vnmc = rvin_read(vin, VNMC_REG);
rvin_write(vin, vnmc & ~VNMC_VUP, VNMC_REG);
- ifmd = VNCSI_IFMD_DES1 | VNCSI_IFMD_DES0 | VNCSI_IFMD_CSI_CHSEL(chsel);
+ /*
+ * Set data expansion mode to "pad with 0s" by inspecting the routes
+ * table to find out which bit fields are available in the IFMD
+ * register. IFMD_DES1 controls data expansion mode for CSI20/21,
+ * IFMD_DES0 controls data expansion mode for CSI40/41.
+ */
+ for (route = vin->info->routes; route->mask; route++) {
+ /* CSI21 is only available in r8a7795es1 and not documented. */
+ if (route->csi == RVIN_CSI20 || route->csi == RVIN_CSI21)
+ ifmd |= VNCSI_IFMD_DES1;
+ else
+ ifmd |= VNCSI_IFMD_DES0;
- rvin_write(vin, ifmd, VNCSI_IFMD_REG);
+ /* If both have been set stop looping. */
+ if (ifmd == (VNCSI_IFMD_DES0 | VNCSI_IFMD_DES1))
+ break;
+ }
+ if (ifmd) {
+ ifmd |= VNCSI_IFMD_CSI_CHSEL(chsel);
+ rvin_write(vin, ifmd, VNCSI_IFMD_REG);
+ }
vin_dbg(vin, "Set IFMD 0x%x\n", ifmd);
--
2.29.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] media: rcar-vin: Mask VNCSI_IFMD register
2020-11-16 11:04 ` [PATCH v2 2/2] media: rcar-vin: Mask VNCSI_IFMD register Jacopo Mondi
@ 2020-11-23 15:31 ` Niklas Söderlund
2020-11-24 21:40 ` Jacopo Mondi
0 siblings, 1 reply; 5+ messages in thread
From: Niklas Söderlund @ 2020-11-23 15:31 UTC (permalink / raw)
To: Jacopo Mondi
Cc: koji.matsuoka.xm, laurent.pinchart, linux-media, linux-renesas-soc
Hi Jacopo,
Thanks for your work.
On 2020-11-16 12:04:28 +0100, Jacopo Mondi wrote:
> The VNCSI_IFMD register controls the data expansion mode and the
> channel routing between the CSI-2 receivers and VIN instances.
>
> According to the chip manual revision 2.20 not all fields are available
> for all the SoCs:
> - V3M, V3H and E3 do not support the DES1 field has they do not feature
> a CSI20 receiver.
> - D3 only supports parallel input, and the whole register shall always
> be written as 0.
>
> Inspect the per-SoC channel routing table where the available CSI-2
> instances are reported and configure VNCSI_IFMD accordingly.
>
> This patch upports the BSP change commit f54697394457
> ("media: rcar-vin: Fix VnCSI_IFMD register access for r8a77990")
I really like this approach, nice work.
>
> Suggested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 26 +++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index 378514a75bc2..ab6818b34e5a 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1570,7 +1570,9 @@ int rvin_dma_register(struct rvin_dev *vin, int irq)
> */
> int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
> {
> - u32 ifmd, vnmc;
> + const struct rvin_group_route *route;
> + u32 ifmd = 0;
> + u32 vnmc;
> int ret;
>
> ret = pm_runtime_get_sync(vin->dev);
> @@ -1583,9 +1585,27 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
> vnmc = rvin_read(vin, VNMC_REG);
> rvin_write(vin, vnmc & ~VNMC_VUP, VNMC_REG);
>
> - ifmd = VNCSI_IFMD_DES1 | VNCSI_IFMD_DES0 | VNCSI_IFMD_CSI_CHSEL(chsel);
> + /*
> + * Set data expansion mode to "pad with 0s" by inspecting the routes
> + * table to find out which bit fields are available in the IFMD
> + * register. IFMD_DES1 controls data expansion mode for CSI20/21,
> + * IFMD_DES0 controls data expansion mode for CSI40/41.
> + */
> + for (route = vin->info->routes; route->mask; route++) {
> + /* CSI21 is only available in r8a7795es1 and not documented. */
I would drop this comment as we do not know what will happen in future
SoCs.
> + if (route->csi == RVIN_CSI20 || route->csi == RVIN_CSI21)
> + ifmd |= VNCSI_IFMD_DES1;
> + else
> + ifmd |= VNCSI_IFMD_DES0;
>
> - rvin_write(vin, ifmd, VNCSI_IFMD_REG);
> + /* If both have been set stop looping. */
> + if (ifmd == (VNCSI_IFMD_DES0 | VNCSI_IFMD_DES1))
> + break;
I would remove the comment here as I think the code is quiet self
explanatory and reading the comment made me think I was missing
something obvious ;-)
> + }
Missing blank line.
With these small nits fixed,
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> + if (ifmd) {
> + ifmd |= VNCSI_IFMD_CSI_CHSEL(chsel);
> + rvin_write(vin, ifmd, VNCSI_IFMD_REG);
> + }
>
> vin_dbg(vin, "Set IFMD 0x%x\n", ifmd);
>
> --
> 2.29.1
>
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] media: rcar-vin: Mask VNCSI_IFMD register
2020-11-23 15:31 ` Niklas Söderlund
@ 2020-11-24 21:40 ` Jacopo Mondi
0 siblings, 0 replies; 5+ messages in thread
From: Jacopo Mondi @ 2020-11-24 21:40 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Jacopo Mondi, koji.matsuoka.xm, laurent.pinchart, linux-media,
linux-renesas-soc
Hi Niklas,
On Mon, Nov 23, 2020 at 04:31:24PM +0100, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2020-11-16 12:04:28 +0100, Jacopo Mondi wrote:
> > The VNCSI_IFMD register controls the data expansion mode and the
> > channel routing between the CSI-2 receivers and VIN instances.
> >
> > According to the chip manual revision 2.20 not all fields are available
> > for all the SoCs:
> > - V3M, V3H and E3 do not support the DES1 field has they do not feature
> > a CSI20 receiver.
> > - D3 only supports parallel input, and the whole register shall always
> > be written as 0.
> >
> > Inspect the per-SoC channel routing table where the available CSI-2
> > instances are reported and configure VNCSI_IFMD accordingly.
> >
> > This patch upports the BSP change commit f54697394457
> > ("media: rcar-vin: Fix VnCSI_IFMD register access for r8a77990")
>
> I really like this approach, nice work.
>
I'm happy you like it, almost like you suggested it in first place,
right ? :)
> >
> > Suggested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > drivers/media/platform/rcar-vin/rcar-dma.c | 26 +++++++++++++++++++---
> > 1 file changed, 23 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index 378514a75bc2..ab6818b34e5a 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -1570,7 +1570,9 @@ int rvin_dma_register(struct rvin_dev *vin, int irq)
> > */
> > int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
> > {
> > - u32 ifmd, vnmc;
> > + const struct rvin_group_route *route;
> > + u32 ifmd = 0;
> > + u32 vnmc;
> > int ret;
> >
> > ret = pm_runtime_get_sync(vin->dev);
> > @@ -1583,9 +1585,27 @@ int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel)
> > vnmc = rvin_read(vin, VNMC_REG);
> > rvin_write(vin, vnmc & ~VNMC_VUP, VNMC_REG);
> >
> > - ifmd = VNCSI_IFMD_DES1 | VNCSI_IFMD_DES0 | VNCSI_IFMD_CSI_CHSEL(chsel);
> > + /*
> > + * Set data expansion mode to "pad with 0s" by inspecting the routes
> > + * table to find out which bit fields are available in the IFMD
> > + * register. IFMD_DES1 controls data expansion mode for CSI20/21,
> > + * IFMD_DES0 controls data expansion mode for CSI40/41.
> > + */
> > + for (route = vin->info->routes; route->mask; route++) {
> > + /* CSI21 is only available in r8a7795es1 and not documented. */
>
> I would drop this comment as we do not know what will happen in future
> SoCs.
>
> > + if (route->csi == RVIN_CSI20 || route->csi == RVIN_CSI21)
> > + ifmd |= VNCSI_IFMD_DES1;
> > + else
> > + ifmd |= VNCSI_IFMD_DES0;
> >
> > - rvin_write(vin, ifmd, VNCSI_IFMD_REG);
> > + /* If both have been set stop looping. */
> > + if (ifmd == (VNCSI_IFMD_DES0 | VNCSI_IFMD_DES1))
> > + break;
>
> I would remove the comment here as I think the code is quiet self
> explanatory and reading the comment made me think I was missing
> something obvious ;-)
>
Correct, I'm breaking the first rule of avoiding superfluous comments.
> > + }
>
> Missing blank line.
Was kind of intentional, but I can add one
>
> With these small nits fixed,
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Thanks
j
>
> > + if (ifmd) {
> > + ifmd |= VNCSI_IFMD_CSI_CHSEL(chsel);
> > + rvin_write(vin, ifmd, VNCSI_IFMD_REG);
> > + }
> >
> > vin_dbg(vin, "Set IFMD 0x%x\n", ifmd);
> >
> > --
> > 2.29.1
> >
>
> --
> Regards,
> Niklas Söderlund
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-24 21:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-16 11:04 [PATCH v2 0/2] media: rcar-vin: Mask access to VNCSI_IFMD register Jacopo Mondi
2020-11-16 11:04 ` [PATCH v2 1/2] media: rcar-vin: Remove unused macro Jacopo Mondi
2020-11-16 11:04 ` [PATCH v2 2/2] media: rcar-vin: Mask VNCSI_IFMD register Jacopo Mondi
2020-11-23 15:31 ` Niklas Söderlund
2020-11-24 21:40 ` Jacopo Mondi
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).