[libcamera-devel,v3,6/7] build: controls: Add Raspberry Pi vendor specific controls
diff mbox series

Message ID 20231124123713.22519-7-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Vendor controls and properties
Related show

Commit Message

Naushir Patuck Nov. 24, 2023, 12:37 p.m. UTC 
Add a new control_ids_rpi.yaml file to hold the Raspberry Pi specific
vendor controls. This contains a single control PispConfigDumpFile that
will be used in the Pi 5 pipeline handler as a trigger to dump the
Backend configuration as a JSON file.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 include/libcamera/meson.build      |  2 ++
 src/libcamera/control_ids_rpi.yaml | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)
 create mode 100644 src/libcamera/control_ids_rpi.yaml

Comments

Jacopo Mondi Nov. 27, 2023, 4:48 p.m. UTC | #1 
Hi Naush

On Fri, Nov 24, 2023 at 12:37:12PM +0000, Naushir Patuck via libcamera-devel wrote:
> Add a new control_ids_rpi.yaml file to hold the Raspberry Pi specific
> vendor controls. This contains a single control PispConfigDumpFile that
> will be used in the Pi 5 pipeline handler as a trigger to dump the
> Backend configuration as a JSON file.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  include/libcamera/meson.build      |  2 ++
>  src/libcamera/control_ids_rpi.yaml | 17 +++++++++++++++++
>  2 files changed, 19 insertions(+)
>  create mode 100644 src/libcamera/control_ids_rpi.yaml
>
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 1504f741ae2f..5d20e4d869e3 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -36,6 +36,8 @@ controls_map = {
>      'controls': {
>          'draft': 'control_ids_draft.yaml',
>          'core': 'control_ids_core.yaml',
> +        'rpi/pisp': 'control_ids_rpi.yaml',

Not mainline yet, but I guess it doesn't hurt :)

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

> +        'rpi/vc4': 'control_ids_rpi.yaml',
>      },
>
>      'properties': {
> diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml
> new file mode 100644
> index 000000000000..abf82098eb12
> --- /dev/null
> +++ b/src/libcamera/control_ids_rpi.yaml
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +#
> +# Copyright (C) 2023, Raspberry Pi Ltd
> +#
> +%YAML 1.1
> +---
> +# Raspberry Pi (VC4 and PiSP) specific vendor controls
> +vendor: rpi
> +controls:
> +  - PispConfigDumpFile:
> +      type: string
> +      description: |
> +        Triggers the Raspberry Pi PiSP pipeline handler to generate a JSON
> +        formatted dump of the Backend configuration to the filename given by the
> +        value of the control.
> +
> +...
> \ No newline at end of file
> --
> 2.34.1
>
Kieran Bingham Nov. 28, 2023, 10:35 a.m. UTC | #2 
Quoting Jacopo Mondi via libcamera-devel (2023-11-27 16:48:54)
> Hi Naush
> 
> On Fri, Nov 24, 2023 at 12:37:12PM +0000, Naushir Patuck via libcamera-devel wrote:
> > Add a new control_ids_rpi.yaml file to hold the Raspberry Pi specific
> > vendor controls. This contains a single control PispConfigDumpFile that
> > will be used in the Pi 5 pipeline handler as a trigger to dump the
> > Backend configuration as a JSON file.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  include/libcamera/meson.build      |  2 ++
> >  src/libcamera/control_ids_rpi.yaml | 17 +++++++++++++++++
> >  2 files changed, 19 insertions(+)
> >  create mode 100644 src/libcamera/control_ids_rpi.yaml
> >
> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > index 1504f741ae2f..5d20e4d869e3 100644
> > --- a/include/libcamera/meson.build
> > +++ b/include/libcamera/meson.build
> > @@ -36,6 +36,8 @@ controls_map = {
> >      'controls': {
> >          'draft': 'control_ids_draft.yaml',
> >          'core': 'control_ids_core.yaml',
> > +        'rpi/pisp': 'control_ids_rpi.yaml',
> 
> Not mainline yet, but I guess it doesn't hurt :)

Eugh ... I  ... <looks away> It's fine ... it's coming up soon right
;-)

It certainly doesn't make sense to add this as only vc4 when the only
control so far is pisp specific, and we want to add this file so there
is an initial user of the vendor controls.

So I say lets go with it.  It's one line referencing something out of
tree that will be in-tree as soon as possible. (waiting to see if I get
shot down on this ...)

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> > +        'rpi/vc4': 'control_ids_rpi.yaml',
> >      },
> >
> >      'properties': {
> > diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml
> > new file mode 100644
> > index 000000000000..abf82098eb12
> > --- /dev/null
> > +++ b/src/libcamera/control_ids_rpi.yaml
> > @@ -0,0 +1,17 @@
> > +# SPDX-License-Identifier: LGPL-2.1-or-later
> > +#
> > +# Copyright (C) 2023, Raspberry Pi Ltd
> > +#
> > +%YAML 1.1
> > +---
> > +# Raspberry Pi (VC4 and PiSP) specific vendor controls
> > +vendor: rpi
> > +controls:
> > +  - PispConfigDumpFile:
> > +      type: string
> > +      description: |
> > +        Triggers the Raspberry Pi PiSP pipeline handler to generate a JSON
> > +        formatted dump of the Backend configuration to the filename given by the
> > +        value of the control.
> > +
> > +...
> > \ No newline at end of file
> > --
> > 2.34.1
> >
Naushir Patuck Nov. 28, 2023, 10:52 a.m. UTC | #3 
On Tue, 28 Nov 2023 at 10:35, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting Jacopo Mondi via libcamera-devel (2023-11-27 16:48:54)
> > Hi Naush
> >
> > On Fri, Nov 24, 2023 at 12:37:12PM +0000, Naushir Patuck via libcamera-devel wrote:
> > > Add a new control_ids_rpi.yaml file to hold the Raspberry Pi specific
> > > vendor controls. This contains a single control PispConfigDumpFile that
> > > will be used in the Pi 5 pipeline handler as a trigger to dump the
> > > Backend configuration as a JSON file.
> > >
> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > > ---
> > >  include/libcamera/meson.build      |  2 ++
> > >  src/libcamera/control_ids_rpi.yaml | 17 +++++++++++++++++
> > >  2 files changed, 19 insertions(+)
> > >  create mode 100644 src/libcamera/control_ids_rpi.yaml
> > >
> > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > > index 1504f741ae2f..5d20e4d869e3 100644
> > > --- a/include/libcamera/meson.build
> > > +++ b/include/libcamera/meson.build
> > > @@ -36,6 +36,8 @@ controls_map = {
> > >      'controls': {
> > >          'draft': 'control_ids_draft.yaml',
> > >          'core': 'control_ids_core.yaml',
> > > +        'rpi/pisp': 'control_ids_rpi.yaml',
> >
> > Not mainline yet, but I guess it doesn't hurt :)
>
> Eugh ... I  ... <looks away> It's fine ... it's coming up soon right
> ;-)
>
> It certainly doesn't make sense to add this as only vc4 when the only
> control so far is pisp specific, and we want to add this file so there
> is an initial user of the vendor controls.
>
> So I say lets go with it.  It's one line referencing something out of
> tree that will be in-tree as soon as possible. (waiting to see if I get
> shot down on this ...)

I put this in its own patch for this very reason :)
We don't need to merge this right now, it can come later when the Pi 5
pipeline handler gets merged.

Naush

>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> > > +        'rpi/vc4': 'control_ids_rpi.yaml',
> > >      },
> > >
> > >      'properties': {
> > > diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml
> > > new file mode 100644
> > > index 000000000000..abf82098eb12
> > > --- /dev/null
> > > +++ b/src/libcamera/control_ids_rpi.yaml
> > > @@ -0,0 +1,17 @@
> > > +# SPDX-License-Identifier: LGPL-2.1-or-later
> > > +#
> > > +# Copyright (C) 2023, Raspberry Pi Ltd
> > > +#
> > > +%YAML 1.1
> > > +---
> > > +# Raspberry Pi (VC4 and PiSP) specific vendor controls
> > > +vendor: rpi
> > > +controls:
> > > +  - PispConfigDumpFile:
> > > +      type: string
> > > +      description: |
> > > +        Triggers the Raspberry Pi PiSP pipeline handler to generate a JSON
> > > +        formatted dump of the Backend configuration to the filename given by the
> > > +        value of the control.
> > > +
> > > +...
> > > \ No newline at end of file
> > > --
> > > 2.34.1
> > >
Laurent Pinchart Nov. 30, 2023, 12:53 p.m. UTC | #4 
On Tue, Nov 28, 2023 at 10:52:02AM +0000, Naushir Patuck via libcamera-devel wrote:
> On Tue, 28 Nov 2023 at 10:35, Kieran Bingham wrote:
> > Quoting Jacopo Mondi via libcamera-devel (2023-11-27 16:48:54)
> > > On Fri, Nov 24, 2023 at 12:37:12PM +0000, Naushir Patuck via libcamera-devel wrote:
> > > > Add a new control_ids_rpi.yaml file to hold the Raspberry Pi specific
> > > > vendor controls. This contains a single control PispConfigDumpFile that
> > > > will be used in the Pi 5 pipeline handler as a trigger to dump the
> > > > Backend configuration as a JSON file.
> > > >
> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > > > ---
> > > >  include/libcamera/meson.build      |  2 ++
> > > >  src/libcamera/control_ids_rpi.yaml | 17 +++++++++++++++++
> > > >  2 files changed, 19 insertions(+)
> > > >  create mode 100644 src/libcamera/control_ids_rpi.yaml
> > > >
> > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > > > index 1504f741ae2f..5d20e4d869e3 100644
> > > > --- a/include/libcamera/meson.build
> > > > +++ b/include/libcamera/meson.build
> > > > @@ -36,6 +36,8 @@ controls_map = {
> > > >      'controls': {
> > > >          'draft': 'control_ids_draft.yaml',
> > > >          'core': 'control_ids_core.yaml',
> > > > +        'rpi/pisp': 'control_ids_rpi.yaml',
> > >
> > > Not mainline yet, but I guess it doesn't hurt :)
> >
> > Eugh ... I  ... <looks away> It's fine ... it's coming up soon right
> > ;-)
> >
> > It certainly doesn't make sense to add this as only vc4 when the only
> > control so far is pisp specific, and we want to add this file so there
> > is an initial user of the vendor controls.
> >
> > So I say lets go with it.  It's one line referencing something out of
> > tree that will be in-tree as soon as possible. (waiting to see if I get
> > shot down on this ...)
> 
> I put this in its own patch for this very reason :)
> We don't need to merge this right now, it can come later when the Pi 5
> pipeline handler gets merged.

I would also prefer merging it with Pi5 support. It's hard to review the
control definition without seeing the implementation.

> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >
> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > >
> > > > +        'rpi/vc4': 'control_ids_rpi.yaml',
> > > >      },
> > > >
> > > >      'properties': {
> > > > diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml
> > > > new file mode 100644
> > > > index 000000000000..abf82098eb12
> > > > --- /dev/null
> > > > +++ b/src/libcamera/control_ids_rpi.yaml
> > > > @@ -0,0 +1,17 @@
> > > > +# SPDX-License-Identifier: LGPL-2.1-or-later
> > > > +#
> > > > +# Copyright (C) 2023, Raspberry Pi Ltd
> > > > +#
> > > > +%YAML 1.1
> > > > +---
> > > > +# Raspberry Pi (VC4 and PiSP) specific vendor controls
> > > > +vendor: rpi
> > > > +controls:
> > > > +  - PispConfigDumpFile:
> > > > +      type: string
> > > > +      description: |
> > > > +        Triggers the Raspberry Pi PiSP pipeline handler to generate a JSON
> > > > +        formatted dump of the Backend configuration to the filename given by the
> > > > +        value of the control.
> > > > +
> > > > +...
> > > > \ No newline at end of file

Patch
diff mbox series 

diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
index 1504f741ae2f..5d20e4d869e3 100644
--- a/include/libcamera/meson.build
+++ b/include/libcamera/meson.build
@@ -36,6 +36,8 @@  controls_map = {
     'controls': {
         'draft': 'control_ids_draft.yaml',
         'core': 'control_ids_core.yaml',
+        'rpi/pisp': 'control_ids_rpi.yaml',
+        'rpi/vc4': 'control_ids_rpi.yaml',
     },
 
     'properties': {
diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml
new file mode 100644
index 000000000000..abf82098eb12
--- /dev/null
+++ b/src/libcamera/control_ids_rpi.yaml
@@ -0,0 +1,17 @@ 
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
+# Copyright (C) 2023, Raspberry Pi Ltd
+#
+%YAML 1.1
+---
+# Raspberry Pi (VC4 and PiSP) specific vendor controls
+vendor: rpi
+controls:
+  - PispConfigDumpFile:
+      type: string
+      description: |
+        Triggers the Raspberry Pi PiSP pipeline handler to generate a JSON
+        formatted dump of the Backend configuration to the filename given by the
+        value of the control.
+
+...
\ No newline at end of file