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+rene...@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+rene...@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]
--
Regards,
Laurent Pinchart
