[PATCH v3 4/5] include: linux: Add RKISP1_V_IMX8MP version

[Laurent Pinchart]

On Fri, Feb 23, 2024 at 01:26:30PM +0000, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-02-23 13:20:40)
> > On Fri, Feb 23, 2024 at 02:15:14PM +0100, Stefan Klug wrote:
> > > Am 18.02.24 um 17:49 schrieb Laurent Pinchart:
> > > > From: Paul Elder <paul.elder at ideasonboard.com>
> > > > 
> > > > Patches have been posted to the linux-media at vger.kernel.org mailing list
> > > > to add i.MX8MP support to the rkisp1 driver ([1]). As no changes are
> > > > expected to the userspace API in future versions of the series, add the
> > > > RKISP1_V_IMX8MP version manually to the rkisp1-config.h header already.
> > > > Once the patches get merged in the kernel, the changes will trickle down
> > > > to libcamera with the next kernel headers update.
> > > > 
> > > > [1] https://lore.kernel.org/linux-media/20240216095458.2919694-1-paul.elder@ideasonboard.com/
> > > > 
> > > > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > 
> > > Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > > 
> > > > ---
> > > >   include/linux/rkisp1-config.h | 8 +++++---
> > > >   1 file changed, 5 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/include/linux/rkisp1-config.h b/include/linux/rkisp1-config.h
> > > > index ec7cde8cd2e3..2d1c448a6ab8 100644
> > > > --- a/include/linux/rkisp1-config.h
> > > > +++ b/include/linux/rkisp1-config.h
> > > > @@ -4,8 +4,8 @@
> > > >    * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
> > > >    */
> > > >   
> > > > -#ifndef _RKISP1_CONFIG_H
> > > > -#define _RKISP1_CONFIG_H
> > > > +#ifndef _UAPI_RKISP1_CONFIG_H
> > > > +#define _UAPI_RKISP1_CONFIG_H
> > > >   
> > > >   #include <linux/types.h>
> > > >   
> > > > @@ -179,12 +179,14 @@
> > > >    * @RKISP1_V11: declared in the original vendor code, but not used
> > > >    * @RKISP1_V12: used at least in rk3326 and px30
> > > >    * @RKISP1_V13: used at least in rk1808
> > > > + * @RKISP1_V_IMX8MP: used in at least imx8mp
> > > >    */
> > > >   enum rkisp1_cif_isp_version {
> > > >     RKISP1_V10 = 10,
> > > >     RKISP1_V11,
> > > >     RKISP1_V12,
> > > >     RKISP1_V13,
> > > > +   RKISP1_V_IMX8MP,
> > > 
> > > IMHO it might be more flexible in the future to start with a new number. 
> > > eg. RKISP1_V_IMX8MP = 100
> > 
> > I dislike the versioning scheme as well :-(
> > 
> > This being said, Rockchip has developed v2 and v3 versions of the ISP,
> > They are not compatible with v1, so I don't think this driver will ever
> > need to support new Rockchip versions.
> 
> Do you /know/ that for a fact?
> 
> I thought Rockchip's BSP used a single driver for all of it's ISP range?
>  (Not rkisp1)

They do, and we could do so in mainline too, adding support for ISP v1
to a new driver that supports v2 and v3 as well. Doing it the other way
around without breaking any backward compatibility doesn't seem
realistic.

These are all speculations anyway. The bottom line is that without
information about the VSI versioning scheme for current and future ISP
versions, there's nothing we can really do.

> > > >   };
> > > >   
> > > >   enum rkisp1_cif_isp_histogram_mode {
> > > > @@ -992,4 +994,4 @@ struct rkisp1_stat_buffer {
> > > >     struct rkisp1_cif_isp_stat params;
> > > >   };
> > > >   
> > > > -#endif /* _RKISP1_CONFIG_H */
> > > > +#endif /* _UAPI_RKISP1_CONFIG_H */

-- 
Regards,

Laurent Pinchart