Hi Laurent
On Fri, Mar 21, 2025 at 02:09:42PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Mar 19, 2025 at 12:28:19PM +0100, Jacopo Mondi wrote:
> > When the RPF/WPF units are used for ISP interfacing through
> > the IIF, the set of accessible registers is limited compared to
> > the regular VSPD operations.
> >
> > Support ISP interfacing in the rpf and wpf entities by checking if
> > the pipe features an IIF instance and writing only the relevant
> > registers.
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > Tested-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/platform/renesas/vsp1/vsp1_rpf.c | 11 +++++++++--
> > drivers/media/platform/renesas/vsp1/vsp1_wpf.c | 14 ++++++++++----
> > 2 files changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > index 056491286577cc8e9e7a6bd096f1107da6009ea7..4e960fc910c16600b875286c2efec558ebdc1ee7 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> > @@ -84,7 +84,7 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
> > sink_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
> > source_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SOURCE);
> >
> > - infmt = VI6_RPF_INFMT_CIPM
> > + infmt = (pipe->iif ? 0 : VI6_RPF_INFMT_CIPM)
> > | (fmtinfo->hwfmt << VI6_RPF_INFMT_RDFMT_SHIFT);
> >
> > if (fmtinfo->swap_yc)
> > @@ -98,7 +98,7 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
> > vsp1_rpf_write(rpf, dlb, VI6_RPF_INFMT, infmt);
> > vsp1_rpf_write(rpf, dlb, VI6_RPF_DSWAP, fmtinfo->swap);
> >
> > - if (entity->vsp1->info->gen == 4) {
> > + if (entity->vsp1->info->gen == 4 && !pipe->iif) {
>
> I think this can be dropped, because ...
>
> > u32 ext_infmt0;
> > u32 ext_infmt1;
> > u32 ext_infmt2;
> > @@ -163,6 +163,13 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
> > if (pipe->interlaced)
> > top /= 2;
> >
> > + /* No further configuration for VSPX. */
> > + if (pipe->iif) {
> > + /* VSPX wants alpha_sel to be set to 0. */
> > + vsp1_rpf_write(rpf, dlb, VI6_RPF_ALPH_SEL, 0);
> > + return;
> > + }
> > +
>
> This block can be moved right after DSWAP. I can handle this when
> applying the series if there's no need to resend for other reasons (I
> would appreciate the change being tested first though).
Indeed, this is way nicer.
I've sent v6 with the change you have suggested and re-tested it
again.
Thanks
j
>
> > vsp1_rpf_write(rpf, dlb, VI6_RPF_LOC,
> > (left << VI6_RPF_LOC_HCOORD_SHIFT) |
> > (top << VI6_RPF_LOC_VCOORD_SHIFT));
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > index a32e4b3527db41e7fac859ad8e13670141c1ef04..fafef9eeb3f898b774287d615bb4a99fed0b4cfe 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c
> > @@ -247,8 +247,11 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
> > sink_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
> > source_format = v4l2_subdev_state_get_format(state, RWPF_PAD_SOURCE);
> >
> > - /* Format */
> > - if (!pipe->lif || wpf->writeback) {
> > + /*
> > + * Format configuration. Skip for IIF (VSPX) or if the pipe doesn't
> > + * write to memory.
> > + */
> > + if (!pipe->iif && (!pipe->lif || wpf->writeback)) {
> > const struct v4l2_pix_format_mplane *format = &wpf->format;
> > const struct vsp1_format_info *fmtinfo = wpf->fmtinfo;
> >
> > @@ -291,7 +294,7 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
> > * Sources. If the pipeline has a single input and BRx is not used,
> > * configure it as the master layer. Otherwise configure all
> > * inputs as sub-layers and select the virtual RPF as the master
> > - * layer.
> > + * layer. For VSPX configure the enabled sources as masters.
> > */
> > for (i = 0; i < vsp1->info->rpf_count; ++i) {
> > struct vsp1_rwpf *input = pipe->inputs[i];
> > @@ -299,7 +302,7 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
> > if (!input)
> > continue;
> >
> > - srcrpf |= (!pipe->brx && pipe->num_inputs == 1)
> > + srcrpf |= (pipe->iif || (!pipe->brx && pipe->num_inputs == 1))
> > ? VI6_WPF_SRCRPF_RPF_ACT_MST(input->entity.index)
> > : VI6_WPF_SRCRPF_RPF_ACT_SUB(input->entity.index);
> > }
> > @@ -316,6 +319,9 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
> > vsp1_dl_body_write(dlb, VI6_WPF_IRQ_ENB(index),
> > VI6_WPF_IRQ_ENB_DFEE);
> >
> > + if (pipe->iif)
> > + return;
> > +
> > /*
> > * Configure writeback for display pipelines (the wpf writeback flag is
> > * never set for memory-to-memory pipelines). Start by adding a chained
>
> --
> Regards,
>
> Laurent Pinchart
