| Message ID | 20200901111612.10552-1-dafna.hirschfeld@collabora.com |
|---|---|
| Headers | show |
| Series | media: staging: rkisp1: add support to V4L2_CAP_IO_MC | expand |
On 9/1/20 8:16 AM, Dafna Hirschfeld wrote: > Add support to more YUV encoded media bus formats on the resizer's > source pad. The patch defines an array rkisp1_rsz_yuv_formats[] > with the list of supported YUV media bus formats and their {hv}div > values. The {hv}div are used in the function 'rkisp1_rsz_config' > instead of the macros RKISP1_MBUS_FMT_(HV)DIV, and instead of > checking the capture format. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> Acked-by: Helen Koike <helen.koike@collabora.com> Thanks Helen > --- > drivers/staging/media/rkisp1/rkisp1-resizer.c | 67 ++++++++++++++----- > 1 file changed, 49 insertions(+), 18 deletions(-) > > diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c > index 0e4a2c931ab0..84325d158355 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c > +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c > @@ -16,8 +16,36 @@ > #define RKISP1_DEF_FMT MEDIA_BUS_FMT_YUYV8_2X8 > #define RKISP1_DEF_PIXEL_ENC V4L2_PIXEL_ENC_YUV > > -#define RKISP1_MBUS_FMT_HDIV 2 > -#define RKISP1_MBUS_FMT_VDIV 1 > +struct rkisp1_rsz_yuv_mbus_info { > + u32 mbus_code; > + u32 hdiv; > + u32 vdiv; > +}; > + > +static const struct rkisp1_rsz_yuv_mbus_info rkisp1_rsz_yuv_src_formats[] = { > + { > + .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8, /* YUV422 */ > + .hdiv = 2, > + .vdiv = 1, > + }, > + { > + .mbus_code = MEDIA_BUS_FMT_YUYV8_1_5X8, /* YUV420 */ > + .hdiv = 2, > + .vdiv = 2, > + }, > +}; > + > +static const struct rkisp1_rsz_yuv_mbus_info *rkisp1_rsz_get_yuv_mbus_info(u32 mbus_code) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(rkisp1_rsz_yuv_src_formats); i++) { > + if (rkisp1_rsz_yuv_src_formats[i].mbus_code == mbus_code) > + return &rkisp1_rsz_yuv_src_formats[i]; > + } > + > + return NULL; > +} > > enum rkisp1_shadow_regs_when { > RKISP1_SHADOW_REGS_SYNC, > @@ -361,16 +389,19 @@ static void rkisp1_rsz_config_regs(struct rkisp1_resizer *rsz, > static void rkisp1_rsz_config(struct rkisp1_resizer *rsz, > enum rkisp1_shadow_regs_when when) > { > - u8 hdiv = RKISP1_MBUS_FMT_HDIV, vdiv = RKISP1_MBUS_FMT_VDIV; > + const struct rkisp1_rsz_yuv_mbus_info *sink_yuv_info, *src_yuv_info; > struct v4l2_rect sink_y, sink_c, src_y, src_c; > - struct v4l2_mbus_framefmt *src_fmt; > + struct v4l2_mbus_framefmt *src_fmt, *sink_fmt; > struct v4l2_rect *sink_crop; > - struct rkisp1_capture *cap = &rsz->rkisp1->capture_devs[rsz->id]; > > sink_crop = rkisp1_rsz_get_pad_crop(rsz, NULL, RKISP1_RSZ_PAD_SINK, > V4L2_SUBDEV_FORMAT_ACTIVE); > src_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SRC, > V4L2_SUBDEV_FORMAT_ACTIVE); > + src_yuv_info = rkisp1_rsz_get_yuv_mbus_info(src_fmt->code); > + sink_fmt = rkisp1_rsz_get_pad_fmt(rsz, NULL, RKISP1_RSZ_PAD_SINK, > + V4L2_SUBDEV_FORMAT_ACTIVE); > + sink_yuv_info = rkisp1_rsz_get_yuv_mbus_info(sink_fmt->code); > > /* > * The resizer only works on yuv formats, > @@ -386,25 +417,17 @@ static void rkisp1_rsz_config(struct rkisp1_resizer *rsz, > src_y.width = src_fmt->width; > src_y.height = src_fmt->height; > > - sink_c.width = sink_y.width / RKISP1_MBUS_FMT_HDIV; > - sink_c.height = sink_y.height / RKISP1_MBUS_FMT_VDIV; > + sink_c.width = sink_y.width / sink_yuv_info->hdiv; > + sink_c.height = sink_y.height / sink_yuv_info->vdiv; > > /* > * The resizer is used not only to change the dimensions of the frame > * but also to change the scale for YUV formats, > * (4:2:2 -> 4:2:0 for example). So the width/height of the CbCr > - * streams should be set according to the pixel format in the capture. > - * The resizer always gets the input as YUV422. If the capture format > - * is RGB then the memory input should be YUV422 so we don't change the > - * default hdiv, vdiv in that case. > + * streams should be set according to the media bus format in the src pad. > */ > - if (v4l2_is_format_yuv(cap->pix.info)) { > - hdiv = cap->pix.info->hdiv; > - vdiv = cap->pix.info->vdiv; > - } > - > - src_c.width = src_y.width / hdiv; > - src_c.height = src_y.height / vdiv; > + src_c.width = src_y.width / src_yuv_info->hdiv; > + src_c.height = src_y.height / src_yuv_info->vdiv; > > if (sink_c.width == src_c.width && sink_c.height == src_c.height) { > rkisp1_rsz_disable(rsz, when); > @@ -496,9 +519,17 @@ static void rkisp1_rsz_set_src_fmt(struct rkisp1_resizer *rsz, > struct v4l2_mbus_framefmt *format, > unsigned int which) > { > + const struct rkisp1_isp_mbus_info *mbus_info; > struct v4l2_mbus_framefmt *src_fmt; > > src_fmt = rkisp1_rsz_get_pad_fmt(rsz, cfg, RKISP1_RSZ_PAD_SRC, which); > + mbus_info = rkisp1_isp_mbus_info_get(src_fmt->code); > + > + /* for YUV formats, userspace can change the mbus code on the src pad if it is supported */ > + if (mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV && > + rkisp1_rsz_get_yuv_mbus_info(format->code)) > + src_fmt->code = format->code; > + > src_fmt->width = clamp_t(u32, format->width, > rsz->config->min_rsz_width, > rsz->config->max_rsz_width); >
typo in the title s/compering/comparing On 9/1/20 8:16 AM, Dafna Hirschfeld wrote: > The capture has a mapping of the mbus code needed for each pixelformat. > This can be used to simplify the link validation by comparing the mbus > code in the capture with the code in the resizer. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> Acked-by: Helen Koike <helen.koike@collabora.com> Thanks Helen > --- > drivers/staging/media/rkisp1/rkisp1-capture.c | 18 ++++-------------- > 1 file changed, 4 insertions(+), 14 deletions(-) > > diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c > index 531381e1801a..79ee6795a58f 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-capture.c > +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c > @@ -1265,22 +1265,11 @@ static int rkisp1_capture_link_validate(struct media_link *link) > struct v4l2_subdev *sd = > media_entity_to_v4l2_subdev(link->source->entity); > struct rkisp1_capture *cap = video_get_drvdata(vdev); > - struct rkisp1_isp *isp = &cap->rkisp1->isp; > - u8 isp_pix_enc = isp->src_fmt->pixel_enc; > - u8 cap_pix_enc = cap->pix.info->pixel_enc; > + const struct rkisp1_capture_fmt_cfg *fmt = > + rkisp1_find_fmt_cfg(cap, cap->pix.fmt.pixelformat); > struct v4l2_subdev_format sd_fmt; > int ret; > > - if (cap_pix_enc != isp_pix_enc && > - !(isp_pix_enc == V4L2_PIXEL_ENC_YUV && > - cap_pix_enc == V4L2_PIXEL_ENC_RGB)) { > - dev_err(cap->rkisp1->dev, > - "format type mismatch in link '%s:%d->%s:%d'\n", > - link->source->entity->name, link->source->index, > - link->sink->entity->name, link->sink->index); > - return -EPIPE; > - } > - > sd_fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE; > sd_fmt.pad = link->source->index; > ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &sd_fmt); > @@ -1288,7 +1277,8 @@ static int rkisp1_capture_link_validate(struct media_link *link) > return ret; > > if (sd_fmt.format.height != cap->pix.fmt.height || > - sd_fmt.format.width != cap->pix.fmt.width) > + sd_fmt.format.width != cap->pix.fmt.width || > + sd_fmt.format.code != fmt->mbus) > return -EPIPE; > > return 0; >
Hi Dafna, On Tue, Sep 01, 2020 at 01:16:02PM +0200, Dafna Hirschfeld wrote: > The patchset solves several problems in the rkisp1 driver. > > 1. Currently the resizers output media code MEDIA_BUS_FMT_YUYV8_2X8 when the input is > MEDIA_BUS_FMT_YUYV8_2X8. > The patchset adds support to other media codes on the resizer according to > the chroma subsampling. > Setting the correct media code on the source pad that matches the > chroma subsampling reflects userspace that the resizer has downsampling > capability and also the resizer entity does not have to check the capture entity's > configuration to get the scaling ratio, the information of how to scale can be > obtained from the source media code of the resizer. > > 2. Add support for the V4L2_CAP_IO_MC capability on > the mainpath and selfpath captures. This helps userspace to know the > right configuration for streaming. This is especially helpful for the > RGB and Grey formats that expect media bus MEDIA_BUS_FMT_YUYV8_2X8 > which is not something userspace can 'guess'. Adding a mapping of the > required mbus code for each pixelformat also makes the link_validation > code much simpler, it just has to check if the configuration matches the mapping. > > 3. Removes unsupported packed yuv formats - this patch was already part of a pull request > and was dropped due to merge conflicts. > > 4. Remove bayer formats on the selfpath resizer since they are not > supported on the selfpath capture. > > 5. Remove support to YUV444 pixel format, I was not able to find a configuration > that supports this format. I kept getting bad looking frames. > I tried to add capture yuv444 formats by adding an entry: > > + { > + .mbus_code = MEDIA_BUS_FMT_YUV8_1X24, > + .hdiv = 1, > + .vdiv = 1, > + } > > to the list of supported formats: rkisp1_rsz_yuv_src_formats[] > > full patch: http://ix.io/2vNJ > > On the mainpath I get good images, but on the selfpath I get bad looking images: > > https://pasteboard.co/JoWp3U4.png > > https://pasteboard.co/Jp1YWLR.png > > Interestingly, when changing the sp_input from default RKISP1_MI_CTRL_SP_INPUT_YUV422 > to RKISP1_MI_CTRL_SP_INPUT_YUV444, then the images that are not upscaled look good: > > https://pasteboard.co/Jp22u6E.png > > but with upscaling (1604x1232 -> 1920x1500) it still looks bad: > > https://pasteboard.co/Jp22MBU.png > > > 6. Fix the configuration to support Grey format - the 'write_format' field should > be 'planar' > > changes since v3: > patch 1 - remove '----' line from commit log > patch 5-7 - refactor code, add documentation > patch 8 - change function name rkisp1_rsz_yuv_mbus_info and code in function rkisp1_rsz_set_src_fmt > > > Dafna Hirschfeld (10): > media: staging: rkisp1: cap: change RGB24 format to XBGR32 > media: staging: rkisp1: cap: remove unsupported formats > media: staging: rkisp1: cap: remove unsupported format YUV444 > media: staging: rkisp1: don't support bayer format on selfpath resizer > media: staging: rkisp1: add capability V4L2_CAP_IO_MC to capture > devices > media: staging: rkisp1: add a helper function to enumerate supported > mbus formats on capture > media: staging: rkisp1: rsz: enumerate the formats on the src pad > according to the capture > media: staging: rkisp1: rsz: Add support to more YUV encoded mbus > codes on src pad > media: staging: rkisp1: cap: simplify the link validation by compering > the media bus code > media: staging: rkisp1: fix configuration for GREY pixelformat > > drivers/staging/media/rkisp1/rkisp1-capture.c | 199 +++++++++++------- > drivers/staging/media/rkisp1/rkisp1-common.h | 11 + > drivers/staging/media/rkisp1/rkisp1-resizer.c | 93 ++++++-- > 3 files changed, 203 insertions(+), 100 deletions(-) > Except one comment for the YUV 4:2:2 format removal patch, Reviewed-by: Tomasz Figa <tfiga@chromium.org> Best regards, Tomasz
The patchset solves several problems in the rkisp1 driver. 1. Currently the resizers output media code MEDIA_BUS_FMT_YUYV8_2X8 when the input is MEDIA_BUS_FMT_YUYV8_2X8. The patchset adds support to other media codes on the resizer according to the chroma subsampling. Setting the correct media code on the source pad that matches the chroma subsampling reflects userspace that the resizer has downsampling capability and also the resizer entity does not have to check the capture entity's configuration to get the scaling ratio, the information of how to scale can be obtained from the source media code of the resizer. 2. Add support for the V4L2_CAP_IO_MC capability on the mainpath and selfpath captures. This helps userspace to know the right configuration for streaming. This is especially helpful for the RGB and Grey formats that expect media bus MEDIA_BUS_FMT_YUYV8_2X8 which is not something userspace can 'guess'. Adding a mapping of the required mbus code for each pixelformat also makes the link_validation code much simpler, it just has to check if the configuration matches the mapping. 3. Removes unsupported packed yuv formats - this patch was already part of a pull request and was dropped due to merge conflicts. 4. Remove bayer formats on the selfpath resizer since they are not supported on the selfpath capture. 5. Remove support to YUV444 pixel format, I was not able to find a configuration that supports this format. I kept getting bad looking frames. I tried to add capture yuv444 formats by adding an entry: + { + .mbus_code = MEDIA_BUS_FMT_YUV8_1X24, + .hdiv = 1, + .vdiv = 1, + } to the list of supported formats: rkisp1_rsz_yuv_src_formats[] full patch: http://ix.io/2vNJ On the mainpath I get good images, but on the selfpath I get bad looking images: https://pasteboard.co/JoWp3U4.png https://pasteboard.co/Jp1YWLR.png Interestingly, when changing the sp_input from default RKISP1_MI_CTRL_SP_INPUT_YUV422 to RKISP1_MI_CTRL_SP_INPUT_YUV444, then the images that are not upscaled look good: https://pasteboard.co/Jp22u6E.png but with upscaling (1604x1232 -> 1920x1500) it still looks bad: https://pasteboard.co/Jp22MBU.png 6. Fix the configuration to support Grey format - the 'write_format' field should be 'planar' changes since v3: patch 1 - remove '----' line from commit log patch 5-7 - refactor code, add documentation patch 8 - change function name rkisp1_rsz_yuv_mbus_info and code in function rkisp1_rsz_set_src_fmt Dafna Hirschfeld (10): media: staging: rkisp1: cap: change RGB24 format to XBGR32 media: staging: rkisp1: cap: remove unsupported formats media: staging: rkisp1: cap: remove unsupported format YUV444 media: staging: rkisp1: don't support bayer format on selfpath resizer media: staging: rkisp1: add capability V4L2_CAP_IO_MC to capture devices media: staging: rkisp1: add a helper function to enumerate supported mbus formats on capture media: staging: rkisp1: rsz: enumerate the formats on the src pad according to the capture media: staging: rkisp1: rsz: Add support to more YUV encoded mbus codes on src pad media: staging: rkisp1: cap: simplify the link validation by compering the media bus code media: staging: rkisp1: fix configuration for GREY pixelformat drivers/staging/media/rkisp1/rkisp1-capture.c | 199 +++++++++++------- drivers/staging/media/rkisp1/rkisp1-common.h | 11 + drivers/staging/media/rkisp1/rkisp1-resizer.c | 93 ++++++-- 3 files changed, 203 insertions(+), 100 deletions(-)