| Message ID | BN6PR04MB0660E680A4F69E3037D87E70A3AE0@BN6PR04MB0660.namprd04.prod.outlook.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: exynos4-is: Improve support for s5pv210 and parallel ports | expand |
Hi Jonathan, On Sat, Apr 25, 2020 at 07:26:43PM -0700, Jonathan Bakker wrote: > Commit bae4500399c4 ("[media] exynos4-is: Add missing entity function > initialization") tried to suppress the warnings such as > > s5p-fimc-md camera: Entity type for entity FIMC.0 was not initialized! > > However, this didn't work in all cases. Correct this by calling the set > function earlier. > > Fixes: bae4500399c4 ("exynos4-is: Add missing entity function initialization") > Signed-off-by: Jonathan Bakker <xc-racer2@live.ca> > --- > drivers/media/platform/exynos4-is/fimc-capture.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Thank you for the patch. Please see my comments inline. > diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c > index 705f182330ca..86c233e2f2c9 100644 > --- a/drivers/media/platform/exynos4-is/fimc-capture.c > +++ b/drivers/media/platform/exynos4-is/fimc-capture.c > @@ -1799,7 +1799,6 @@ static int fimc_register_capture_device(struct fimc_dev *fimc, > vid_cap->wb_fmt.code = fmt->mbus_code; > > vid_cap->vd_pad.flags = MEDIA_PAD_FL_SINK; > - vfd->entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER; Isn't vfd->entity above a different entity than sd->entity below? If so, this line must stay. > ret = media_entity_pads_init(&vfd->entity, 1, &vid_cap->vd_pad); > if (ret) > goto err_free_ctx; > @@ -1898,6 +1897,7 @@ int fimc_initialize_capture_subdev(struct fimc_dev *fimc) > return ret; > > sd->entity.ops = &fimc_sd_media_ops; > + sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER; My understanding is that this is the capture subdev and not the scaler. Looking at the other drivers, MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER could be the right function to use here. Best regards, Tomasz
Hi, On 07.07.2020 20:09, Tomasz Figa wrote: > On Sat, Apr 25, 2020 at 07:26:43PM -0700, Jonathan Bakker wrote: >> Commit bae4500399c4 ("[media] exynos4-is: Add missing entity function >> initialization") tried to suppress the warnings such as >> >> s5p-fimc-md camera: Entity type for entity FIMC.0 was not initialized! >> >> However, this didn't work in all cases. Correct this by calling the set >> function earlier. >> >> Fixes: bae4500399c4 ("exynos4-is: Add missing entity function initialization") >> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca> >> --- >> drivers/media/platform/exynos4-is/fimc-capture.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > Thank you for the patch. Please see my comments inline. >> diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c >> index 705f182330ca..86c233e2f2c9 100644 >> --- a/drivers/media/platform/exynos4-is/fimc-capture.c >> +++ b/drivers/media/platform/exynos4-is/fimc-capture.c >> @@ -1799,7 +1799,6 @@ static int fimc_register_capture_device(struct fimc_dev *fimc, >> vid_cap->wb_fmt.code = fmt->mbus_code; >> >> vid_cap->vd_pad.flags = MEDIA_PAD_FL_SINK; >> - vfd->entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER; I think we should leave above line as is, or perhaps change the function to MEDIA_ENT_F_PROC_VIDEO_COMPOSER and... > Isn't vfd->entity above a different entity than sd->entity below? If so, > this line must stay. > >> ret = media_entity_pads_init(&vfd->entity, 1, &vid_cap->vd_pad); >> if (ret) >> goto err_free_ctx; >> @@ -1898,6 +1897,7 @@ int fimc_initialize_capture_subdev(struct fimc_dev *fimc) >> return ret; >> >> sd->entity.ops = &fimc_sd_media_ops; >> + sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER; ...also add an assignment like this. vfd->entity and sd->entity are different entities, vfd->entity corresponds to the capture video node and sd->entity is the capture subdevice media entity. > My understanding is that this is the capture subdev and not the scaler. > Looking at the other drivers, MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER > could be the right function to use here. Scaling can also be configured on that subdev, actually both functions would be valid.
Hi Sylwester and Tomasz, On 2020-07-08 8:34 a.m., Sylwester Nawrocki wrote: > Hi, > > On 07.07.2020 20:09, Tomasz Figa wrote: >> On Sat, Apr 25, 2020 at 07:26:43PM -0700, Jonathan Bakker wrote: >>> Commit bae4500399c4 ("[media] exynos4-is: Add missing entity function >>> initialization") tried to suppress the warnings such as >>> >>> s5p-fimc-md camera: Entity type for entity FIMC.0 was not initialized! >>> >>> However, this didn't work in all cases. Correct this by calling the set >>> function earlier. >>> >>> Fixes: bae4500399c4 ("exynos4-is: Add missing entity function initialization") >>> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca> >>> --- >>> drivers/media/platform/exynos4-is/fimc-capture.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >> Thank you for the patch. Please see my comments inline. > >>> diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c >>> index 705f182330ca..86c233e2f2c9 100644 >>> --- a/drivers/media/platform/exynos4-is/fimc-capture.c >>> +++ b/drivers/media/platform/exynos4-is/fimc-capture.c >>> @@ -1799,7 +1799,6 @@ static int fimc_register_capture_device(struct fimc_dev *fimc, >>> vid_cap->wb_fmt.code = fmt->mbus_code; >>> >>> vid_cap->vd_pad.flags = MEDIA_PAD_FL_SINK; >>> - vfd->entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER; > > I think we should leave above line as is, or perhaps change the function > to MEDIA_ENT_F_PROC_VIDEO_COMPOSER and... > >> Isn't vfd->entity above a different entity than sd->entity below? If so, >> this line must stay. >> >>> ret = media_entity_pads_init(&vfd->entity, 1, &vid_cap->vd_pad); >>> if (ret) >>> goto err_free_ctx; >>> @@ -1898,6 +1897,7 @@ int fimc_initialize_capture_subdev(struct fimc_dev *fimc) >>> return ret; >>> >>> sd->entity.ops = &fimc_sd_media_ops; >>> + sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER; > > ...also add an assignment like this. > > vfd->entity and sd->entity are different entities, vfd->entity corresponds to > the capture video node and sd->entity is the capture subdevice media entity. > Whoops, I totally misinterpreted the fact that they are different entities. For v2 I'll remove the removal from vfd and just add MEDIA_ENT_F_PROC_VIDEO_SCALER to the sd entity. >> My understanding is that this is the capture subdev and not the scaler. >> Looking at the other drivers, MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER >> could be the right function to use here. > > Scaling can also be configured on that subdev, actually both functions would > be valid. > Thanks, Jonathan
diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c index 705f182330ca..86c233e2f2c9 100644 --- a/drivers/media/platform/exynos4-is/fimc-capture.c +++ b/drivers/media/platform/exynos4-is/fimc-capture.c @@ -1799,7 +1799,6 @@ static int fimc_register_capture_device(struct fimc_dev *fimc, vid_cap->wb_fmt.code = fmt->mbus_code; vid_cap->vd_pad.flags = MEDIA_PAD_FL_SINK; - vfd->entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER; ret = media_entity_pads_init(&vfd->entity, 1, &vid_cap->vd_pad); if (ret) goto err_free_ctx; @@ -1898,6 +1897,7 @@ int fimc_initialize_capture_subdev(struct fimc_dev *fimc) return ret; sd->entity.ops = &fimc_sd_media_ops; + sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER; sd->internal_ops = &fimc_capture_sd_internal_ops; v4l2_set_subdevdata(sd, fimc); return 0;
Commit bae4500399c4 ("[media] exynos4-is: Add missing entity function initialization") tried to suppress the warnings such as s5p-fimc-md camera: Entity type for entity FIMC.0 was not initialized! However, this didn't work in all cases. Correct this by calling the set function earlier. Fixes: bae4500399c4 ("exynos4-is: Add missing entity function initialization") Signed-off-by: Jonathan Bakker <xc-racer2@live.ca> --- drivers/media/platform/exynos4-is/fimc-capture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)