Hi Dan,
On Wed, Jul 31, 2024 at 04:12:41PM +0100, Daniel Scally wrote:
> On 30/07/2024 23:16, Laurent Pinchart wrote:
> > On Tue, Jul 09, 2024 at 02:29:05PM +0100, Daniel Scally wrote:
> >> Add a new code file to the mali-c55 driver that registers an output
> >> video node for userspace to queue buffers of parameters to. Handlers
> >> are included to program the statistics generation plus the white
> >> balance, black level correction and mesh shading correction blocks.
> >>
> >> Update the rest of the driver to register and link the new video node
> >>
> >> Acked-by: Nayden Kanchev <nayden.kanchev@xxxxxxx>
> >> Co-developed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> >> Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> >> Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
> >> ---
> >> Changes in v6:
> >>
> >> - Used a union to generalise the block pointer rather than resorting to
> >> casting everywhere - fantastic idea Sakari, this made it much cleaner.
> >> - Reworked the loop in mali_c55_params_write_config() so that we can be
> >> sure there's remaining space for the next block header.
> >>
> >> Changes in v5:
> >>
> >> - New patch
> >>
> >> drivers/media/platform/arm/mali-c55/Makefile | 1 +
> >> .../platform/arm/mali-c55/mali-c55-common.h | 20 +
> >> .../platform/arm/mali-c55/mali-c55-core.c | 23 +
> >> .../platform/arm/mali-c55/mali-c55-isp.c | 21 +-
> >> .../platform/arm/mali-c55/mali-c55-params.c | 671 ++++++++++++++++++
> >> .../arm/mali-c55/mali-c55-registers.h | 128 ++++
> >> 6 files changed, 863 insertions(+), 1 deletion(-)
> >> create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-params.c
[snip]
> >> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-core.c b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> >> index ed0db34767a4..55b3cbf53791 100644
> >> --- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> >> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> >> @@ -384,6 +384,16 @@ static int mali_c55_create_links(struct mali_c55 *mali_c55)
> >> goto err_remove_links;
> >> }
> >>
> >> + ret = media_create_pad_link(&mali_c55->params.vdev.entity, 0,
> >> + &mali_c55->isp.sd.entity,
> >> + MALI_C55_ISP_PAD_SINK_PARAMS,
> >> + MEDIA_LNK_FL_ENABLED);
> > Should this be immutable, or do you think it makes sense to support
> > operating the ISP without parameters ? I know we did so when developing
> > the driver to test the initial code, but are there real use cases now ?
>
> If all you're interested in is RAW data bypassed through the ISP then
> you could skip the parameters and statistics - which would be the case
> for RGB/YUV cameras feeding the ISP too. Is that sufficient use-case
> to leave them mutable?
I suppose that's a good point, yes. Fine with me.
> >> + if (ret) {
> >> + dev_err(mali_c55->dev,
> >> + "failed to link ISP and parameters video node\n");
> >> + goto err_remove_links;
> >> + }
> >> +
> >> return 0;
> >>
> >> err_remove_links:
[snip]
--
Regards,
Laurent Pinchart
