Re: [PATCH] media: allegro-dvt: Moving the current driver to subdirectory

From: Michael Tretter
Date: Thu Dec 05 2024 - 03:59:15 EST

Next message: Krzysztof Kozlowski: "Re: [PATCH v4 1/4] dt-bindings: firmware: imx: add nvmem phandle"
Previous message: Greg KH: "Re: [PATCH] USB: core: Disable LPM only for non-suspended ports"
In reply to: Yassine Ouaissa: "Re: [PATCH] media: allegro-dvt: Moving the current driver to subdirectory"
Next in thread: Nicolas Dufresne: "Re: [PATCH] media: allegro-dvt: Moving the current driver to subdirectory"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Yassine,

On Thu, 05 Dec 2024 08:11:48 +0000, Yassine Ouaissa wrote:
> On 04.12.2024 17:27, Michael Tretter wrote:
> > On Wed, 04 Dec 2024 14:44:31 +0000, Yassine Ouaissa wrote:
> > > On 02.12.2024 11:55, Michael Tretter wrote:
> > > > On Mon, 02 Dec 2024 11:26:38 +0100, Yassine Ouaissa wrote:
> > > > What kind of conflicts do you encounter or expect? It's kind of
> > > > surprising that your driver would conflict with anything in the existing
> > > > driver.
> > >
> > > The current "NOT allegro" driver support only the ZynqMP (ex : ZCU106/ZCU104 ).
> > > And the upcoming driver, has support for many platforms.
> >
> > What are the supported platforms of the new driver? Does your driver
> > support ZynqMP, too?
> >
> The upcoming driver is not using the ZynqMP, it can be used by other
> clients including Xilinx.
> > If there are enough similarities between the encoders, I'd happily
> > replace parts of the current driver with functionality of the upcoming
> > driver or reuse the infrastructure of the new driver.
> >
>
> For now, we do have only the decoder (stateful). the encoder is "in
> progress". We prefer to have a separate driver core for each platform.

Thanks! I'd like to wait until I've seen your patches before agreeing to
move the driver to a subdirectory. Maybe even include this patch as the
first patch in your series to give a full picture of the new structure.

Michael

