Hi Kieran,
On Thursday 13 Jul 2017 18:49:19 Kieran Bingham wrote:
On 26/06/17 19:12, Laurent Pinchart wrote:
New Gen3 SoCs come with two new VSP2 variants names VSP2-BS and VSP2-DL, as well as a new VSP2-D variant on V3M and V3H SoCs. Add new entries for them in the VSP device info table.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
Code in the patch looks OK - but I can't see where the difference between the horizontal widths are supported between VSPD H3/VC
I see this in the datasheet: (32.1.1.6 in this particular part)
Direct connection to display module — Supporting 4096 pixels in horizontal direction [R-Car H3/R-Car M3-W/ R-Car M3-N] — Supporting 2048 pixels in horizontal direction [R-Car V3M/R-Car V3H/R-Car D3/R-Car E3]
Do we have this information encoded anywhere? or are they just talking about maximum performance capability there?
No, we don't. It's a limit that we should have. I think we should fix that in a separate patch, as the 4096 pixels limit isn't implemented either.
Also some features that are implied as supported aren't mentioned - but that's not a blocker to adding in the initial devices at all.
Therefore:
Reviewed-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com
drivers/media/platform/vsp1/vsp1_drv.c | 24 ++++++++++++++++++++++++ drivers/media/platform/vsp1/vsp1_regs.h | 15 +++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c index 6a9aeb71aedf..c4f2ac61f7d2 100644 --- a/drivers/media/platform/vsp1/vsp1_drv.c +++ b/drivers/media/platform/vsp1/vsp1_drv.c @@ -710,6 +710,14 @@ static const struct vsp1_device_info vsp1_device_infos[] = {> .num_bru_inputs = 5, .uapi = true, }, {
.version = VI6_IP_VERSION_MODEL_VSPBS_GEN3,.model = "VSP2-BS",.gen = 3,.features = VSP1_HAS_BRS,32.1.1.5 implies: | VSP1_HAS_WPF_VFLIP
But Figure 32.5 implies that it doesn't ...
The figures only tell whether the full combination of rotation and H/V flip is available. I think you're right, I'll add VSP1_HAS_WPF_VFLIP.
Figure 32.5 also implies that | VSP1_HAS_CLU is there too on both RPF0, and RPF1
Note that CLUT != CLU. I know it's confusing :-)
.rpf_count = 2,.wpf_count = 1,.uapi = true,- }, { .version = VI6_IP_VERSION_MODEL_VSPD_GEN3, .model = "VSP2-D", .gen = 3,
@@ -717,6 +725,22 @@ static const struct vsp1_device_info vsp1_device_infos[] = {> .rpf_count = 5, .wpf_count = 2, .num_bru_inputs = 5,
- }, {
.version = VI6_IP_VERSION_MODEL_VSPD_V3,.model = "VSP2-D",.gen = 3,.features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_LIF,.rpf_count = 5,.wpf_count = 1,.num_bru_inputs = 5,- }, {
.version = VI6_IP_VERSION_MODEL_VSPDL_GEN3,.model = "VSP2-DL",.gen = 3,.features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_LIF,Hrm. 32.1.1.7 says: — Vertical flipping in case of output to memory. So thats some sort of a conditional : | VSP1_HAS_WPF_VFLIP
So looking at this and the settings of the existing models, I guess it looks like we don't support flip if we have an LIF output (as that would then be unsupported)
On Gen3 vertical flipping seems to always be supported, unlike on Gen2 where VSPD is specifically documented as not supporting vertical flipping. We could add the WFLIP on all VSP2-D* instances. This would create a corresponding control, which wouldn't do much harm as the VSPD instances on Gen3 are not exposed to userspace, but that would waste a bit of memory for no good purpose (beside correctness I suppose). I wonder if that's worth it, what do you think ? If so, VSP2-D should be fixed too, so I'd prefer doing that in a separate patch.
.rpf_count = 5,.wpf_count = 2,.num_bru_inputs = 5,};
[snip]