vsp1: fix video output on R8A77970
PATCH v3 1/1 vsp1: fix video output on R8A77970
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 <[email protected]>.
---
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(+)
drivers/media/platform/vsp1/vsp1_lif.c +15 -0
@@ -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 = {
drivers/media/platform/vsp1/vsp1_regs.h +5 -0
@@ -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 */
Hi Sergei,
Thank you for the patch.
Quoted 35 lines · 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 <[email protected]>.
Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL andVSP2-D instances") Signed-off-by: Sergei Shtylyov
<[email protected]>---
This patch is against the 'media_tree.git' repo's 'fixes' branch.Changes in version 3:
- reworded the comment inlif_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
===================================================================
drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 164
Quoted 10 lines
drivers/media/platform/vsp1/vsp1_lif.c +7
-0@@ -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 :-)
drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 172
Quoted 8 lines
drivers/media/platform/vsp1/vsp1_lif.c +8
-0+*/ +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,
I'll post a v4 with that change in reply to this e-mail, please let me know if you're fine with it.
Quoted 19 lines
drivers/media/platform/vsp1/vsp1_lif.c +0
-0}static const struct vsp1_entity_operations lif_entity_ops = {Index:
media_tree/drivers/media/platform/vsp1/vsp1_regs.h
===================================================================drivers/media/platform/vsp1/vsp1_regs.h +5
-0@@ -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 */
Signature · 3 lines
Regards, Laurent Pinchart
Hi Sergei,
Quoted 51 lines · On Thursday, 22 February 2018 18:26:20 EET Laurent Pinchart wrote:
Quoted 37 lines · 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 <[email protected]>.
Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL andVSP2-D instances") Signed-off-by: Sergei Shtylyov
<[email protected]>---
This patch is against the 'media_tree.git' repo's 'fixes' branch.Changes in version 3:
- reworded the comment inlif_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
===================================================================drivers/media/platform/vsp1/vsp1_lif.c +0
-0@@ -155,6 +155,21 @@ static void lif_configure(struct vsp1_en (obth << VI6_LIF_CTRL_OBTH_SHIFT) |drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 166
Quoted 10 lines
drivers/media/platform/vsp1/vsp1_lif.c +7
-0(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 :-)
Quoted 10 lines
drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 172
Quoted 8 lines
drivers/media/platform/vsp1/vsp1_lif.c +8
-0+*/ +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.
Quoted 29 lines
To make this easier to read I propose defining a new macro
VI6_IP_VERSION_MASKthat combines both the model and SoC. Otherwise this looks good to me,Reviewed-by Laurent Pinchart <[email protected]>I'll post a v4 with that change in reply to this e-mail, please let me know if you're fine with it.
Quoted 20 lines
drivers/media/platform/vsp1/vsp1_lif.c +0
-0}static const struct
vsp1_entity_operationslif_entity_ops= {Index:
media_tree/drivers/media/platform/vsp1/vsp1_regs.h
===================================================================drivers/media/platform/vsp1/vsp1_regs.h +5
-0@@ -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 */
Signature · 3 lines
Regards, Laurent Pinchart
PATCH v4 1/1 v4l: vsp1: Fix video output on R8A77970
In reply to Laurent PinchartFrom: Sergei Shtylyov <[email protected]>
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 <[email protected]>.
--- drivers/media/platform/vsp1/vsp1_lif.c | 12 ++++++++++++ drivers/media/platform/vsp1/vsp1_regs.h | 6 ++++++ 2 files changed, 18 insertions(+)
drivers/media/platform/vsp1/vsp1_lif.c +12 -0
@@ -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 = {
drivers/media/platform/vsp1/vsp1_regs.h +6 -0
@@ -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)
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970
Quoted 17 lines · Laurent Pinchart
From: Sergei Shtylyov <[email protected]>
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 <[email protected]>.
Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances")Signed-off-by Sergei Shtylyov <[email protected]>Reviewed-by Laurent Pinchart <[email protected]>[Removed braces, added VI6_IP_VERSION_MASK to improve readabiliy]Signed-off-by Laurent Pinchart <[email protected]>
[...]
Quoted 4 lines
drivers/media/platform/vsp1/vsp1_regs.h +0
-0
[...]
drivers/media/platform/vsp1/vsp1_regs.h line 713
Quoted 5 lines
drivers/media/platform/vsp1/vsp1_regs.h +1
-0@@ -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
Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970
Hi Sergei,
Quoted 34 lines · On Thursday, 22 February 2018 20:34:37 EET Sergei Shtylyov wrote:
Quoted 19 lines · Laurent Pinchart
From: Sergei Shtylyov <[email protected]>
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 <[email protected]>.
Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL andVSP2-D instances") Signed-off-by: Sergei Shtylyov <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> [Removed braces, added
VI6_IP_VERSION_MASKto improve readabiliy]
Signed-off-by: Laurent Pinchart <[email protected]>[...]
Quoted 5 lines
(quoted diff) +0
-0
b/drivers/media/platform/vsp1/vsp1_regs.hindexb1912c83a1da..dae0c1901297100644drivers/media/platform/vsp1/vsp1_regs.h +0
-0[...]
Quoted 3 lines
drivers/media/platform/vsp1/vsp1_regs.h +0
-0@@ -705,6 +710,7 @@*/
Quoted 6 lines
drivers/media/platform/vsp1/vsp1_regs.h line 713
Quoted 3 lines
drivers/media/platform/vsp1/vsp1_regs.h +1
-0#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.
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v4] v4l: vsp1: Fix video output on R8A77970
Quoted 35 lines · Laurent Pinchart
Quoted 35 lines
Quoted 19 lines
From: Sergei Shtylyov <[email protected]>
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 <[email protected]>.
Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL andVSP2-D instances") Signed-off-by: Sergei Shtylyov <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> [Removed braces, added
VI6_IP_VERSION_MASKto improve readabiliy]
Signed-off-by: Laurent Pinchart <[email protected]>[...]
Quoted 5 lines
(quoted diff) +0
-0
b/drivers/media/platform/vsp1/vsp1_regs.hindexb1912c83a1da..dae0c1901297100644drivers/media/platform/vsp1/vsp1_regs.h +0
-0[...]
Quoted 5 lines
drivers/media/platform/vsp1/vsp1_regs.h +0
-0@@ -705,6 +710,7 @@*/ #define VI6_IP_VERSION 0x3f00
Quoted 7 lines
Quoted 4 lines
drivers/media/platform/vsp1/vsp1_regs.h line 713
Quoted 1 line
drivers/media/platform/vsp1/vsp1_regs.h +1
-0+#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
Quoted 44 lines · Laurent Pinchart
Hi Sergei,
Thank you for the patch.
Quoted 39 lines · 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 <[email protected]>.
Fixes d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL andVSP2-D instances") Signed-off-by: Sergei Shtylyov
<[email protected]>---
This patch is against the 'media_tree.git' repo's 'fixes' branch.Changes in version 3:
- reworded the comment inlif_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
===================================================================drivers/media/platform/vsp1/vsp1_lif.c +1
-0@@ -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); +
Quoted 8 lines
drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 164
Quoted 6 lines
drivers/media/platform/vsp1/vsp1_lif.c +6
-0+/* +* 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! :-)
Quoted 10 lines
drivers/media/platform/vsp1/vsp1_lif.c static void lif_configure(struct vsp1_en · line 172
Quoted 8 lines
drivers/media/platform/vsp1/vsp1_lif.c +8
-0+*/ +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...
Quoted 1 line
or inner parentheses.
I thought so too. :-)
Quoted 8 lines
To make this easier to read I
propose defining a new macroVI6_IP_VERSION_MASKthat combines both the model and SoC. Otherwise this looks good to me,Reviewed-by Laurent Pinchart <[email protected]>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