| Message ID | 20230505205101.54569-1-sakari.ailus@linux.intel.com |
|---|---|
| Headers | show |
| Series | Random cleanups | expand |
Hi Sakari, Thank you for the patch. On Fri, May 05, 2023 at 11:51:00PM +0300, Sakari Ailus wrote: > Rename meta format files, using "metafmt" prefix instead of "pixfmt-meta". > These are metadata formats, not pixel formats. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: Bingbu Cao <bingbu.cao@intel.com> > Cc: Dafna Hirschfeld <dafna@fastmail.com> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > .../userspace-api/media/v4l/meta-formats.rst | 14 +++++++------- > .../v4l/{pixfmt-meta-d4xx.rst => metafmt-d4xx.rst} | 0 > ...-meta-intel-ipu3.rst => metafmt-intel-ipu3.rst} | 0 > .../{pixfmt-meta-rkisp1.rst => metafmt-rkisp1.rst} | 0 > .../v4l/{pixfmt-meta-uvc.rst => metafmt-uvc.rst} | 0 > .../{pixfmt-meta-vivid.rst => metafmt-vivid.rst} | 0 > ...xfmt-meta-vsp1-hgo.rst => metafmt-vsp1-hgo.rst} | 0 > ...xfmt-meta-vsp1-hgt.rst => metafmt-vsp1-hgt.rst} | 0 > MAINTAINERS | 4 ++-- > 9 files changed, 9 insertions(+), 9 deletions(-) > rename Documentation/userspace-api/media/v4l/{pixfmt-meta-d4xx.rst => metafmt-d4xx.rst} (100%) > rename Documentation/userspace-api/media/v4l/{pixfmt-meta-intel-ipu3.rst => metafmt-intel-ipu3.rst} (100%) > rename Documentation/userspace-api/media/v4l/{pixfmt-meta-rkisp1.rst => metafmt-rkisp1.rst} (100%) > rename Documentation/userspace-api/media/v4l/{pixfmt-meta-uvc.rst => metafmt-uvc.rst} (100%) > rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vivid.rst => metafmt-vivid.rst} (100%) > rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vsp1-hgo.rst => metafmt-vsp1-hgo.rst} (100%) > rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vsp1-hgt.rst => metafmt-vsp1-hgt.rst} (100%) > > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst > index fff25357fe86..0bb61fc5bc00 100644 > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst > @@ -12,10 +12,10 @@ These formats are used for the :ref:`metadata` interface only. > .. toctree:: > :maxdepth: 1 > > - pixfmt-meta-d4xx > - pixfmt-meta-intel-ipu3 > - pixfmt-meta-rkisp1 > - pixfmt-meta-uvc > - pixfmt-meta-vsp1-hgo > - pixfmt-meta-vsp1-hgt > - pixfmt-meta-vivid > + metafmt-d4xx > + metafmt-intel-ipu3 > + metafmt-rkisp1 > + metafmt-uvc > + metafmt-vsp1-hgo > + metafmt-vsp1-hgt > + metafmt-vivid > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-d4xx.rst b/Documentation/userspace-api/media/v4l/metafmt-d4xx.rst > similarity index 100% > rename from Documentation/userspace-api/media/v4l/pixfmt-meta-d4xx.rst > rename to Documentation/userspace-api/media/v4l/metafmt-d4xx.rst > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst b/Documentation/userspace-api/media/v4l/metafmt-intel-ipu3.rst > similarity index 100% > rename from Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst > rename to Documentation/userspace-api/media/v4l/metafmt-intel-ipu3.rst > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst b/Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst > similarity index 100% > rename from Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst > rename to Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-uvc.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc.rst > similarity index 100% > rename from Documentation/userspace-api/media/v4l/pixfmt-meta-uvc.rst > rename to Documentation/userspace-api/media/v4l/metafmt-uvc.rst > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-vivid.rst b/Documentation/userspace-api/media/v4l/metafmt-vivid.rst > similarity index 100% > rename from Documentation/userspace-api/media/v4l/pixfmt-meta-vivid.rst > rename to Documentation/userspace-api/media/v4l/metafmt-vivid.rst > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgo.rst b/Documentation/userspace-api/media/v4l/metafmt-vsp1-hgo.rst > similarity index 100% > rename from Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgo.rst > rename to Documentation/userspace-api/media/v4l/metafmt-vsp1-hgo.rst > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgt.rst b/Documentation/userspace-api/media/v4l/metafmt-vsp1-hgt.rst > similarity index 100% > rename from Documentation/userspace-api/media/v4l/pixfmt-meta-vsp1-hgt.rst > rename to Documentation/userspace-api/media/v4l/metafmt-vsp1-hgt.rst > diff --git a/MAINTAINERS b/MAINTAINERS > index e25ebb7c781b..546826109900 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10359,7 +10359,7 @@ L: linux-media@vger.kernel.org > S: Maintained > F: Documentation/admin-guide/media/ipu3.rst > F: Documentation/admin-guide/media/ipu3_rcb.svg > -F: Documentation/userspace-api/media/v4l/pixfmt-meta-intel-ipu3.rst > +F: Documentation/userspace-api/media/v4l/metafmt-intel-ipu3.rst > F: drivers/staging/media/ipu3/ > > INTEL IXP4XX CRYPTO SUPPORT > @@ -18026,7 +18026,7 @@ L: linux-rockchip@lists.infradead.org > S: Maintained > F: Documentation/admin-guide/media/rkisp1.rst > F: Documentation/devicetree/bindings/media/rockchip-isp1.yaml > -F: Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst > +F: Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst > F: drivers/media/platform/rockchip/rkisp1 > F: include/uapi/linux/rkisp1-config.h >
Hi Sakari, Thank you for the patch. On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote: > Use unsigned int values annoted by "U" for u32 fields. While this is a > good practice, there doesn't appear to be a bug that this patch would fix. > > The patch has been generated using the following command: > > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h How about using the _BITUL() macro from include/uapi/linux/const.h ? > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > include/uapi/linux/media.h | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > index 3ddadaea849f..edb8dfef9eba 100644 > --- a/include/uapi/linux/media.h > +++ b/include/uapi/linux/media.h > @@ -140,8 +140,8 @@ struct media_device_info { > #define MEDIA_ENT_F_DV_ENCODER (MEDIA_ENT_F_BASE + 0x6002) > > /* Entity flags */ > -#define MEDIA_ENT_FL_DEFAULT (1 << 0) > -#define MEDIA_ENT_FL_CONNECTOR (1 << 1) > +#define MEDIA_ENT_FL_DEFAULT (1U << 0) > +#define MEDIA_ENT_FL_CONNECTOR (1U << 1) > > /* OR with the entity id value to find the next entity */ > #define MEDIA_ENT_ID_FLAG_NEXT (1U << 31) > @@ -205,9 +205,9 @@ struct media_entity_desc { > }; > }; > > -#define MEDIA_PAD_FL_SINK (1 << 0) > -#define MEDIA_PAD_FL_SOURCE (1 << 1) > -#define MEDIA_PAD_FL_MUST_CONNECT (1 << 2) > +#define MEDIA_PAD_FL_SINK (1U << 0) > +#define MEDIA_PAD_FL_SOURCE (1U << 1) > +#define MEDIA_PAD_FL_MUST_CONNECT (1U << 2) > > struct media_pad_desc { > __u32 entity; /* entity ID */ > @@ -216,14 +216,14 @@ struct media_pad_desc { > __u32 reserved[2]; > }; > > -#define MEDIA_LNK_FL_ENABLED (1 << 0) > -#define MEDIA_LNK_FL_IMMUTABLE (1 << 1) > -#define MEDIA_LNK_FL_DYNAMIC (1 << 2) > +#define MEDIA_LNK_FL_ENABLED (1U << 0) > +#define MEDIA_LNK_FL_IMMUTABLE (1U << 1) > +#define MEDIA_LNK_FL_DYNAMIC (1U << 2) > > #define MEDIA_LNK_FL_LINK_TYPE (0xf << 28) > -# define MEDIA_LNK_FL_DATA_LINK (0 << 28) > -# define MEDIA_LNK_FL_INTERFACE_LINK (1 << 28) > -# define MEDIA_LNK_FL_ANCILLARY_LINK (2 << 28) > +# define MEDIA_LNK_FL_DATA_LINK (0U << 28) > +# define MEDIA_LNK_FL_INTERFACE_LINK (1U << 28) > +# define MEDIA_LNK_FL_ANCILLARY_LINK (2U << 28) > > struct media_link_desc { > struct media_pad_desc source; > @@ -293,7 +293,7 @@ struct media_links_enum { > * struct media_device_info. > */ > #define MEDIA_V2_ENTITY_HAS_FLAGS(media_version) \ > - ((media_version) >= ((4 << 16) | (19 << 8) | 0)) > + ((media_version) >= ((4U << 16) | (19U << 8) | 0)) > > struct media_v2_entity { > __u32 id; > @@ -328,7 +328,7 @@ struct media_v2_interface { > * struct media_device_info. > */ > #define MEDIA_V2_PAD_HAS_INDEX(media_version) \ > - ((media_version) >= ((4 << 16) | (19 << 8) | 0)) > + ((media_version) >= ((4U << 16) | (19U << 8) | 0)) > > struct media_v2_pad { > __u32 id; > @@ -432,7 +432,7 @@ struct media_v2_topology { > #define MEDIA_INTF_T_ALSA_TIMER (MEDIA_INTF_T_ALSA_BASE + 7) > > /* Obsolete symbol for media_version, no longer used in the kernel */ > -#define MEDIA_API_VERSION ((0 << 16) | (1 << 8) | 0) > +#define MEDIA_API_VERSION ((0U << 16) | (1U << 8) | 0) > > #endif >
Hi Laurent, On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote: > > Use unsigned int values annoted by "U" for u32 fields. While this is a > > good practice, there doesn't appear to be a bug that this patch would fix. > > > > The patch has been generated using the following command: > > > > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h > > How about using the _BITUL() macro from include/uapi/linux/const.h ? These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32 bits on all platforms where Linux is used AFAIK. And thanks for the review!
Hi Sakari, On Mon, May 08, 2023 at 09:19:24AM +0300, Sakari Ailus wrote: > On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote: > > On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote: > > > Use unsigned int values annoted by "U" for u32 fields. While this is a > > > good practice, there doesn't appear to be a bug that this patch would fix. > > > > > > The patch has been generated using the following command: > > > > > > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h > > > > How about using the _BITUL() macro from include/uapi/linux/const.h ? > > These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32 > bits on all platforms where Linux is used AFAIK. I know, but is it a problem ? > And thanks for the review!
Hi Laurent, On Mon, May 08, 2023 at 09:30:23AM +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Mon, May 08, 2023 at 09:19:24AM +0300, Sakari Ailus wrote: > > On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote: > > > On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote: > > > > Use unsigned int values annoted by "U" for u32 fields. While this is a > > > > good practice, there doesn't appear to be a bug that this patch would fix. > > > > > > > > The patch has been generated using the following command: > > > > > > > > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h > > > > > > How about using the _BITUL() macro from include/uapi/linux/const.h ? > > > > These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32 > > bits on all platforms where Linux is used AFAIK. > > I know, but is it a problem ? If we have a u32 field, unsigned int is the right type for that (from non-fixed length C types), not unsigned long. In practice it would work, I have no doubts about that. The compiler could still do different decisions due to this, promoting values to a 64-bits for instance. If we had _BITU(), I'd be happy to use that. :-) How about this: let's merge this patch and then see how a _BITU() macro would fare.
Hi Sakari, On Mon, May 08, 2023 at 09:58:48AM +0300, Sakari Ailus wrote: > On Mon, May 08, 2023 at 09:30:23AM +0300, Laurent Pinchart wrote: > > On Mon, May 08, 2023 at 09:19:24AM +0300, Sakari Ailus wrote: > > > On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote: > > > > On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote: > > > > > Use unsigned int values annoted by "U" for u32 fields. While this is a > > > > > good practice, there doesn't appear to be a bug that this patch would fix. > > > > > > > > > > The patch has been generated using the following command: > > > > > > > > > > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h > > > > > > > > How about using the _BITUL() macro from include/uapi/linux/const.h ? > > > > > > These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32 > > > bits on all platforms where Linux is used AFAIK. > > > > I know, but is it a problem ? > > If we have a u32 field, unsigned int is the right type for that (from > non-fixed length C types), not unsigned long. In practice it would work, I > have no doubts about that. The compiler could still do different decisions > due to this, promoting values to a 64-bits for instance. > > If we had _BITU(), I'd be happy to use that. :-) Note how BIT() is defined in include/vdso/bits.h: #include <vdso/const.h> #define BIT(nr) (UL(1) << (nr)) And in include/vdso/const.h: #include <uapi/linux/const.h> #define UL(x) (_UL(x)) BIT() is thus essentially identical to _BITUL(). As we use the former everywhere without any trouble, I wouldn't expect issue with the latter. > How about this: let's merge this patch and then see how a _BITU() macro > would fare.
On Mon, May 08, 2023 at 10:52:09AM +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Mon, May 08, 2023 at 09:58:48AM +0300, Sakari Ailus wrote: > > On Mon, May 08, 2023 at 09:30:23AM +0300, Laurent Pinchart wrote: > > > On Mon, May 08, 2023 at 09:19:24AM +0300, Sakari Ailus wrote: > > > > On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote: > > > > > On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote: > > > > > > Use unsigned int values annoted by "U" for u32 fields. While this is a > > > > > > good practice, there doesn't appear to be a bug that this patch would fix. > > > > > > > > > > > > The patch has been generated using the following command: > > > > > > > > > > > > perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h > > > > > > > > > > How about using the _BITUL() macro from include/uapi/linux/const.h ? > > > > > > > > These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32 > > > > bits on all platforms where Linux is used AFAIK. > > > > > > I know, but is it a problem ? > > > > If we have a u32 field, unsigned int is the right type for that (from > > non-fixed length C types), not unsigned long. In practice it would work, I > > have no doubts about that. The compiler could still do different decisions > > due to this, promoting values to a 64-bits for instance. > > > > If we had _BITU(), I'd be happy to use that. :-) > > Note how BIT() is defined in include/vdso/bits.h: > > #include <vdso/const.h> > > #define BIT(nr) (UL(1) << (nr)) > > And in include/vdso/const.h: > > #include <uapi/linux/const.h> > > #define UL(x) (_UL(x)) > > BIT() is thus essentially identical to _BITUL(). As we use the former > everywhere without any trouble, I wouldn't expect issue with the latter. Fair enough. I'll switch to _BITUL() in v2.
Hi all, These three patches are seemingly random cleanups but are a pre-requisite for another set for supporting generic metadata formats. I'll post that set separately. These three should be merged independently though.. Sakari Ailus (3): media: mc: Make media_get_pad_index() use pad type flag media: Documentation: Rename meta format files media: uapi: Use unsigned int values for assigning bits in u32 fields .../userspace-api/media/v4l/meta-formats.rst | 14 +++---- ...{pixfmt-meta-d4xx.rst => metafmt-d4xx.rst} | 0 ...-intel-ipu3.rst => metafmt-intel-ipu3.rst} | 0 ...fmt-meta-rkisp1.rst => metafmt-rkisp1.rst} | 0 .../{pixfmt-meta-uvc.rst => metafmt-uvc.rst} | 0 ...ixfmt-meta-vivid.rst => metafmt-vivid.rst} | 0 ...meta-vsp1-hgo.rst => metafmt-vsp1-hgo.rst} | 0 ...meta-vsp1-hgt.rst => metafmt-vsp1-hgt.rst} | 0 MAINTAINERS | 4 +- drivers/media/dvb-core/dvbdev.c | 4 +- drivers/media/mc/mc-entity.c | 16 +++----- drivers/media/usb/au0828/au0828-core.c | 2 +- drivers/media/v4l2-core/v4l2-mc.c | 38 ++++++++++++------- include/media/media-entity.h | 4 +- include/uapi/linux/media.h | 28 +++++++------- 15 files changed, 58 insertions(+), 52 deletions(-) rename Documentation/userspace-api/media/v4l/{pixfmt-meta-d4xx.rst => metafmt-d4xx.rst} (100%) rename Documentation/userspace-api/media/v4l/{pixfmt-meta-intel-ipu3.rst => metafmt-intel-ipu3.rst} (100%) rename Documentation/userspace-api/media/v4l/{pixfmt-meta-rkisp1.rst => metafmt-rkisp1.rst} (100%) rename Documentation/userspace-api/media/v4l/{pixfmt-meta-uvc.rst => metafmt-uvc.rst} (100%) rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vivid.rst => metafmt-vivid.rst} (100%) rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vsp1-hgo.rst => metafmt-vsp1-hgo.rst} (100%) rename Documentation/userspace-api/media/v4l/{pixfmt-meta-vsp1-hgt.rst => metafmt-vsp1-hgt.rst} (100%)