> > >
> > > > I'd like to see how you plan to integrate your driver, what it looks
> > > > like and what conflicts with the existing driver actually occur. Without
> > > > that information I don't see the point in moving the driver to a
> > > > sub-directory.
> > >
> > > The upcoming driver is not using the parsing, conversion inside of the
> > > driver. Actually the new Gen of ALLEGRO IP does that.
> >
> > Sorry, I don't understand what you mean by parsing and conversion. Do
> > you mean the mailbox messages or the NAL units?
> >
> > Can you point me to some repository with your upcoming driver? I'd
> > really like to know what's coming up to have an informed discussion how
> > to structure the directory.
> >
> > Michael
> >
>
> Yassine
> > > > >
> > > > > Signed-off-by: Yassine Ouaissa <yassine.ouaissa@xxxxxxxxxxxxxx>
> > > > > ---
> > > > > MAINTAINERS | 2 +-
> > > > > drivers/media/platform/allegro-dvt/Kconfig | 4 ++--
> > > > > drivers/media/platform/allegro-dvt/Makefile | 8 ++------
> > > > > drivers/media/platform/allegro-dvt/zynqmp/Makefile | 6 ++++++
> > > > > .../media/platform/allegro-dvt/{ => zynqmp}/nal-h264.c | 0
> > > > > .../media/platform/allegro-dvt/{ => zynqmp}/nal-h264.h | 0
> > > > > .../media/platform/allegro-dvt/{ => zynqmp}/nal-hevc.c | 0
> > > > > .../media/platform/allegro-dvt/{ => zynqmp}/nal-hevc.h | 0
> > > > > .../media/platform/allegro-dvt/{ => zynqmp}/nal-rbsp.c | 0
> > > > > .../media/platform/allegro-dvt/{ => zynqmp}/nal-rbsp.h | 0
> > > > > .../{allegro-core.c => zynqmp/zynqmp-allegro-core.c} | 2 +-
> > > > > .../{allegro-mail.c => zynqmp/zynqmp-allegro-mail.c} | 2 +-
> > > > > .../{allegro-mail.h => zynqmp/zynqmp-allegro-mail.h} | 0
> > > > > 13 files changed, 13 insertions(+), 11 deletions(-)
> > > > > create mode 100644 drivers/media/platform/allegro-dvt/zynqmp/Makefile
> > > > > rename drivers/media/platform/allegro-dvt/{ => zynqmp}/nal-h264.c (100%)
> > > > > rename drivers/media/platform/allegro-dvt/{ => zynqmp}/nal-h264.h (100%)
> > > > > rename drivers/media/platform/allegro-dvt/{ => zynqmp}/nal-hevc.c (100%)
> > > > > rename drivers/media/platform/allegro-dvt/{ => zynqmp}/nal-hevc.h (100%)
> > > > > rename drivers/media/platform/allegro-dvt/{ => zynqmp}/nal-rbsp.c (100%)
> > > > > rename drivers/media/platform/allegro-dvt/{ => zynqmp}/nal-rbsp.h (100%)
> > > > > rename drivers/media/platform/allegro-dvt/{allegro-core.c => zynqmp/zynqmp-allegro-core.c} (99%)
> > > > > rename drivers/media/platform/allegro-dvt/{allegro-mail.c => zynqmp/zynqmp-allegro-mail.c} (99%)
> > > > > rename drivers/media/platform/allegro-dvt/{allegro-mail.h => zynqmp/zynqmp-allegro-mail.h} (100%)
> > > > >
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index 7292e4a1ddb8..d79efe6f8992 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -795,7 +795,7 @@ R: Pengutronix Kernel Team <kernel@xxxxxxxxxxxxxx>
> > > > > L: linux-media@xxxxxxxxxxxxxxx
> > > > > S: Maintained
> > > > > F: Documentation/devicetree/bindings/media/allegro,al5e.yaml
> > > > > -F: drivers/media/platform/allegro-dvt/
> > > > > +F: drivers/media/platform/allegro-dvt/zynqmp
> > > > >
> > > > > ALLIED VISION ALVIUM CAMERA DRIVER
> > > > > M: Tommaso Merciai <tomm.merciai@xxxxxxxxx>
> > > > > diff --git a/drivers/media/platform/allegro-dvt/Kconfig b/drivers/media/platform/allegro-dvt/Kconfig
> > > > > index 2182e1277568..9bc7e99788b8 100644
> > > > > --- a/drivers/media/platform/allegro-dvt/Kconfig
> > > > > +++ b/drivers/media/platform/allegro-dvt/Kconfig
> > > > > @@ -2,8 +2,8 @@
> > > > >
> > > > > comment "Allegro DVT media platform drivers"
> > > > >
> > > > > -config VIDEO_ALLEGRO_DVT
> > > > > - tristate "Allegro DVT Video IP Core"
> > > > > +config VIDEO_ZYNQMP_ALLEGRO_DVT
> > > > > + tristate "Allegro DVT Video IP Core for ZynqMP"
> > > > > depends on V4L_MEM2MEM_DRIVERS
> > > > > depends on VIDEO_DEV
> > > > > depends on ARCH_ZYNQMP || COMPILE_TEST
> > > > > diff --git a/drivers/media/platform/allegro-dvt/Makefile b/drivers/media/platform/allegro-dvt/Makefile
> > > > > index 66108a303774..04727c5f7ef4 100644
> > > > > --- a/drivers/media/platform/allegro-dvt/Makefile
> > > > > +++ b/drivers/media/platform/allegro-dvt/Makefile
> > > > > @@ -1,6 +1,2 @@
> > > > > -# SPDX-License-Identifier: GPL-2.0
> > > > > -
> > > > > -allegro-objs := allegro-core.o allegro-mail.o
> > > > > -allegro-objs += nal-rbsp.o nal-h264.o nal-hevc.o
> > > > > -
> > > > > -obj-$(CONFIG_VIDEO_ALLEGRO_DVT) += allegro.o
> > > > > +# SPDX-License-Identifier: GPL-2.0-only
> > > > > +obj-y += zynqmp/
> > > > > diff --git a/drivers/media/platform/allegro-dvt/zynqmp/Makefile b/drivers/media/platform/allegro-dvt/zynqmp/Makefile
> > > > > new file mode 100644
> > > > > index 000000000000..a8ddb9cf93a8
> > > > > --- /dev/null
> > > > > +++ b/drivers/media/platform/allegro-dvt/zynqmp/Makefile
> > > > > @@ -0,0 +1,6 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +
> > > > > +zynqmp-allegro-objs := zynqmp-allegro-core.o zynqmp-allegro-mail.o
> > > > > +zynqmp-allegro-objs += nal-rbsp.o nal-h264.o nal-hevc.o
> > > > > +
> > > > > +obj-$(CONFIG_VIDEO_ZYNQMP_ALLEGRO_DVT) += zynqmp-allegro.o
> > > > > diff --git a/drivers/media/platform/allegro-dvt/nal-h264.c b/drivers/media/platform/allegro-dvt/zynqmp/nal-h264.c
> > > > > similarity index 100%
> > > > > rename from drivers/media/platform/allegro-dvt/nal-h264.c
> > > > > rename to drivers/media/platform/allegro-dvt/zynqmp/nal-h264.c
> > > > > diff --git a/drivers/media/platform/allegro-dvt/nal-h264.h b/drivers/media/platform/allegro-dvt/zynqmp/nal-h264.h
> > > > > similarity index 100%
> > > > > rename from drivers/media/platform/allegro-dvt/nal-h264.h
> > > > > rename to drivers/media/platform/allegro-dvt/zynqmp/nal-h264.h
> > > > > diff --git a/drivers/media/platform/allegro-dvt/nal-hevc.c b/drivers/media/platform/allegro-dvt/zynqmp/nal-hevc.c
> > > > > similarity index 100%
> > > > > rename from drivers/media/platform/allegro-dvt/nal-hevc.c
> > > > > rename to drivers/media/platform/allegro-dvt/zynqmp/nal-hevc.c
> > > > > diff --git a/drivers/media/platform/allegro-dvt/nal-hevc.h b/drivers/media/platform/allegro-dvt/zynqmp/nal-hevc.h
> > > > > similarity index 100%
> > > > > rename from drivers/media/platform/allegro-dvt/nal-hevc.h
> > > > > rename to drivers/media/platform/allegro-dvt/zynqmp/nal-hevc.h
> > > > > diff --git a/drivers/media/platform/allegro-dvt/nal-rbsp.c b/drivers/media/platform/allegro-dvt/zynqmp/nal-rbsp.c
> > > > > similarity index 100%
> > > > > rename from drivers/media/platform/allegro-dvt/nal-rbsp.c
> > > > > rename to drivers/media/platform/allegro-dvt/zynqmp/nal-rbsp.c
> > > > > diff --git a/drivers/media/platform/allegro-dvt/nal-rbsp.h b/drivers/media/platform/allegro-dvt/zynqmp/nal-rbsp.h
> > > > > similarity index 100%
> > > > > rename from drivers/media/platform/allegro-dvt/nal-rbsp.h
> > > > > rename to drivers/media/platform/allegro-dvt/zynqmp/nal-rbsp.h
> > > > > diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/zynqmp/zynqmp-allegro-core.c
> > > > > similarity index 99%
> > > > > rename from drivers/media/platform/allegro-dvt/allegro-core.c
> > > > > rename to drivers/media/platform/allegro-dvt/zynqmp/zynqmp-allegro-core.c
> > > > > index e491399afcc9..8895d7755987 100644
> > > > > --- a/drivers/media/platform/allegro-dvt/allegro-core.c
> > > > > +++ b/drivers/media/platform/allegro-dvt/zynqmp/zynqmp-allegro-core.c
> > > > > @@ -31,7 +31,7 @@
> > > > > #include <media/videobuf2-dma-contig.h>
> > > > > #include <media/videobuf2-v4l2.h>
> > > > >
> > > > > -#include "allegro-mail.h"
> > > > > +#include "zynqmp-allegro-mail.h"
> > > > > #include "nal-h264.h"
> > > > > #include "nal-hevc.h"
> > > > >
> > > > > diff --git a/drivers/media/platform/allegro-dvt/allegro-mail.c b/drivers/media/platform/allegro-dvt/zynqmp/zynqmp-allegro-mail.c
> > > > > similarity index 99%
> > > > > rename from drivers/media/platform/allegro-dvt/allegro-mail.c
> > > > > rename to drivers/media/platform/allegro-dvt/zynqmp/zynqmp-allegro-mail.c
> > > > > index aadc947a77ae..88a98f9e5d00 100644
> > > > > --- a/drivers/media/platform/allegro-dvt/allegro-mail.c
> > > > > +++ b/drivers/media/platform/allegro-dvt/zynqmp/zynqmp-allegro-mail.c
> > > > > @@ -12,7 +12,7 @@
> > > > > #include <linux/string.h>
> > > > > #include <linux/videodev2.h>
> > > > >
> > > > > -#include "allegro-mail.h"
> > > > > +#include "zynqmp-allegro-mail.h"
> > > > >
> > > > > const char *msg_type_name(enum mcu_msg_type type)
> > > > > {
> > > > > diff --git a/drivers/media/platform/allegro-dvt/allegro-mail.h b/drivers/media/platform/allegro-dvt/zynqmp/zynqmp-allegro-mail.h
> > > > > similarity index 100%
> > > > > rename from drivers/media/platform/allegro-dvt/allegro-mail.h
> > > > > rename to drivers/media/platform/allegro-dvt/zynqmp/zynqmp-allegro-mail.h
> > > > > --
> > > > > 2.30.2
> > > > >
> > > > >

Next message: Krzysztof Kozlowski: "Re: [PATCH v4 1/4] dt-bindings: firmware: imx: add nvmem phandle"
Previous message: Greg KH: "Re: [PATCH] USB: core: Disable LPM only for non-suspended ports"
In reply to: Yassine Ouaissa: "Re: [PATCH] media: allegro-dvt: Moving the current driver to subdirectory"
Next in thread: Nicolas Dufresne: "Re: [PATCH] media: allegro-dvt: Moving the current driver to subdirectory"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]