* [PATCH v3] vsp1: fix video output on R8A77970
@ 2018-01-18 14:05 Sergei Shtylyov
2018-02-22 16:26 ` Laurent Pinchart
0 siblings, 1 reply; 8+ messages in thread
From: Sergei Shtylyov @ 2018-01-18 14:05 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, linux-media, linux-renesas-soc
Cc: Sergei Shtylyov
[-- Attachment #1: vsp1-fix-video-output-on-R8A77970-v3.patch --]
[-- Type: text/plain, Size: 2850 bytes --]
Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
and VSP2-D instances") added support for the VSP2-D found in the R-Car
V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
garbage-like line repeated every 8 screen rows. It turns out that R-Car
V3M has the LIF0 buffer attribute register that you need to set to a non-
default value in order to get rid of the output artifacts...
Based on the original (and large) patch by Daisuke Matsushita
<daisuke.matsushita.ns@hitachi.com>.
Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
This patch is against the 'media_tree.git' repo's 'fixes' branch.
Changes in version 3:
- reworded the comment in lif_configure();
- reworded the patch description.
Changes in version 2:
- added a comment before the V3M SoC check;
- fixed indetation in that check;
- reformatted the patch description.
drivers/media/platform/vsp1/vsp1_lif.c | 15 +++++++++++++++
drivers/media/platform/vsp1/vsp1_regs.h | 5 +++++
2 files changed, 20 insertions(+)
Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
===================================================================
--- media_tree.orig/drivers/media/platform/vsp1/vsp1_lif.c
+++ media_tree/drivers/media/platform/vsp1/vsp1_lif.c
@@ -155,6 +155,21 @@ static void lif_configure(struct vsp1_en
(obth << VI6_LIF_CTRL_OBTH_SHIFT) |
(format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
+
+ /*
+ * On R-Car V3M the LIF0 buffer attribute register has to be set
+ * to a non-default value to guarantee proper operation (otherwise
+ * artifacts may appear on the output). The value required by
+ * the manual is not explained but is likely a buffer size or
+ * threshold...
+ */
+ if ((entity->vsp1->version &
+ (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
+ (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
+ vsp1_lif_write(lif, dl, VI6_LIF_LBA,
+ VI6_LIF_LBA_LBA0 |
+ (1536 << VI6_LIF_LBA_LBA1_SHIFT));
+ }
}
static const struct vsp1_entity_operations lif_entity_ops = {
Index: media_tree/drivers/media/platform/vsp1/vsp1_regs.h
===================================================================
--- media_tree.orig/drivers/media/platform/vsp1/vsp1_regs.h
+++ media_tree/drivers/media/platform/vsp1/vsp1_regs.h
@@ -693,6 +693,11 @@
#define VI6_LIF_CSBTH_LBTH_MASK (0x7ff << 0)
#define VI6_LIF_CSBTH_LBTH_SHIFT 0
+#define VI6_LIF_LBA 0x3b0c
+#define VI6_LIF_LBA_LBA0 (1 << 31)
+#define VI6_LIF_LBA_LBA1_MASK (0xfff << 16)
+#define VI6_LIF_LBA_LBA1_SHIFT 16
+
/* -----------------------------------------------------------------------------
* Security Control Registers
*/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] vsp1: fix video output on R8A77970
2018-01-18 14:05 [PATCH v3] vsp1: fix video output on R8A77970 Sergei Shtylyov
@ 2018-02-22 16:26 ` Laurent Pinchart
2018-02-22 16:29 ` Laurent Pinchart
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Laurent Pinchart @ 2018-02-22 16:26 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc
Hi Sergei,
Thank you for the patch.
On Thursday, 18 January 2018 16:05:51 EET Sergei Shtylyov wrote:
> Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
> and VSP2-D instances") added support for the VSP2-D found in the R-Car
> V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
> garbage-like line repeated every 8 screen rows. It turns out that R-Car
> V3M has the LIF0 buffer attribute register that you need to set to a non-
> default value in order to get rid of the output artifacts...
>
> Based on the original (and large) patch by Daisuke Matsushita
> <daisuke.matsushita.ns@hitachi.com>.
>
> Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and
> VSP2-D instances") Signed-off-by: Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com>
>
> ---
> This patch is against the 'media_tree.git' repo's 'fixes' branch.
>
> Changes in version 3:
> - reworded the comment in lif_configure();
> - reworded the patch description.
>
> Changes in version 2:
> - added a comment before the V3M SoC check;
> - fixed indetation in that check;
> - reformatted the patch description.
>
> drivers/media/platform/vsp1/vsp1_lif.c | 15 +++++++++++++++
> drivers/media/platform/vsp1/vsp1_regs.h | 5 +++++
> 2 files changed, 20 insertions(+)
>
> Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
> ===================================================================
> --- media_tree.orig/drivers/media/platform/vsp1/vsp1_lif.c
> +++ media_tree/drivers/media/platform/vsp1/vsp1_lif.c
> @@ -155,6 +155,21 @@ static void lif_configure(struct vsp1_en
> (obth << VI6_LIF_CTRL_OBTH_SHIFT) |
> (format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
> VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
> +
> + /*
> + * On R-Car V3M the LIF0 buffer attribute register has to be set
> + * to a non-default value to guarantee proper operation (otherwise
> + * artifacts may appear on the output). The value required by
> + * the manual is not explained but is likely a buffer size or
> + * threshold...
One period is enough :-)
> + */
> + if ((entity->vsp1->version &
> + (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
> + (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
> + vsp1_lif_write(lif, dl, VI6_LIF_LBA,
> + VI6_LIF_LBA_LBA0 |
> + (1536 << VI6_LIF_LBA_LBA1_SHIFT));
> + }
There's no need for braces or inner parentheses. To make this easier to read I
propose defining a new macro VI6_IP_VERSION_MASK that combines both the model
and SoC. Otherwise this looks good to me,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
I'll post a v4 with that change in reply to this e-mail, please let me know if
you're fine with it.
> }
>
> static const struct vsp1_entity_operations lif_entity_ops = {
> Index: media_tree/drivers/media/platform/vsp1/vsp1_regs.h
> ===================================================================
> --- media_tree.orig/drivers/media/platform/vsp1/vsp1_regs.h
> +++ media_tree/drivers/media/platform/vsp1/vsp1_regs.h
> @@ -693,6 +693,11 @@
> #define VI6_LIF_CSBTH_LBTH_MASK (0x7ff << 0)
> #define VI6_LIF_CSBTH_LBTH_SHIFT 0
>
> +#define VI6_LIF_LBA 0x3b0c
> +#define VI6_LIF_LBA_LBA0 (1 << 31)
> +#define VI6_LIF_LBA_LBA1_MASK (0xfff << 16)
> +#define VI6_LIF_LBA_LBA1_SHIFT 16
> +
> /* ------------------------------------------------------------------------
> * Security Control Registers
> */
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] vsp1: fix video output on R8A77970
2018-02-22 16:26 ` Laurent Pinchart
@ 2018-02-22 16:29 ` Laurent Pinchart
2018-02-22 16:32 ` [PATCH v4] v4l: vsp1: Fix " Laurent Pinchart
2018-02-22 18:42 ` [PATCH v3] vsp1: fix " Sergei Shtylyov
2 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2018-02-22 16:29 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc
Hi Sergei,
On Thursday, 22 February 2018 18:26:20 EET Laurent Pinchart wrote:
> On Thursday, 18 January 2018 16:05:51 EET Sergei Shtylyov wrote:
> > Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
> > and VSP2-D instances") added support for the VSP2-D found in the R-Car
> > V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
> > garbage-like line repeated every 8 screen rows. It turns out that R-Car
> > V3M has the LIF0 buffer attribute register that you need to set to a non-
> > default value in order to get rid of the output artifacts...
> >
> > Based on the original (and large) patch by Daisuke Matsushita
> > <daisuke.matsushita.ns@hitachi.com>.
> >
> > Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and
> > VSP2-D instances") Signed-off-by: Sergei Shtylyov
> > <sergei.shtylyov@cogentembedded.com>
> >
> > ---
> > This patch is against the 'media_tree.git' repo's 'fixes' branch.
> >
> > Changes in version 3:
> > - reworded the comment in lif_configure();
> > - reworded the patch description.
> >
> > Changes in version 2:
> > - added a comment before the V3M SoC check;
> > - fixed indetation in that check;
> > - reformatted the patch description.
> >
> > drivers/media/platform/vsp1/vsp1_lif.c | 15 +++++++++++++++
> > drivers/media/platform/vsp1/vsp1_regs.h | 5 +++++
> > 2 files changed, 20 insertions(+)
> >
> > Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
> > ===================================================================
> > --- media_tree.orig/drivers/media/platform/vsp1/vsp1_lif.c
> > +++ media_tree/drivers/media/platform/vsp1/vsp1_lif.c
> > @@ -155,6 +155,21 @@ static void lif_configure(struct vsp1_en
> >
> > (obth << VI6_LIF_CTRL_OBTH_SHIFT) |
> > (format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
> > VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
> >
> > +
> > + /*
> > + * On R-Car V3M the LIF0 buffer attribute register has to be set
> > + * to a non-default value to guarantee proper operation (otherwise
> > + * artifacts may appear on the output). The value required by
> > + * the manual is not explained but is likely a buffer size or
> > + * threshold...
>
> One period is enough :-)
>
> > + */
> > + if ((entity->vsp1->version &
> > + (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
> > + (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
> > + vsp1_lif_write(lif, dl, VI6_LIF_LBA,
> > + VI6_LIF_LBA_LBA0 |
> > + (1536 << VI6_LIF_LBA_LBA1_SHIFT));
> > + }
>
> There's no need for braces or inner parentheses.
There's of course a need for the inner parentheses, please ignore this
comment.
> To make this easier to read I propose defining a new macro
> VI6_IP_VERSION_MASK that combines both the model and SoC. Otherwise this
> looks good to me,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> I'll post a v4 with that change in reply to this e-mail, please let me know
> if you're fine with it.
>
> > }
> >
> > static const struct vsp1_entity_operations lif_entity_ops = {
> >
> > Index: media_tree/drivers/media/platform/vsp1/vsp1_regs.h
> > ===================================================================
> > --- media_tree.orig/drivers/media/platform/vsp1/vsp1_regs.h
> > +++ media_tree/drivers/media/platform/vsp1/vsp1_regs.h
> > @@ -693,6 +693,11 @@
> > #define VI6_LIF_CSBTH_LBTH_MASK (0x7ff << 0)
> > #define VI6_LIF_CSBTH_LBTH_SHIFT 0
> >
> > +#define VI6_LIF_LBA 0x3b0c
> > +#define VI6_LIF_LBA_LBA0 (1 << 31)
> > +#define VI6_LIF_LBA_LBA1_MASK (0xfff << 16)
> > +#define VI6_LIF_LBA_LBA1_SHIFT 16
> > +
> > /* ----------------------------------------------------------------------
> > * Security Control Registers
> > */
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4] v4l: vsp1: Fix video output on R8A77970
2018-02-22 16:26 ` Laurent Pinchart
2018-02-22 16:29 ` Laurent Pinchart
@ 2018-02-22 16:32 ` Laurent Pinchart
2018-02-22 18:34 ` Sergei Shtylyov
2018-02-22 18:42 ` [PATCH v3] vsp1: fix " Sergei Shtylyov
2 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2018-02-22 16:32 UTC (permalink / raw)
To: linux-media; +Cc: linux-renesas-soc, Sergei Shtylyov
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
and VSP2-D instances") added support for the VSP2-D found in the R-Car
V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
garbage-like line repeated every 8 screen rows. It turns out that R-Car
V3M has the LIF0 buffer attribute register that you need to set to a non-
default value in order to get rid of the output artifacts.
Based on the original (and large) patch by Daisuke Matsushita
<daisuke.matsushita.ns@hitachi.com>.
Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
[Removed braces, added VI6_IP_VERSION_MASK to improve readabiliy]
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_lif.c | 12 ++++++++++++
drivers/media/platform/vsp1/vsp1_regs.h | 6 ++++++
2 files changed, 18 insertions(+)
diff --git a/drivers/media/platform/vsp1/vsp1_lif.c b/drivers/media/platform/vsp1/vsp1_lif.c
index e6fa16d7fda8..704920753998 100644
--- a/drivers/media/platform/vsp1/vsp1_lif.c
+++ b/drivers/media/platform/vsp1/vsp1_lif.c
@@ -155,6 +155,18 @@ static void lif_configure(struct vsp1_entity *entity,
(obth << VI6_LIF_CTRL_OBTH_SHIFT) |
(format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
+
+ /*
+ * On R-Car V3M the LIF0 buffer attribute register has to be set to a
+ * non-default value to guarantee proper operation (otherwise artifacts
+ * may appear on the output). The value required by the manual is not
+ * explained but is likely a buffer size or threshold.
+ */
+ if ((entity->vsp1->version & VI6_IP_VERSION_MASK) ==
+ (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M))
+ vsp1_lif_write(lif, dl, VI6_LIF_LBA,
+ VI6_LIF_LBA_LBA0 |
+ (1536 << VI6_LIF_LBA_LBA1_SHIFT));
}
static const struct vsp1_entity_operations lif_entity_ops = {
diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h
index b1912c83a1da..dae0c1901297 100644
--- a/drivers/media/platform/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/vsp1/vsp1_regs.h
@@ -693,6 +693,11 @@
#define VI6_LIF_CSBTH_LBTH_MASK (0x7ff << 0)
#define VI6_LIF_CSBTH_LBTH_SHIFT 0
+#define VI6_LIF_LBA 0x3b0c
+#define VI6_LIF_LBA_LBA0 (1 << 31)
+#define VI6_LIF_LBA_LBA1_MASK (0xfff << 16)
+#define VI6_LIF_LBA_LBA1_SHIFT 16
+
/* -----------------------------------------------------------------------------
* Security Control Registers
*/
@@ -705,6 +710,7 @@
*/
#define VI6_IP_VERSION 0x3f00
+#define VI6_IP_VERSION_MASK (0xffff << 0)
#define VI6_IP_VERSION_MODEL_MASK (0xff << 8)
#define VI6_IP_VERSION_MODEL_VSPS_H2 (0x09 << 8)
#define VI6_IP_VERSION_MODEL_VSPR_H2 (0x0a << 8)
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970
2018-02-22 16:32 ` [PATCH v4] v4l: vsp1: Fix " Laurent Pinchart
@ 2018-02-22 18:34 ` Sergei Shtylyov
2018-02-22 18:46 ` Laurent Pinchart
0 siblings, 1 reply; 8+ messages in thread
From: Sergei Shtylyov @ 2018-02-22 18:34 UTC (permalink / raw)
To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc
On 02/22/2018 07:32 PM, Laurent Pinchart wrote:
> From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
> and VSP2-D instances") added support for the VSP2-D found in the R-Car
> V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
> garbage-like line repeated every 8 screen rows. It turns out that R-Car
> V3M has the LIF0 buffer attribute register that you need to set to a non-
> default value in order to get rid of the output artifacts.
>
> Based on the original (and large) patch by Daisuke Matsushita
> <daisuke.matsushita.ns@hitachi.com>.
>
> Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> [Removed braces, added VI6_IP_VERSION_MASK to improve readabiliy]
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
[...]
> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h
> index b1912c83a1da..dae0c1901297 100644
> --- a/drivers/media/platform/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
[...]
> @@ -705,6 +710,7 @@
> */
>
> #define VI6_IP_VERSION 0x3f00
> +#define VI6_IP_VERSION_MASK (0xffff << 0)
Perhaps (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK) would be clearer?
MBR, Sergei
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] vsp1: fix video output on R8A77970
2018-02-22 16:26 ` Laurent Pinchart
2018-02-22 16:29 ` Laurent Pinchart
2018-02-22 16:32 ` [PATCH v4] v4l: vsp1: Fix " Laurent Pinchart
@ 2018-02-22 18:42 ` Sergei Shtylyov
2 siblings, 0 replies; 8+ messages in thread
From: Sergei Shtylyov @ 2018-02-22 18:42 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc
On 02/22/2018 07:26 PM, Laurent Pinchart wrote:
> Hi Sergei,
>
> Thank you for the patch.
>
> On Thursday, 18 January 2018 16:05:51 EET Sergei Shtylyov wrote:
>> Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
>> and VSP2-D instances") added support for the VSP2-D found in the R-Car
>> V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
>> garbage-like line repeated every 8 screen rows. It turns out that R-Car
>> V3M has the LIF0 buffer attribute register that you need to set to a non-
>> default value in order to get rid of the output artifacts...
>>
>> Based on the original (and large) patch by Daisuke Matsushita
>> <daisuke.matsushita.ns@hitachi.com>.
>>
>> Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and
>> VSP2-D instances") Signed-off-by: Sergei Shtylyov
>> <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> This patch is against the 'media_tree.git' repo's 'fixes' branch.
>>
>> Changes in version 3:
>> - reworded the comment in lif_configure();
>> - reworded the patch description.
>>
>> Changes in version 2:
>> - added a comment before the V3M SoC check;
>> - fixed indetation in that check;
>> - reformatted the patch description.
>>
>> drivers/media/platform/vsp1/vsp1_lif.c | 15 +++++++++++++++
>> drivers/media/platform/vsp1/vsp1_regs.h | 5 +++++
>> 2 files changed, 20 insertions(+)
>>
>> Index: media_tree/drivers/media/platform/vsp1/vsp1_lif.c
>> ===================================================================
>> --- media_tree.orig/drivers/media/platform/vsp1/vsp1_lif.c
>> +++ media_tree/drivers/media/platform/vsp1/vsp1_lif.c
>> @@ -155,6 +155,21 @@ static void lif_configure(struct vsp1_en
>> (obth << VI6_LIF_CTRL_OBTH_SHIFT) |
>> (format->code == 0 ? VI6_LIF_CTRL_CFMT : 0) |
>> VI6_LIF_CTRL_REQSEL | VI6_LIF_CTRL_LIF_EN);
>> +
>> + /*
>> + * On R-Car V3M the LIF0 buffer attribute register has to be set
>> + * to a non-default value to guarantee proper operation (otherwise
>> + * artifacts may appear on the output). The value required by
>> + * the manual is not explained but is likely a buffer size or
>> + * threshold...
>
> One period is enough :-)
Naw, I just like ellipsis! :-)
>> + */
>> + if ((entity->vsp1->version &
>> + (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK)) ==
>> + (VI6_IP_VERSION_MODEL_VSPD_V3 | VI6_IP_VERSION_SOC_V3M)) {
>> + vsp1_lif_write(lif, dl, VI6_LIF_LBA,
>> + VI6_LIF_LBA_LBA0 |
>> + (1536 << VI6_LIF_LBA_LBA1_SHIFT));
>> + }
>
> There's no need for braces
Well, multi-line statement was asking for them...
> or inner parentheses.
I thought so too. :-)
> To make this easier to read I
> propose defining a new macro VI6_IP_VERSION_MASK that combines both the model
> and SoC. Otherwise this looks good to me,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> I'll post a v4 with that change in reply to this e-mail, please let me know if
> you're fine with it.
Generally fine, yes...
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970
2018-02-22 18:34 ` Sergei Shtylyov
@ 2018-02-22 18:46 ` Laurent Pinchart
2018-02-22 18:57 ` Sergei Shtylyov
0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2018-02-22 18:46 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Laurent Pinchart, linux-media, linux-renesas-soc
Hi Sergei,
On Thursday, 22 February 2018 20:34:37 EET Sergei Shtylyov wrote:
> On 02/22/2018 07:32 PM, Laurent Pinchart wrote:
> > From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >
> > Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
> > and VSP2-D instances") added support for the VSP2-D found in the R-Car
> > V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
> > garbage-like line repeated every 8 screen rows. It turns out that R-Car
> > V3M has the LIF0 buffer attribute register that you need to set to a non-
> > default value in order to get rid of the output artifacts.
> >
> > Based on the original (and large) patch by Daisuke Matsushita
> > <daisuke.matsushita.ns@hitachi.com>.
> >
> > Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and
> > VSP2-D instances") Signed-off-by: Sergei Shtylyov
> > <sergei.shtylyov@cogentembedded.com> Reviewed-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com> [Removed braces, added
> > VI6_IP_VERSION_MASK to improve readabiliy]
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
>
> [...]
>
> > diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
> > b/drivers/media/platform/vsp1/vsp1_regs.h index
> > b1912c83a1da..dae0c1901297 100644
> > --- a/drivers/media/platform/vsp1/vsp1_regs.h
> > +++ b/drivers/media/platform/vsp1/vsp1_regs.h
>
> [...]
>
> > @@ -705,6 +710,7 @@
> > */
> >
> > #define VI6_IP_VERSION 0x3f00
> >
> > +#define VI6_IP_VERSION_MASK (0xffff << 0)
>
> Perhaps (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK) would be
> clearer?
I thought about it and the line length went over 80 characters so I went for
an easy solution. I can change it if you want.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970
2018-02-22 18:46 ` Laurent Pinchart
@ 2018-02-22 18:57 ` Sergei Shtylyov
0 siblings, 0 replies; 8+ messages in thread
From: Sergei Shtylyov @ 2018-02-22 18:57 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Laurent Pinchart, linux-media, linux-renesas-soc
On 02/22/2018 09:46 PM, Laurent Pinchart wrote:
>>> From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
>>> Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
>>> and VSP2-D instances") added support for the VSP2-D found in the R-Car
>>> V3M (R8A77970) but the video output that VSP2-D sends to DU has a greenish
>>> garbage-like line repeated every 8 screen rows. It turns out that R-Car
>>> V3M has the LIF0 buffer attribute register that you need to set to a non-
>>> default value in order to get rid of the output artifacts.
>>>
>>> Based on the original (and large) patch by Daisuke Matsushita
>>> <daisuke.matsushita.ns@hitachi.com>.
>>>
>>> Fixes: d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and
>>> VSP2-D instances") Signed-off-by: Sergei Shtylyov
>>> <sergei.shtylyov@cogentembedded.com> Reviewed-by: Laurent Pinchart
>>> <laurent.pinchart+renesas@ideasonboard.com> [Removed braces, added
>>> VI6_IP_VERSION_MASK to improve readabiliy]
>>> Signed-off-by: Laurent Pinchart
>>> <laurent.pinchart+renesas@ideasonboard.com>
>>
>> [...]
>>
>>> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
>>> b/drivers/media/platform/vsp1/vsp1_regs.h index
>>> b1912c83a1da..dae0c1901297 100644
>>> --- a/drivers/media/platform/vsp1/vsp1_regs.h
>>> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
>>
>> [...]
>>
>>> @@ -705,6 +710,7 @@
>>> */
>>>
>>> #define VI6_IP_VERSION 0x3f00
>>>
>>> +#define VI6_IP_VERSION_MASK (0xffff << 0)
>>
>> Perhaps (VI6_IP_VERSION_MODEL_MASK | VI6_IP_VERSION_SOC_MASK) would be
>> clearer?
>
> I thought about it and the line length went over 80 characters so I went for
> an easy solution. I can change it if you want.
OK, let's be leave it as is.
MBR, Sergei
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-02-22 18:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-18 14:05 [PATCH v3] vsp1: fix video output on R8A77970 Sergei Shtylyov
2018-02-22 16:26 ` Laurent Pinchart
2018-02-22 16:29 ` Laurent Pinchart
2018-02-22 16:32 ` [PATCH v4] v4l: vsp1: Fix " Laurent Pinchart
2018-02-22 18:34 ` Sergei Shtylyov
2018-02-22 18:46 ` Laurent Pinchart
2018-02-22 18:57 ` Sergei Shtylyov
2018-02-22 18:42 ` [PATCH v3] vsp1: fix " Sergei Shtylyov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.