| Message ID | 20230705142142.3526-1-machel@vivo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] media:Fix repeated initialization | expand |
On 05/07/2023 16:21, Wang Ming wrote: > If the first report returns NULL, > there is no need to execute the > fimc_capture_try_format() function > again. Few nitpicks only, as I don't know the code unfortunately: Please wrap commit message according to Linux coding style / submission process (neither too early nor over the limit): https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597 Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. > > Signed-off-by: Wang Ming <machel@vivo.com> > --- > .../media/platform/samsung/exynos4-is/fimc-capture.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c > index a0d43bf892e6..5ce2f04afa1e 100644 > --- a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c > +++ b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c > @@ -825,10 +825,12 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx, > tfmt->height = mf->height; > ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, > NULL, &fcc, FIMC_SD_PAD_SINK_CAM); > - ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, > - NULL, &fcc, FIMC_SD_PAD_SOURCE); > - if (ffmt && ffmt->mbus_code) > - mf->code = ffmt->mbus_code; > + if (ffmt) { > + ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, > + NULL, &fcc, FIMC_SD_PAD_SOURCE); This does not look aligned. Best regards, Krzysztof
Hi Krzysztof Kozlowski. Look closely, there seems to be a misalignment problem here. In addition, what modifications do you need me to make, I will submit them repeatedly according to your requirements. : ) -----邮件原件----- 发件人: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 发送时间: 2023年7月5日 23:10 收件人: 王明-软件底层技术部 <machel@vivo.com>; Sylwester Nawrocki <s.nawrocki@samsung.com>; Mauro Carvalho Chehab <mchehab@kernel.org>; Alim Akhtar <alim.akhtar@samsung.com>; linux-media@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org 抄送: opensource.kernel <opensource.kernel@vivo.com> 主题: Re: [PATCH v1] media:Fix repeated initialization On 05/07/2023 16:21, Wang Ming wrote: > If the first report returns NULL, > there is no need to execute the > fimc_capture_try_format() function > again. Few nitpicks only, as I don't know the code unfortunately: Please wrap commit message according to Linux coding style / submission process (neither too early nor over the limit): https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597 Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. > > Signed-off-by: Wang Ming <machel@vivo.com> > --- > .../media/platform/samsung/exynos4-is/fimc-capture.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c > b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c > index a0d43bf892e6..5ce2f04afa1e 100644 > --- a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c > +++ b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c > @@ -825,10 +825,12 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx, > tfmt->height = mf->height; > ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, > NULL, &fcc, FIMC_SD_PAD_SINK_CAM); > - ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, > - NULL, &fcc, FIMC_SD_PAD_SOURCE); > - if (ffmt && ffmt->mbus_code) > - mf->code = ffmt->mbus_code; > + if (ffmt) { > + ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, > + NULL, &fcc, FIMC_SD_PAD_SOURCE); This does not look aligned. Best regards, Krzysztof
On 05/07/2023 16:21, Wang Ming wrote: > If the first report returns NULL, > there is no need to execute the > fimc_capture_try_format() function > again. > > Signed-off-by: Wang Ming <machel@vivo.com> > --- > .../media/platform/samsung/exynos4-is/fimc-capture.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c > index a0d43bf892e6..5ce2f04afa1e 100644 > --- a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c > +++ b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c > @@ -825,10 +825,12 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx, > tfmt->height = mf->height; > ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, > NULL, &fcc, FIMC_SD_PAD_SINK_CAM); > - ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, > - NULL, &fcc, FIMC_SD_PAD_SOURCE); > - if (ffmt && ffmt->mbus_code) > - mf->code = ffmt->mbus_code; > + if (ffmt) { > + ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, > + NULL, &fcc, FIMC_SD_PAD_SOURCE); > + if (ffmt && ffmt->mbus_code) > + mf->code = ffmt->mbus_code; > + } I'm not so sure this is a bug at all. I think (not quite certain) that the first fimc_capture_try_format() call constrains the width and height to whatever is the SINK_CAM pad, then further constrains it to that of the SOURCE pad. But really, someone from Samsung should take a look as well. Adding Marek and Sylwester. Regards, Hans > if (mf->width != tfmt->width || mf->height != tfmt->height) > continue; > tfmt->code = mf->code;
diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c index a0d43bf892e6..5ce2f04afa1e 100644 --- a/drivers/media/platform/samsung/exynos4-is/fimc-capture.c +++ b/drivers/media/platform/samsung/exynos4-is/fimc-capture.c @@ -825,10 +825,12 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx, tfmt->height = mf->height; ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, NULL, &fcc, FIMC_SD_PAD_SINK_CAM); - ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, - NULL, &fcc, FIMC_SD_PAD_SOURCE); - if (ffmt && ffmt->mbus_code) - mf->code = ffmt->mbus_code; + if (ffmt) { + ffmt = fimc_capture_try_format(ctx, &tfmt->width, &tfmt->height, + NULL, &fcc, FIMC_SD_PAD_SOURCE); + if (ffmt && ffmt->mbus_code) + mf->code = ffmt->mbus_code; + } if (mf->width != tfmt->width || mf->height != tfmt->height) continue; tfmt->code = mf->code;
If the first report returns NULL, there is no need to execute the fimc_capture_try_format() function again. Signed-off-by: Wang Ming <machel@vivo.com> --- .../media/platform/samsung/exynos4-is/fimc-capture.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)