| Message ID | 20220221111611.52905-1-jacopo@jmondi.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx: imx8mp-mipi-csi2: Remove YUV422 2X8 | expand |
Hi Jacopo, Thank you for the patch. On Mon, Feb 21, 2022 at 12:16:11PM +0100, Jacopo Mondi wrote: > The 2X8 variants of MEDIA_BUS_FMT_YUYV8_2X8 does not apply to serial Maybe s/serial/CSI-2/ > busses. > > Drop it and while at it beautify the formats declaration list a little > by putting the opening curly brace after the comment. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > drivers/staging/media/imx/imx8mq-mipi-csi2.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c > index 56ef3b3b2906..a3e33ce83c1f 100644 > --- a/drivers/staging/media/imx/imx8mq-mipi-csi2.c > +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c > @@ -200,11 +200,9 @@ static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = { > }, { > .code = MEDIA_BUS_FMT_SRGGB14_1X14, > .width = 14, > - }, { > + }, > /* YUV formats */ > - .code = MEDIA_BUS_FMT_YUYV8_2X8, > - .width = 16, > - }, { > + { > .code = MEDIA_BUS_FMT_YUYV8_1X16, While at it, should we also switch this to MEDIA_BUS_FMT_UYVY8_1X16, which is the canonical format for YUV422 8-bit on CSI-2 ? > .width = 16, > }
Hi Laurent On Mon, Feb 21, 2022 at 01:26:45PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Mon, Feb 21, 2022 at 12:16:11PM +0100, Jacopo Mondi wrote: > > The 2X8 variants of MEDIA_BUS_FMT_YUYV8_2X8 does not apply to serial > > Maybe s/serial/CSI-2/ > > > busses. > > > > Drop it and while at it beautify the formats declaration list a little > > by putting the opening curly brace after the comment. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > drivers/staging/media/imx/imx8mq-mipi-csi2.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c > > index 56ef3b3b2906..a3e33ce83c1f 100644 > > --- a/drivers/staging/media/imx/imx8mq-mipi-csi2.c > > +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c > > @@ -200,11 +200,9 @@ static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = { > > }, { > > .code = MEDIA_BUS_FMT_SRGGB14_1X14, > > .width = 14, > > - }, { > > + }, > > /* YUV formats */ > > - .code = MEDIA_BUS_FMT_YUYV8_2X8, > > - .width = 16, > > - }, { > > + { > > .code = MEDIA_BUS_FMT_YUYV8_1X16, > > While at it, should we also switch this to MEDIA_BUS_FMT_UYVY8_1X16, > which is the canonical format for YUV422 8-bit on CSI-2 ? > Should I ? I forgot why UYVY is preferred over YUYV. Also this will break quite some scripts for many users :) Should I rather add it instead ? No problem for me in either cases > > .width = 16, > > } > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Mon, Feb 21, 2022 at 12:29:15PM +0100, Jacopo Mondi wrote: > On Mon, Feb 21, 2022 at 01:26:45PM +0200, Laurent Pinchart wrote: > > On Mon, Feb 21, 2022 at 12:16:11PM +0100, Jacopo Mondi wrote: > > > The 2X8 variants of MEDIA_BUS_FMT_YUYV8_2X8 does not apply to serial > > > > Maybe s/serial/CSI-2/ > > > > > busses. > > > > > > Drop it and while at it beautify the formats declaration list a little > > > by putting the opening curly brace after the comment. > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > drivers/staging/media/imx/imx8mq-mipi-csi2.c | 6 ++---- > > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c > > > index 56ef3b3b2906..a3e33ce83c1f 100644 > > > --- a/drivers/staging/media/imx/imx8mq-mipi-csi2.c > > > +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c > > > @@ -200,11 +200,9 @@ static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = { > > > }, { > > > .code = MEDIA_BUS_FMT_SRGGB14_1X14, > > > .width = 14, > > > - }, { > > > + }, > > > /* YUV formats */ > > > - .code = MEDIA_BUS_FMT_YUYV8_2X8, > > > - .width = 16, > > > - }, { > > > + { > > > .code = MEDIA_BUS_FMT_YUYV8_1X16, > > > > While at it, should we also switch this to MEDIA_BUS_FMT_UYVY8_1X16, > > which is the canonical format for YUV422 8-bit on CSI-2 ? > > > > Should I ? > I forgot why UYVY is preferred over YUYV. It's a convention, we picked UYVY for CSI-2 a long time ago. Sensor drivers should match that convention. > Also this will break quite some scripts for many users :) > Should I rather add it instead ? We can also support the non-standard YUYV order if the sensor does as well, so I don't mind having both options here. The important part is to have UYVY. > No problem for me in either cases > > > > .width = 16, > > > }
On Mon, Feb 21, 2022 at 5:17 AM Jacopo Mondi <jacopo@jmondi.org> wrote: > > The 2X8 variants of MEDIA_BUS_FMT_YUYV8_2X8 does not apply to serial > busses. > > Drop it and while at it beautify the formats declaration list a little > by putting the opening curly brace after the comment. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > drivers/staging/media/imx/imx8mq-mipi-csi2.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c > index 56ef3b3b2906..a3e33ce83c1f 100644 > --- a/drivers/staging/media/imx/imx8mq-mipi-csi2.c > +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c The subject says imx8mp but the file modified is imx8mq. I assume the subject is a typo. To my knowledge, the 8mp uses the same csis as the 8mm. adam > @@ -200,11 +200,9 @@ static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = { > }, { > .code = MEDIA_BUS_FMT_SRGGB14_1X14, > .width = 14, > - }, { > + }, > /* YUV formats */ > - .code = MEDIA_BUS_FMT_YUYV8_2X8, > - .width = 16, > - }, { > + { > .code = MEDIA_BUS_FMT_YUYV8_1X16, > .width = 16, > } > -- > 2.35.0 >
diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c index 56ef3b3b2906..a3e33ce83c1f 100644 --- a/drivers/staging/media/imx/imx8mq-mipi-csi2.c +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c @@ -200,11 +200,9 @@ static const struct csi2_pix_format imx8mq_mipi_csi_formats[] = { }, { .code = MEDIA_BUS_FMT_SRGGB14_1X14, .width = 14, - }, { + }, /* YUV formats */ - .code = MEDIA_BUS_FMT_YUYV8_2X8, - .width = 16, - }, { + { .code = MEDIA_BUS_FMT_YUYV8_1X16, .width = 16, }
The 2X8 variants of MEDIA_BUS_FMT_YUYV8_2X8 does not apply to serial busses. Drop it and while at it beautify the formats declaration list a little by putting the opening curly brace after the comment. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- drivers/staging/media/imx/imx8mq-mipi-csi2.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)