On 5/20/20 6:54 PM, Helen Koike wrote:
> Hi Dafna,
>
> On 5/15/20 11:29 AM, Dafna Hirschfeld wrote:
>> The resize block of rkisp1 always get the input as yuv422.
>> Instead of defining the default hdiv, vdiv as macros, the
>> code is more clear if it takes the (hv)div from the YUV422P
>> info. This makes it clear where those values come from.
>> The patch also adds documentation to explain that.
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
>
> Acked-by: Helen Koike <helen.koike@xxxxxxxxxxxxx>
>
> Thanks!
> Helen
>
>> ---
>> drivers/staging/media/rkisp1/rkisp1-resizer.c | 25 +++++++++----------
>> 1 file changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
>> index d049374413dc..04a29af8cc92 100644
>> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
>> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
>> @@ -16,9 +16,6 @@
>> #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
>> -
>> enum rkisp1_shadow_regs_when {
>> RKISP1_SHADOW_REGS_SYNC,
>> RKISP1_SHADOW_REGS_ASYNC,
>> @@ -361,11 +358,12 @@ 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;
>> struct v4l2_rect sink_y, sink_c, src_y, src_c;
>> struct v4l2_mbus_framefmt *src_fmt;
>> struct v4l2_rect *sink_crop;
>> struct rkisp1_capture *cap = &rsz->rkisp1->capture_devs[rsz->id];
>> + const struct v4l2_format_info *yuv422_info =
>> + v4l2_format_info(V4L2_PIX_FMT_YUV422P);
>>
>> sink_crop = rkisp1_rsz_get_pad_crop(rsz, NULL, RKISP1_RSZ_PAD_SINK,
>> V4L2_SUBDEV_FORMAT_ACTIVE);
>> @@ -386,8 +384,9 @@ 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;
>> + /* The input format of the resizer is always yuv422 */
>> + sink_c.width = sink_y.width / yuv422_info->hdiv;
>> + sink_c.height = sink_y.height / yuv422_info->vdiv;
>>
>> /*
>> * The resizer is used not only to change the dimensions of the frame
>> @@ -395,17 +394,17 @@ static void rkisp1_rsz_config(struct rkisp1_resizer *rsz,
>> * (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.
>> + * is RGB then the memory input (the resizer output) should be YUV422
>> + * so we use the hdiv, vdiv of the YUV422 info in this case.
>> */
>> if (v4l2_is_format_yuv(cap->pix.info)) {
>> - hdiv = cap->pix.info->hdiv;
>> - vdiv = cap->pix.info->vdiv;
>> + src_c.width = cap->pix.info->hdiv;
>> + src_c.height = cap->pix.info->vdiv;
Sorry, I just noticed you are assigning hdiv and vdiv directly to width and height, which looks wrong.
It should be:
src_c.width = src_y.width / cap->pix.info->hdiv;
src_c.height = src_y.height / cap->pix.info->vdiv;
Regards,
Helen
>> + } else {
>> + src_c.width = src_y.width / yuv422_info->hdiv;
>> + src_c.height = src_y.height / yuv422_info->vdiv;
>> }
>>
>> - src_c.width = src_y.width / hdiv;
>> - src_c.height = src_y.height / vdiv;
>> -
>> if (sink_c.width == src_c.width && sink_c.height == src_c.height) {
>> rkisp1_rsz_disable(rsz, when);
>> return;
>>
