Hi, This patch seems complicated and I do not understand your motives. Could you explain what is the problem with the current aligning of the values? Is this a hardware problem? Which MFC version does it affect? Is it a software problem? If so, maybe the user space application should take extra care on what value it passes/receives to try_fmt? > From: Kiran AVND [mailto:avnd.kiran@xxxxxxxxxxx] > Sent: Friday, September 26, 2014 6:52 AM > To: linux-media@xxxxxxxxxxxxxxx > Cc: k.debski@xxxxxxxxxxx; wuchengli@xxxxxxxxxxxx; posciak@xxxxxxxxxxxx; > arun.m@xxxxxxxxxxx; ihf@xxxxxxxxxxxx; prathyush.k@xxxxxxxxxxx; > arun.kk@xxxxxxxxxxx; kiran@xxxxxxxxxxxx > Subject: [PATCH v2 14/14] [media] s5p-mfc: Don't change the image size > to smaller than the request. > > From: Wu-Cheng Li <wuchengli@xxxxxxxxxxxx> > > Use the requested size as the minimum bound, unless it's less than the > required hardware minimum. The bound align function will align to the > closest value but we do not want to adjust below the requested size. This patch does also change the alignment. This is not mentioned in the commit message (!). It was 2, now it enforces 16. Could you justify this? If I remember correctly having even number was enough for MFC v5 encoder to work properly. > Signed-off-by: Wu-Cheng Li <wuchengli@xxxxxxxxxxxx> > Signed-off-by: Kiran AVND <avnd.kiran@xxxxxxxxxxx> > --- > drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c > b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c > index 407dc63..7b48180 100644 > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c > @@ -1056,6 +1056,7 @@ static int vidioc_try_fmt(struct file *file, void > *priv, struct v4l2_format *f) > struct s5p_mfc_dev *dev = video_drvdata(file); > struct s5p_mfc_fmt *fmt; > struct v4l2_pix_format_mplane *pix_fmt_mp = &f->fmt.pix_mp; > + u32 min_w, min_h; > > if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { > fmt = find_format(f, MFC_FMT_ENC); > @@ -1090,8 +1091,16 @@ static int vidioc_try_fmt(struct file *file, > void *priv, struct v4l2_format *f) > return -EINVAL; > } > > - v4l_bound_align_image(&pix_fmt_mp->width, 8, 1920, 1, > - &pix_fmt_mp->height, 4, 1080, 1, 0); > + /* > + * Use the requested size as the minimum bound, unless it's > less > + * than the required hardware minimum. The bound align > function > + * will align to the closest value but we do not want to > adjust > + * below the requested size. Other drivers use v4l2_bound_align and user space apps can cope with the driver returning a value that is below the requested value. > + */ > + min_w = min(max(16u, pix_fmt_mp->width), 1920u); > + min_h = min(max(16u, pix_fmt_mp->height), 1088u); > + v4l_bound_align_image(&pix_fmt_mp->width, min_w, 1920, 4, > + &pix_fmt_mp->height, min_h, 1088, 4, 0); > } else { > mfc_err("invalid buf type\n"); > return -EINVAL; > -- > 1.7.9.5 Best wishes, -- Kamil Debski Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
