* Re: [PATCH v2] staging: media: atomisp: fix memory leak of dvs2_coeff
From: Andy Shevchenko @ 2026-04-16 18:32 UTC (permalink / raw)
To: Huihui Huang
Cc: Hans de Goede, Mauro Carvalho Chehab, Andy Shevchenko,
Sakari Ailus, Greg Kroah-Hartman, linux-media, linux-staging,
linux-kernel
In-Reply-To: <20260416131626.2544105-1-hhhuang@smu.edu.sg>
On Thu, Apr 16, 2026 at 09:16:26PM +0800, Huihui Huang wrote:
> There is a memory leak in
> drivers/staging/media/atomisp/pci/atomisp_compat_css20.c.
>
> In atomisp_alloc_dis_coef_buf(), dvs2_coeff is allocated by
> ia_css_dvs2_coefficients_allocate() and stored in
> asd->params.css_param.dvs2_coeff. If the subsequent
> ia_css_dvs2_statistics_allocate() for dvs_stat fails, the function
> returns -ENOMEM without freeing the previously allocated dvs2_coeff.
> Add the missing ia_css_dvs2_coefficients_free() call and set the
> pointer to NULL before returning on the error path.
Why do we need this?
...
> asd->params.dvs_stat = ia_css_dvs2_statistics_allocate(dvs_grid);
> - if (!asd->params.dvs_stat)
> + if (!asd->params.dvs_stat) {
> + ia_css_dvs2_coefficients_free(asd->params.css_param.dvs2_coeff);
> + asd->params.css_param.dvs2_coeff = NULL;
Actually same Q here: Do we need this NULLification?
> return -ENOMEM;
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] media: dvbdev: fix missing refcount update in dvb_generic_open()
From: Heitor Alves de Siqueira @ 2026-04-16 19:57 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Lin Ma
Cc: linux-media, linux-kernel, kernel-dev,
syzbot+ae466a728017ec940b41, stable
After introducing a reference counter to struct dvb_device, it's
possible for a dvbdev to be prematurely freed by dvb_free_device(). This
is due to a missing kref_get() in the dvb_generic_open() path, that was
not balanced with the existing kref_put() in dvb_generic_release().
Add dvb_device_get() to correctly increment the reference counter at the
end of dvb_generic_open(). This also avoids incorrectly increasing the
counter in case of EBUSY errors.
Fixes: 0fc044b2b5e2 ("media: dvbdev: adopts refcnt to avoid UAF")
Reported-by: syzbot+ae466a728017ec940b41@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ae466a728017ec940b41
Tested-by: syzbot+ae466a728017ec940b41@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Heitor Alves de Siqueira <halves@igalia.com>
---
drivers/media/dvb-core/dvbdev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index d753d329502a..84575610253b 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -147,6 +147,7 @@ int dvb_generic_open(struct inode *inode, struct file *file)
dvbdev->writers--;
}
+ dvb_device_get(dvbdev);
dvbdev->users--;
return 0;
}
---
base-commit: 1d51b370a0f8f642f4fc84c795fbedac0fcdbbd2
change-id: 20260416-dvbdev-refcount-589bc77e42ef
Best regards,
--
Heitor Alves de Siqueira <halves@igalia.com>
^ permalink raw reply related
* Re: [PATCH] media: atomisp: use kmalloc_objs for array allocations
From: Pedro Pontes @ 2026-04-16 20:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: hansg, gregkh, mchehab, andy, sakari.ailus, kees, linux-media,
linux-staging, linux-kernel
In-Reply-To: <CAHp75VeyCRCdR4VD8+KM33zYv3OMBSRGaeSE5B2Wc5hn9Q9hEA@mail.gmail.com>
> There is already a patch doing it in a slightly better way. Have you
> followed the mailing list?
I searched lore and patchwork but couldn’t find the patch you’re referring to.
Could you please point me to it?
Are you referring to the patch I linked in my submission, or a different one?
> Please, better to help with this driver is to subscribe to the mailing
> list and review already
> submitted ones.
Understood, I’ll follow the list more closely and review related patches.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 07/13] media: renesas: vsp1: brx: Fix format propagation
From: Laurent Pinchart @ 2026-04-16 21:11 UTC (permalink / raw)
To: Lad, Prabhakar; +Cc: linux-media, linux-renesas-soc, Jacopo Mondi
In-Reply-To: <CA+V-a8t481xuwava0nb7uY9CUPqFWZ_8EP0xrK3BgumP7HDcLg@mail.gmail.com>
Hi Prabhakar,
On Thu, Apr 16, 2026 at 06:49:14PM +0100, Lad, Prabhakar wrote:
> On Wed, Mar 18, 2026 at 11:59 PM Laurent Pinchart wrote:
> >
> > The format width and height is never propagated to the BRX source pad,
> > leaving its initial configuration invalid. Propagate the whole format
> > from the first sink pad to the source pad instead of only propagating
> > the media bus code. This fixes compliance with the subdev format
> > propagation rules.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> > Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> > drivers/media/platform/renesas/vsp1/vsp1_brx.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
> > index dd651cef93e4..911359faa600 100644
> > --- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
> > +++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
> > @@ -156,14 +156,20 @@ static int brx_set_format(struct v4l2_subdev *subdev,
> > compose->height = format->height;
> > }
> >
> > - /* Propagate the format code to all pads. */
> > + /*
> > + * Propagate the format code to all pads, and the whole format to the
> > + * source pad.
> > + */
> > if (fmt->pad == BRX_PAD_SINK(0)) {
> > unsigned int i;
> >
> > - for (i = 0; i <= brx->entity.source_pad; ++i) {
> > + for (i = 0; i < brx->entity.source_pad; ++i) {
> > format = v4l2_subdev_state_get_format(state, i);
> > format->code = fmt->format.code;
> > }
> > +
> > + format = v4l2_subdev_state_get_format(state, i);
> > + *format = fmt->format;
>
> When running kms-test-plane-position.py (from [0]) on RZ/V2H EVK, Im
> getting vblank timeouts as seen below:
Oops :-/
I'm run the KMS tests on a R-Car board when I submitted the series. I'll
test again tomorrow.
> [ 51.295849] ------------[ cut here ]------------
> [ 51.300538] [CRTC:45:crtc-0] vblank wait timed out
> [ 51.305514] WARNING: drivers/gpu/drm/drm_atomic_helper.c:1921 at drm_atomic_helper_wait_for_vblanks.part.0+0x248/0x27c [drm_kms_helper], CPU#1: python3/413
> [ 51.319577] Modules linked in: sha256 cfg80211 bluetooth ecdh_generic kpp ecc rfkill snd_soc_hdmi_codec snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer snd soundcore rzg2l_du_drm spi_rpc_if drm_client_lib vsp1 rzg2l_cru videobuf2_vmalloc drm_dma_helper videobuf2_dma_contig videobuf2_memops rcar_fcp rzg2l_csi2 videobuf2_v4l2 renesas_usbhs rzg2l_mipi_dsi ov5645 videobuf2_common adv7511 v4l2_cci phy_rzg3e_usb3 panfrost v4l2_fwnode reset_rzv2h_usb2phy v4l2_async drm_display_helper drm_shmem_helper videodev rtc_isl1208 cec gpu_sched rtc_renesas_rtca3 mc display_connector drm_kms_helper renesas_rpc_if drm fuse backlight
> [ 51.374382] CPU: 1 UID: 0 PID: 413 Comm: python3 Not tainted 7.0.0-next-20260415-00258-gf9ef0131676a-dirty #340 PREEMPT
> [ 51.385280] Hardware name: Renesas RZ/V2H EVK Board based on r9a09g057h44 (DT)
> [ 51.392521] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 51.399505] pc : drm_atomic_helper_wait_for_vblanks.part.0+0x248/0x27c [drm_kms_helper]
> [ 51.407624] lr : drm_atomic_helper_wait_for_vblanks.part.0+0x248/0x27c [drm_kms_helper]
> [ 51.415739] sp : ffff800083dbb9d0
> [ 51.419067] x29: ffff800083dbba00 x28: 000000000000000a x27: 00000000000005c5
> [ 51.426237] x26: 0000000000000000 x25: ffff0000ca4c0888 x24: 0000000000000001
> [ 51.433406] x23: 0000000000000001 x22: 0000000000000000 x21: 0000000000000000
> [ 51.440575] x20: ffff0000c7cb4980 x19: ffff0000c325e618 x18: 000000000000000a
> [ 51.447743] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [ 51.454911] x14: 0000000000000000 x13: ffff8000818c3ca0 x12: 00000000000001fc
> [ 51.462079] x11: ffff0000c0fcd360 x10: ffff8000832bd200 x9 : ffff8000818c3ca0
> [ 51.469248] x8 : 3fffffffffffefff x7 : ffff80008191bca0 x6 : 0000000000000000
> [ 51.476416] x5 : ffff0003fdf93088 x4 : 0000000000000001 x3 : 0000000000000000
> [ 51.483584] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000cc19af40
> [ 51.490753] Call trace:
> [ 51.493212] drm_atomic_helper_wait_for_vblanks.part.0+0x248/0x27c [drm_kms_helper] (P)
> [ 51.501336] drm_atomic_helper_commit_tail_rpm+0xbc/0xd8 [drm_kms_helper]
> [ 51.508237] commit_tail+0xa4/0x1a4 [drm_kms_helper]
> [ 51.513313] drm_atomic_helper_commit+0x178/0x194 [drm_kms_helper]
> [ 51.519605] drm_atomic_commit+0x8c/0xd0 [drm]
> [ 51.524307] drm_mode_atomic_ioctl+0xac8/0xe00 [drm]
> [ 51.529523] drm_ioctl_kernel+0xc0/0x128 [drm]
> [ 51.534217] drm_ioctl+0x354/0x4c0 [drm]
> [ 51.538389] __arm64_sys_ioctl+0xa4/0xf4
> [ 51.542342] invoke_syscall.constprop.0+0x40/0x108
> [ 51.547161] el0_svc_common.constprop.0+0xb8/0xd8
> [ 51.551892] do_el0_svc+0x1c/0x28
> [ 51.555229] el0_svc+0x38/0x140
> [ 51.558397] el0t_64_sync_handler+0xa0/0xe4
> [ 51.562603] el0t_64_sync+0x198/0x19c
> [ 51.566286] ---[ end trace 0000000000000000 ]---
> [ 64.735886] rzg2l-du 16460000.display: [drm] *ERROR* flip_done timed out
> [ 64.742630] rzg2l-du 16460000.display: [drm] *ERROR* [CRTC:45:crtc-0] commit wait timed out
> [ 74.975884] rzg2l-du 16460000.display: [drm] *ERROR* flip_done timed out
> [ 74.982639] rzg2l-du 16460000.display: [drm] *ERROR* [PLANE:40:plane-1] commit wait timed out
> [ 75.999845] ------------[ cut here ]------------
>
> Reverting this patch fixes the issue, but compliance might fail. On
> V2H media device is not registered for VSP.
>
> [0] https://git.ideasonboard.com/renesas/kms-tests/src/branch/master/tests
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 1/2] media: uvcvideo: Do not open code uvc_queue_get_current_buffer
From: Laurent Pinchart @ 2026-04-16 21:22 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Hans de Goede, Mauro Carvalho Chehab, Guennadi Liakhovetski,
linux-media, linux-kernel
In-Reply-To: <20260415-uvc-meta-partial-v1-1-a0acc79a6300@chromium.org>
Hi Ricardo,
Thank you for the patch.
On Wed, Apr 15, 2026 at 03:59:57PM +0000, Ricardo Ribalda wrote:
> Do not re-implement uvc_queue_get_current_buffer() logic inside
> uvc_video_complete(), just call the function.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/usb/uvc/uvc_video.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 40c76c051da2..4feb3699f520 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1693,7 +1693,6 @@ static void uvc_video_complete(struct urb *urb)
> struct vb2_queue *vb2_qmeta = stream->meta.queue.vdev.queue;
> struct uvc_buffer *buf = NULL;
> struct uvc_buffer *buf_meta = NULL;
> - unsigned long flags;
> int ret;
>
> switch (urb->status) {
> @@ -1719,13 +1718,8 @@ static void uvc_video_complete(struct urb *urb)
>
> buf = uvc_queue_get_current_buffer(queue);
>
> - if (vb2_qmeta) {
> - spin_lock_irqsave(&qmeta->irqlock, flags);
> - if (!list_empty(&qmeta->irqqueue))
> - buf_meta = list_first_entry(&qmeta->irqqueue,
> - struct uvc_buffer, queue);
> - spin_unlock_irqrestore(&qmeta->irqlock, flags);
> - }
> + if (vb2_qmeta)
> + buf_meta = uvc_queue_get_current_buffer(qmeta);
>
> /* Re-initialise the URB async work. */
> uvc_urb->async_operations = 0;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 2/2] media: uvcvideo: Avoid partial metadata buffers
From: Laurent Pinchart @ 2026-04-16 21:35 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Hans de Goede, Mauro Carvalho Chehab, Guennadi Liakhovetski,
linux-media, linux-kernel
In-Reply-To: <20260415-uvc-meta-partial-v1-2-a0acc79a6300@chromium.org>
Hi Ricardo,
Thank you for the patch.
On Wed, Apr 15, 2026 at 03:59:58PM +0000, Ricardo Ribalda wrote:
> If the metadata queue that is empty receives a new buffer while we are
> in the middle of processing a frame, the first metadata buffer will
> contain partial information.
>
> Avoid this by tracking the state of the metadata buffer and making sure
> that it is in sync with the data buffer.
>
> Now that we are at it, simplify the code a bit by not getting a metadata
> buffer if there is no data buffer ready.
>
> Fixes: 088ead255245 ("media: uvcvideo: Add a metadata device node")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_video.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 4feb3699f520..f339d6d19a39 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1517,6 +1517,8 @@ static void uvc_video_next_buffers(struct uvc_streaming *stream,
> *meta_buf);
> }
> *video_buf = uvc_queue_next_buffer(&stream->queue, *video_buf);
> + if (*video_buf && *meta_buf)
> + (*meta_buf)->state = UVC_BUF_STATE_ACTIVE;
> }
>
> static void uvc_video_decode_isoc(struct uvc_urb *uvc_urb,
> @@ -1718,9 +1720,23 @@ static void uvc_video_complete(struct urb *urb)
>
> buf = uvc_queue_get_current_buffer(queue);
>
> - if (vb2_qmeta)
> + if (buf && vb2_qmeta)
> buf_meta = uvc_queue_get_current_buffer(qmeta);
>
> + /*
> + * Avoid partial metadata buffers, by making sure that the data buffer
> + * and metadata buffer state is in sync.
> + *
> + * A new (QUEUED) buffer is only allowed to become ACTIVE if we are also
> + * at the start of a new data buffer.
> + */
> + if (buf_meta && buf_meta->state == UVC_BUF_STATE_QUEUED) {
> + if (buf->state != UVC_BUF_STATE_QUEUED)
> + buf_meta = NULL;
> + else
> + buf_meta->state = UVC_BUF_STATE_ACTIVE;
> + }
> +
This creates a different logic for video buffers and metadata buffers.
Wouldn't it be better to use the same logic, and transition the metadata
buffer to the ACTIVE state when we detect the beginning of a new frame ?
> /* Re-initialise the URB async work. */
> uvc_urb->async_operations = 0;
>
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v1 1/1] dt-bindings: media: mt9m114: document common video device properties
From: Laurent Pinchart @ 2026-04-16 21:40 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-media, devicetree, linux-kernel
In-Reply-To: <20260406081330.30362-2-clamor95@gmail.com>
On Mon, Apr 06, 2026 at 11:13:30AM +0300, Svyatoslav Ryhel wrote:
> Document common video interface device properties, such as rotation and
> orientation.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> .../devicetree/bindings/media/i2c/onnn,mt9m114.yaml | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml b/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
> index e896f4db2421..2b39614f5cbf 100644
> --- a/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/onnn,mt9m114.yaml
> @@ -15,6 +15,9 @@ description: |-
> an I2C interface and outputs image data over a 8-bit parallel or 1-lane MIPI
> CSI-2 connection.
>
> +allOf:
> + - $ref: /schemas/media/video-interface-devices.yaml#
> +
> properties:
> compatible:
> enum:
> @@ -90,7 +93,7 @@ required:
> - vaa-supply
> - port
>
> -additionalProperties: false
> +unevaluatedProperties: false
>
> examples:
> - |
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 0/2] media: uvcvideo: Avoid partial metadata buffers
From: Ricardo Ribalda @ 2026-04-17 5:19 UTC (permalink / raw)
To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Guennadi Liakhovetski
Cc: linux-media, linux-kernel, Ricardo Ribalda
The current code can lead to partial metadata buffers when the metadata
queue transitions from empty to ready. Fix that.
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Changes in v2: (Thanks Laurent)
- Transition to UVC_BUF_STATE_ACTIVE with the data buffer
- Link to v1: https://lore.kernel.org/r/20260415-uvc-meta-partial-v1-0-a0acc79a6300@chromium.org
---
Ricardo Ribalda (2):
media: uvcvideo: Do not open code uvc_queue_get_current_buffer
media: uvcvideo: Avoid partial metadata buffers
drivers/media/usb/uvc/uvc_video.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
---
base-commit: 2e9a8a967f836cf879f35c7434025de265826cc1
change-id: 20260415-uvc-meta-partial-a5767866d0e0
Best regards,
--
Ricardo Ribalda <ribalda@chromium.org>
^ permalink raw reply
* [PATCH v2 1/2] media: uvcvideo: Do not open code uvc_queue_get_current_buffer
From: Ricardo Ribalda @ 2026-04-17 5:19 UTC (permalink / raw)
To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Guennadi Liakhovetski
Cc: linux-media, linux-kernel, Ricardo Ribalda
In-Reply-To: <20260417-uvc-meta-partial-v2-0-31d274af7d2d@chromium.org>
Do not re-implement uvc_queue_get_current_buffer() logic inside
uvc_video_complete(), just call the function.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/uvc/uvc_video.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 271c246a02ea..5d45c74c6041 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1704,7 +1704,6 @@ static void uvc_video_complete(struct urb *urb)
struct vb2_queue *vb2_qmeta = stream->meta.queue.vdev.queue;
struct uvc_buffer *buf = NULL;
struct uvc_buffer *buf_meta = NULL;
- unsigned long flags;
int ret;
switch (urb->status) {
@@ -1730,13 +1729,8 @@ static void uvc_video_complete(struct urb *urb)
buf = uvc_queue_get_current_buffer(queue);
- if (vb2_qmeta) {
- spin_lock_irqsave(&qmeta->irqlock, flags);
- if (!list_empty(&qmeta->irqqueue))
- buf_meta = list_first_entry(&qmeta->irqqueue,
- struct uvc_buffer, queue);
- spin_unlock_irqrestore(&qmeta->irqlock, flags);
- }
+ if (vb2_qmeta)
+ buf_meta = uvc_queue_get_current_buffer(qmeta);
/* Re-initialise the URB async work. */
uvc_urb->async_operations = 0;
--
2.54.0.rc1.513.gad8abe7a5a-goog
^ permalink raw reply related
* [PATCH v2 2/2] media: uvcvideo: Avoid partial metadata buffers
From: Ricardo Ribalda @ 2026-04-17 5:19 UTC (permalink / raw)
To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Guennadi Liakhovetski
Cc: linux-media, linux-kernel, Ricardo Ribalda
In-Reply-To: <20260417-uvc-meta-partial-v2-0-31d274af7d2d@chromium.org>
If the metadata queue that is empty receives a new buffer while we are
in the middle of processing a frame, the first metadata buffer will
contain partial information.
Avoid this by tracking the state of the metadata buffer and making sure
that it is in sync with the data buffer.
Now that we are at it, make sure that we skip buffers of size 1 or 0.
They are not allowed by the spec... but it is a simple check to add and
better be safe than sorry.
Fixes: 088ead255245 ("media: uvcvideo: Add a metadata device node")
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/uvc/uvc_video.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 5d45c74c6041..cd77ca6f8136 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1149,7 +1149,9 @@ static void uvc_video_stats_stop(struct uvc_streaming *stream)
* uvc_video_decode_end will never be called with a NULL buffer.
*/
static int uvc_video_decode_start(struct uvc_streaming *stream,
- struct uvc_buffer *buf, const u8 *data, int len)
+ struct uvc_buffer *buf,
+ struct uvc_buffer *meta_buf,
+ const u8 *data, int len)
{
u8 header_len;
u8 fid;
@@ -1278,6 +1280,8 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
/* TODO: Handle PTS and SCR. */
buf->state = UVC_BUF_STATE_ACTIVE;
+ if (meta_buf)
+ meta_buf->state = UVC_BUF_STATE_ACTIVE;
}
stream->last_fid = fid;
@@ -1435,7 +1439,7 @@ static void uvc_video_decode_meta(struct uvc_streaming *stream,
ktime_t time;
const u8 *scr;
- if (!meta_buf || length == 2)
+ if (length <= 2 || !meta_buf || meta_buf->state != UVC_BUF_STATE_ACTIVE)
return;
has_pts = mem[1] & UVC_STREAM_PTS;
@@ -1552,7 +1556,7 @@ static void uvc_video_decode_isoc(struct uvc_urb *uvc_urb,
/* Decode the payload header. */
mem = urb->transfer_buffer + urb->iso_frame_desc[i].offset;
do {
- ret = uvc_video_decode_start(stream, buf, mem,
+ ret = uvc_video_decode_start(stream, buf, meta_buf, mem,
urb->iso_frame_desc[i].actual_length);
if (ret == -EAGAIN)
uvc_video_next_buffers(stream, &buf, &meta_buf);
@@ -1601,7 +1605,8 @@ static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
*/
if (stream->bulk.header_size == 0 && !stream->bulk.skip_payload) {
do {
- ret = uvc_video_decode_start(stream, buf, mem, len);
+ ret = uvc_video_decode_start(stream, buf, meta_buf, mem,
+ len);
if (ret == -EAGAIN)
uvc_video_next_buffers(stream, &buf, &meta_buf);
} while (ret == -EAGAIN);
--
2.54.0.rc1.513.gad8abe7a5a-goog
^ permalink raw reply related
* Re: [PATCH RFC 1/7] media: qcom: iris: add QC10C & P010 buffer size calculations
From: Dikshita Agarwal @ 2026-04-17 6:47 UTC (permalink / raw)
To: Neil Armstrong, Vikash Garodia, Abhinav Kumar,
Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: linux-media, linux-arm-msm, linux-kernel
In-Reply-To: <20260408-topic-sm8x50-iris-10bit-decoding-v1-1-428c1ec2e3f3@linaro.org>
On 4/8/2026 10:13 PM, Neil Armstrong wrote:
> The P010 (YUV format with 16-bits per pixel with interleaved UV)
> and QC10C (P010 compressed mode similar to QC08C) requires specific
> buffer calculations to allocate the right buffer size for DPB frames
> and frames consumed by userspace.
>
> Similar to 8bit, the 10bit DPB frames uses QC10C format.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
> drivers/media/platform/qcom/iris/iris_buffer.c | 81 +++++++++++++++++++++++++-
> 1 file changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> index 9151f43bc6b9..a0e31bff8f26 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> @@ -15,7 +15,9 @@
> #define MAX_WIDTH 4096
> #define MAX_HEIGHT 2304
> #define Y_STRIDE_ALIGN 128
> +#define Y_STRIDE_ALIGN_P010 256
> #define UV_STRIDE_ALIGN 128
> +#define UV_STRIDE_ALIGN_P010 256
> #define Y_SCANLINE_ALIGN 32
> #define UV_SCANLINE_ALIGN 16
> #define UV_SCANLINE_ALIGN_QC08C 32
> @@ -80,6 +82,26 @@ static u32 iris_yuv_buffer_size_nv12(struct iris_inst *inst)
> return ALIGN(y_plane + uv_plane, PIXELS_4K);
> }
>
> +static u32 iris_yuv_buffer_size_p010(struct iris_inst *inst)
> +{
> + u32 y_plane, uv_plane, y_stride, uv_stride, y_scanlines, uv_scanlines;
> + struct v4l2_format *f;
> +
> + if (inst->domain == DECODER)
> + f = inst->fmt_dst;
> + else
> + f = inst->fmt_src;
> +
> + y_stride = ALIGN(f->fmt.pix_mp.width * 2, Y_STRIDE_ALIGN_P010);
> + uv_stride = ALIGN(f->fmt.pix_mp.width * 2, UV_STRIDE_ALIGN_P010);
> + y_scanlines = ALIGN(f->fmt.pix_mp.height, Y_SCANLINE_ALIGN);
> + uv_scanlines = ALIGN((f->fmt.pix_mp.height + 1) >> 1, UV_SCANLINE_ALIGN);
> + y_plane = y_stride * y_scanlines;
> + uv_plane = uv_stride * uv_scanlines;
> +
> + return ALIGN(y_plane + uv_plane, PIXELS_4K);
> +}
> +
> /*
> * QC08C:
> * Compressed Macro-tile format for NV12.
> @@ -204,6 +226,55 @@ static u32 iris_yuv_buffer_size_qc08c(struct iris_inst *inst)
> return ALIGN(y_meta_plane + y_plane + uv_meta_plane + uv_plane, PIXELS_4K);
> }
>
> +/*
> + * QC10C:
> + * Compressed Macro-tile format for TP10.
> + */
> +static u32 iris_yuv_buffer_size_qc10c(struct iris_inst *inst)
> +{
> + u32 y_stride, y_buf_height;
> + u32 uv_stride, uv_buf_height;
> + u32 y_md_stride, y_md_height;
> + u32 uv_md_stride, uv_md_height;
> + u32 y_data_size, uv_data_size;
> + u32 y_md_size, uv_md_size;
> + struct v4l2_format *f = NULL;
> +
> + if (inst->domain == DECODER)
> + f = inst->fmt_dst;
> + else
> + f = inst->fmt_src;
> +
> + y_stride = ALIGN(ALIGN(f->fmt.pix_mp.width, 192) * 4 / 3,
> + Y_STRIDE_ALIGN_P010);
Y_STRIDE_ALIGN_P010 is being used for both P010 and QC10C, lets keep it
Y_STRIDE_ALIGN_10_BIT ? or something similar ?
> + y_buf_height = ALIGN(f->fmt.pix_mp.height, UV_SCANLINE_ALIGN);
why not call it y_scanlines only?
> +
> + y_data_size = ALIGN(y_stride * y_buf_height, PIXELS_4K);
s/y_data_size/y_plane ?
> +
> + uv_stride = ALIGN(ALIGN(f->fmt.pix_mp.width, 192) * 4 / 3,
> + UV_STRIDE_ALIGN_P010);
> + uv_buf_height = ALIGN((f->fmt.pix_mp.height + 1) / 2,
> + UV_SCANLINE_ALIGN);
s/uv_buf_height/uv_scanline?
> +
> + uv_data_size = ALIGN(uv_stride * uv_buf_height, PIXELS_4K);
s/uv_data_size/uv_plane?
Pls keep all these names consistent with other functions, applies to below
variables as well.
Thanks,
Dikshita
> +
> + y_md_stride = ALIGN(DIV_ROUND_UP(f->fmt.pix_mp.width, 48),
> + META_STRIDE_ALIGNED);
> + y_md_height = ALIGN(DIV_ROUND_UP(f->fmt.pix_mp.height, 4),
> + META_SCANLINE_ALIGNED);
> +
> + y_md_size = ALIGN(y_md_stride * y_md_height, PIXELS_4K);
> +
> + uv_md_stride = ALIGN(DIV_ROUND_UP((f->fmt.pix_mp.width + 1) / 2, 24),
> + META_STRIDE_ALIGNED);
> + uv_md_height = ALIGN(DIV_ROUND_UP((f->fmt.pix_mp.height + 1) / 2, 4),
> + META_SCANLINE_ALIGNED);
> +
> + uv_md_size = ALIGN(uv_md_stride * uv_md_height, PIXELS_4K);
> +
> + return y_data_size + uv_data_size + y_md_size + uv_md_size;
> +}
> +
> static u32 iris_dec_bitstream_buffer_size(struct iris_inst *inst)
> {
> struct platform_inst_caps *caps = inst->core->iris_platform_data->inst_caps;
> @@ -268,10 +339,18 @@ int iris_get_buffer_size(struct iris_inst *inst,
> case BUF_OUTPUT:
> if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
> return iris_yuv_buffer_size_qc08c(inst);
> + else if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC10C)
> + return iris_yuv_buffer_size_qc10c(inst);
> + else if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_P010)
> + return iris_yuv_buffer_size_p010(inst);
> else
> return iris_yuv_buffer_size_nv12(inst);
> case BUF_DPB:
> - return iris_yuv_buffer_size_qc08c(inst);
> + if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC10C ||
> + inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_P010)
Please introduce one API, is_10bit_format and use that here instead.
Thanks,
Dikshita
> + return iris_yuv_buffer_size_qc10c(inst);
> + else
> + return iris_yuv_buffer_size_qc08c(inst);
> default:
> return 0;
> }
>
^ permalink raw reply
* [PATCH v2] media: marvell-cam: fix missing pci_disable_device() on remove
From: Guangshuo Li @ 2026-04-17 6:53 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Guangshuo Li, Kees Cook, Hans Verkuil,
Jonathan Corbet, linux-media, linux-kernel
Cc: stable
During manual code audit, we found that cafe_pci_probe() enables the
PCI device with pci_enable_device(), and its probe error path properly
calls pci_disable_device() on failure.
However, cafe_pci_remove() tears down the controller and frees the
driver data without disabling the PCI device, leaving the remove path
inconsistent with probe cleanup.
Add the missing pci_disable_device() call to cafe_pci_remove().
Fixes: abfa3df36c01 ("[media] marvell-cam: Separate out the Marvell camera core")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
- Fix subject prefix to use "media:" as reported by CI
drivers/media/platform/marvell/cafe-driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/marvell/cafe-driver.c b/drivers/media/platform/marvell/cafe-driver.c
index 632c15572aa8..22034df6cba9 100644
--- a/drivers/media/platform/marvell/cafe-driver.c
+++ b/drivers/media/platform/marvell/cafe-driver.c
@@ -609,6 +609,7 @@ static void cafe_pci_remove(struct pci_dev *pdev)
return;
}
cafe_shutdown(cam);
+ pci_disable_device(pdev);
kfree(cam);
}
--
2.43.0
^ permalink raw reply related
* [PATCH v3] staging: media: atomisp: fix memory leak of dvs2_coeff
From: Huihui Huang @ 2026-04-17 7:01 UTC (permalink / raw)
To: Hans de Goede, Mauro Carvalho Chehab, Andy Shevchenko
Cc: Sakari Ailus, Greg Kroah-Hartman, linux-media, linux-staging,
linux-kernel, Huihui Huang
In-Reply-To: <V2_MESSAGE_ID>
There is a memory leak in
drivers/staging/media/atomisp/pci/atomisp_compat_css20.c.
In atomisp_alloc_dis_coef_buf(), dvs2_coeff is allocated by
ia_css_dvs2_coefficients_allocate() and stored in
asd->params.css_param.dvs2_coeff. If the subsequent
ia_css_dvs2_statistics_allocate() for dvs_stat fails, the function
returns -ENOMEM without freeing the previously allocated dvs2_coeff.
Add the missing ia_css_dvs2_coefficients_free() call before returning
on the error path.
Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
---
v3: Remove unnecessary NULL assignment per review feedback.
v2: Reword commit message per review feedback (no code change).
---
drivers/staging/media/atomisp/pci/atomisp_compat_css20.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index be5f37f4a6fd..d6e135c42e2f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -1363,8 +1363,10 @@ int atomisp_alloc_dis_coef_buf(struct atomisp_sub_device *asd)
/* DIS projections. */
asd->params.dis_proj_data_valid = false;
asd->params.dvs_stat = ia_css_dvs2_statistics_allocate(dvs_grid);
- if (!asd->params.dvs_stat)
+ if (!asd->params.dvs_stat) {
+ ia_css_dvs2_coefficients_free(asd->params.css_param.dvs2_coeff);
return -ENOMEM;
+ }
asd->params.dvs_hor_proj_bytes =
dvs_grid->aligned_height * dvs_grid->aligned_width *
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v1 03/11] media: v4l2-isp: Add helper function to compute extended stats size
From: Jacopo Mondi @ 2026-04-17 7:15 UTC (permalink / raw)
To: Antoine Bouyer
Cc: julien.vuillaumier, alexi.birlinger, daniel.baluta, peng.fan,
frank.li, jacopo.mondi, laurent.pinchart, mchehab, robh, krzk+dt,
conor+dt, michael.riesch, anthony.mcgivern, linux-media,
linux-kernel, devicetree, imx, jai.luthra, paul.elder
In-Reply-To: <20260413160331.2611829-4-antoine.bouyer@nxp.com>
Hi Antoine
On Mon, Apr 13, 2026 at 06:03:23PM +0200, Antoine Bouyer wrote:
> v4l2-isp framework only supports extended buffer for generic ISP
> configuration. This patch adds simple helper function to compute the
> extended statistics buffer size, exactly the same as for extended
> parameters, except that it uses the `v4l2_isp_stats_block_header`
> structure definition to prevent conflict with the
> `v4l2_isp_params_block_header` one.
>
> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> ---
> include/media/v4l2-isp.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/media/v4l2-isp.h b/include/media/v4l2-isp.h
> index f3a6d0edcb24..9a93a534e7b0 100644
> --- a/include/media/v4l2-isp.h
> +++ b/include/media/v4l2-isp.h
> @@ -27,6 +27,19 @@ struct vb2_buffer;
> #define v4l2_isp_params_buffer_size(max_params_size) \
> (offsetof(struct v4l2_isp_params_buffer, data) + (max_params_size))
>
> +/**
> + * v4l2_isp_stats_buffer_size - Calculate size of v4l2_isp_stats_buffer
> + * @max_stats_size: The total size of the ISP statistic blocks
> + *
> + * Users of the v4l2 extensible statistics buffers will have differing sized data
> + * arrays depending on their specific ISP blocks. Drivers and userspace will need
> + * to be able to calculate the appropriate size of the struct to accommodate all
> + * ISP statistics blocks provided by the platform.
> + * This macro provides a convenient tool for the calculation.
> + */
> +#define v4l2_isp_stats_buffer_size(max_stats_size) \
> + (offsetof(struct v4l2_isp_stats_buffer, data) + (max_stats_size))
> +
Should we do this or simply:
--- a/include/media/v4l2-isp.h
+++ b/include/media/v4l2-isp.h
@@ -15,17 +15,21 @@ struct device;
struct vb2_buffer;
/**
- * v4l2_isp_params_buffer_size - Calculate size of v4l2_isp_params_buffer
- * @max_params_size: The total size of the ISP configuration blocks
+ * v4l2_isp_buffer_size - Calculate size of v4l2_isp_buffer
+ * @max_size: The total size of the ISP configuration or statistics blocks
+ *
+ * Users of v4l2-isp will have differing sized data arrays for parameters and
+ * statistics, depending on their specific blocks. Drivers need to be able to
+ * calculate the appropriate size of the buffer to accommodate all ISP blocks
+ * supported by the platform. This macro provides a convenient tool for the
+ * calculation.
+ *
+ * The intended users of this function are drivers initializing the size
+ * of their metadata (parameters and statistics) buffers.
*
- * Users of the v4l2 extensible parameters will have differing sized data arrays
- * depending on their specific parameter buffers. Drivers and userspace will
- * need to be able to calculate the appropriate size of the struct to
- * accommodate all ISP configuration blocks provided by the platform.
- * This macro provides a convenient tool for the calculation.
*/
-#define v4l2_isp_params_buffer_size(max_params_size) \
- (offsetof(struct v4l2_isp_params_buffer, data) + (max_params_size))
+#define v4l2_isp_buffer_size(max_size) \
+ (offsetof(struct v4l2_isp_buffer, data) + (max_size))
(I wrote this before noticing your patch :)
> /**
> * v4l2_isp_params_validate_buffer_size - Validate a V4L2 ISP buffer sizes
> * @dev: the driver's device pointer
> --
> 2.51.0
>
^ permalink raw reply
* Re: [PATCH RFC 2/7] media: qcom: iris: gen2: add support for 10bit decoding
From: Dikshita Agarwal @ 2026-04-17 7:22 UTC (permalink / raw)
To: Neil Armstrong, Vikash Garodia, Abhinav Kumar,
Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: linux-media, linux-arm-msm, linux-kernel
In-Reply-To: <20260408-topic-sm8x50-iris-10bit-decoding-v1-2-428c1ec2e3f3@linaro.org>
On 4/8/2026 10:13 PM, Neil Armstrong wrote:
> Add the necessary plumbing into the HFi Gen2 to signal the decoder
> the right 10bit pixel format and stride when in compressed mode.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
> .../platform/qcom/iris/iris_hfi_gen2_command.c | 71 +++++++++++++++++++++-
> .../platform/qcom/iris/iris_hfi_gen2_defines.h | 1 +
> drivers/media/platform/qcom/iris/iris_utils.c | 4 +-
> 3 files changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> index 30bfd90d423b..8e547e390fa3 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> @@ -481,8 +481,20 @@ static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32 plane)
>
> if (inst->domain == DECODER) {
> pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
> - hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
> - HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
> + switch (pixelformat) {
> + case V4L2_PIX_FMT_NV12:
> + hfi_colorformat = HFI_COLOR_FMT_NV12;
> + break;
> + case V4L2_PIX_FMT_QC08C:
> + hfi_colorformat = HFI_COLOR_FMT_NV12_UBWC;
> + break;
> + case V4L2_PIX_FMT_P010:
> + hfi_colorformat = HFI_COLOR_FMT_P010;
> + break;
> + case V4L2_PIX_FMT_QC10C:
> + hfi_colorformat = HFI_COLOR_FMT_TP10_UBWC;
> + break;
> + };
> } else {
> pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
> hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
> @@ -517,7 +529,8 @@ static int iris_hfi_gen2_set_linear_stride_scanline(struct iris_inst *inst, u32
> stride_uv = stride_y;
> scanline_uv = scanline_y / 2;
>
> - if (pixelformat != V4L2_PIX_FMT_NV12)
> + if (pixelformat != V4L2_PIX_FMT_NV12 &&
> + pixelformat != V4L2_PIX_FMT_P010)
> return 0;
>
> payload[0] = stride_y << 16 | scanline_y;
> @@ -532,6 +545,57 @@ static int iris_hfi_gen2_set_linear_stride_scanline(struct iris_inst *inst, u32
> sizeof(u64));
> }
>
> +static int iris_hfi_gen2_set_ubwc_stride_scanline(struct iris_inst *inst, u32 plane)
> +{
> + u32 meta_stride_y, meta_scanline_y, meta_stride_uv, meta_scanline_uv;
> + u32 stride_y, scanline_y, stride_uv, scanline_uv;
> + u32 port = iris_hfi_gen2_get_port(inst, plane);
> + u32 pixelformat, width, height;
> + u32 payload[4];
> +
> + pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
> + width = inst->fmt_dst->fmt.pix_mp.width;
> + height = inst->fmt_dst->fmt.pix_mp.height;
This HFI is only applicable to AV1, you might see some corruption due to
this. Please check.
Thanks,
Dikshita
^ permalink raw reply
* Re: [PATCH RFC 3/7] media: qcom: iris: add helpers for 8bit and 10bit formats
From: Dikshita Agarwal @ 2026-04-17 7:23 UTC (permalink / raw)
To: Neil Armstrong, Vikash Garodia, Abhinav Kumar,
Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: linux-media, linux-arm-msm, linux-kernel
In-Reply-To: <20260408-topic-sm8x50-iris-10bit-decoding-v1-3-428c1ec2e3f3@linaro.org>
On 4/8/2026 10:13 PM, Neil Armstrong wrote:
> To simplify code checking for pixel formats, add helpers to
> check for 8bit and 10bit formats.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
> drivers/media/platform/qcom/iris/iris_utils.c | 12 ++++++++++++
> drivers/media/platform/qcom/iris/iris_utils.h | 2 ++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_utils.c b/drivers/media/platform/qcom/iris/iris_utils.c
> index 26f51a0ccd04..c75dcb8e671e 100644
> --- a/drivers/media/platform/qcom/iris/iris_utils.c
> +++ b/drivers/media/platform/qcom/iris/iris_utils.c
> @@ -40,6 +40,18 @@ bool iris_split_mode_enabled(struct iris_inst *inst)
> inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC10C;
> }
>
> +bool iris_fmt_is_8bit(__u32 pixelformat)
> +{
> + return pixelformat == V4L2_PIX_FMT_NV12 ||
> + pixelformat == V4L2_PIX_FMT_QC08C;
> +}
> +
> +bool iris_fmt_is_10bit(__u32 pixelformat)
> +{
> + return pixelformat == V4L2_PIX_FMT_P010 ||
> + pixelformat == V4L2_PIX_FMT_QC10C;
> +}
A similar check is required in the current first patch of the series. So
you can move this patch earlier in the series?
Thanks,
Dikshita
^ permalink raw reply
* [PATCH v2] staging: media: ipu7: fix boot_config leak on queue_mem failure
From: Huihui Huang @ 2026-04-17 7:39 UTC (permalink / raw)
To: Sakari Ailus, Mauro Carvalho Chehab
Cc: Bingbu Cao, Greg Kroah-Hartman, linux-media, linux-staging,
linux-kernel, Huihui Huang
In-Reply-To: <20260416074800.2493565-1-hhhuang@smu.edu.sg>
There is a memory leak in drivers/staging/media/ipu7/ipu7-boot.c.
In ipu7_boot_init_boot_config(), boot_config is allocated by
ipu7_dma_alloc(). If the second ipu7_dma_alloc() for queue_mem fails,
the function returns -ENOMEM without freeing the previously allocated
boot_config.
Add the missing ipu7_dma_free() call before returning on the error
path.
Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
---
v2: Reword commit message in imperative mood. Remove unnecessary
NULL assignment on the error path.
---
drivers/staging/media/ipu7/ipu7-boot.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/media/ipu7/ipu7-boot.c b/drivers/staging/media/ipu7/ipu7-boot.c
index d7901ff78b38..495b3e05a9b1 100644
--- a/drivers/staging/media/ipu7/ipu7-boot.c
+++ b/drivers/staging/media/ipu7/ipu7-boot.c
@@ -263,6 +263,8 @@ int ipu7_boot_init_boot_config(struct ipu7_bus_device *adev,
GFP_KERNEL, 0);
if (!syscom->queue_mem) {
dev_err(dev, "Failed to allocate queue memory.\n");
+ ipu7_dma_free(adev, adev->boot_config_size,
+ adev->boot_config, adev->boot_config_dma_addr, 0);
return -ENOMEM;
}
syscom->queue_mem_size = total_queue_size_aligned;
--
2.50.1
^ permalink raw reply related
* Re: Pinned, non-revocable mappings of VRAM: will bad things happen?
From: Christian König @ 2026-04-17 7:53 UTC (permalink / raw)
To: Demi Marie Obenour, dri-devel, Xen developer discussion,
linux-media
Cc: Val Packett, Suwit Semal
In-Reply-To: <c7865b27-6bf1-4df1-9520-c9ef6b3ef368@gmail.com>
On 4/16/26 18:13, Demi Marie Obenour wrote:
> On 4/16/26 05:57, Christian König wrote:
>> On 4/16/26 01:27, Demi Marie Obenour wrote:
>>> Is it safe to assume that if a dmabuf exporter cannot handle
>>> non-revocable, pinned importers, it will fail the import? Or is
>>> using dma_buf_pin() unsafe if one does not know the exporter?
>>
>> Neither.
>>
>> dma_buf_pin() makes sure that the importer doesn't get any invalidation notifications because the exporter moves the backing store of the buffer around for memory management.
>>
>> But what is still possible is that the exporter is hot removed, in which case the importer should basically terminate it's DMA operation as soon as possible.
>>
>> GPU drivers usually reject pin requests to VRAM from DMA-buf importers when that isn't restricted by cgroups for example, because that can otherwise easily result in a deny of service.
>>
>> Amdgpu only recently started to allow pinning into VRAM to support RDMA without ODP (I think it was ODP, but could be that I mixed up the RDMA three letter code for that feature).
>>
>>> For context, Xen grant tables do not support revocation. One can ask
>>> the guest to unmap the grants, but if the guest doesn't obey the only
>>> recourse is to ungracefully kill it. They also do not support page
>>> faults, so the pages must be pinned. Right now, grant tables don't
>>> support PCI BAR mappings, but that's fixable.
>>
>> That sounds like an use case for the DMA-buf pin interface.
>>
>>> How badly is this going to break with dGPU VRAM, if at all? I know
>>> that AMDGPU has a fallback when the BAR isn't mappable. What about
>>> other drivers? Supporting page faults the way KVM does is going to
>>> be extremely hard, so pinned mappings and DMA transfers are vastly
>>> preferable.
>>
>> Well if you only want to share a fixed amount of VRAM then that is pretty much ok.
>>
>> But when the client VM can trigger pinning on demand without any limitation you can pretty easily have deny of service against the host. That is usually a rather bad idea.
>
> Is there a reasonable way to choose such an amount?
Not really.
> Unless I am
> mistaken, client workloads are highly non-uniform: a single game or
> compute job might well use more VRAM than every other program on the
> system combined.
Yeah, perfectly correct.
> Are these workloads impossible to make work well with pinning?
No, as long as you don't know the workload beforehand, e.g. when you define the limit.
I mean that's why basically everybody avoids pinning and assigning fixed amounts of resources.
Even if you can make it work technically pinning usually results in a rather bad end user experience.
Regards,
Christian.
^ permalink raw reply
* Re: [PATCH v3] staging: media: atomisp: fix memory leak of dvs2_coeff
From: Dan Carpenter @ 2026-04-17 7:55 UTC (permalink / raw)
To: Huihui Huang
Cc: Hans de Goede, Mauro Carvalho Chehab, Andy Shevchenko,
Sakari Ailus, Greg Kroah-Hartman, linux-media, linux-staging,
linux-kernel
In-Reply-To: <20260417070124.2677399-1-hhhuang@smu.edu.sg>
On Fri, Apr 17, 2026 at 03:01:24PM +0800, Huihui Huang wrote:
> There is a memory leak in
> drivers/staging/media/atomisp/pci/atomisp_compat_css20.c.
>
> In atomisp_alloc_dis_coef_buf(), dvs2_coeff is allocated by
> ia_css_dvs2_coefficients_allocate() and stored in
> asd->params.css_param.dvs2_coeff. If the subsequent
> ia_css_dvs2_statistics_allocate() for dvs_stat fails, the function
> returns -ENOMEM without freeing the previously allocated dvs2_coeff.
>
> Add the missing ia_css_dvs2_coefficients_free() call before returning
> on the error path.
>
> Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
> ---
> v3: Remove unnecessary NULL assignment per review feedback.
> v2: Reword commit message per review feedback (no code change).
> ---
> drivers/staging/media/atomisp/pci/atomisp_compat_css20.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> index be5f37f4a6fd..d6e135c42e2f 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> @@ -1363,8 +1363,10 @@ int atomisp_alloc_dis_coef_buf(struct atomisp_sub_device *asd)
> /* DIS projections. */
> asd->params.dis_proj_data_valid = false;
> asd->params.dvs_stat = ia_css_dvs2_statistics_allocate(dvs_grid);
> - if (!asd->params.dvs_stat)
> + if (!asd->params.dvs_stat) {
> + ia_css_dvs2_coefficients_free(asd->params.css_param.dvs2_coeff);
> return -ENOMEM;
> + }
The design of this code is that if we have an -ENOMEM here the caller,
atomisp_update_grid_info(), calls atomisp_css_free_stat_buffers() which
calls ia_css_dvs2_coefficients_free(). I can't tell if adding a second
ia_css_dvs2_coefficients_free() here leads to a double free.
I call this style of error handling One Magical Cleanup Function. It's
always buggy...
https://staticthinking.wordpress.com/2025/03/31/reviewing-magical-cleanup-functions/
regards,
dan carpenter
^ permalink raw reply
* [PATCH] staging: media: ipu7: fix MMU resource leak in ipu7_psys_init()
From: Huihui Huang @ 2026-04-17 7:58 UTC (permalink / raw)
To: Sakari Ailus, Mauro Carvalho Chehab
Cc: Bingbu Cao, Greg Kroah-Hartman, linux-media, linux-staging,
linux-kernel, Huihui Huang
There is a memory leak in drivers/staging/media/ipu7/ipu7.c.
In ipu7_psys_init(), psys_adev->mmu is allocated by ipu7_mmu_init().
When ipu7_bus_add_device() fails, the function returns without
cleaning up the MMU resources.
Add the missing ipu7_mmu_cleanup() call on the error path to free
the MMU resources.
Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
---
drivers/staging/media/ipu7/ipu7.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/ipu7/ipu7.c b/drivers/staging/media/ipu7/ipu7.c
index c771e763f8c5..4039e548dc8f 100644
--- a/drivers/staging/media/ipu7/ipu7.c
+++ b/drivers/staging/media/ipu7/ipu7.c
@@ -2228,6 +2228,7 @@ ipu7_psys_init(struct pci_dev *pdev, struct device *parent,
ret = ipu7_bus_add_device(psys_adev);
if (ret) {
+ ipu7_mmu_cleanup(psys_adev->mmu);
kfree(pdata);
return ERR_PTR(ret);
}
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v2] staging: media: ipu7: fix boot_config leak on queue_mem failure
From: Dan Carpenter @ 2026-04-17 8:01 UTC (permalink / raw)
To: Huihui Huang
Cc: Sakari Ailus, Mauro Carvalho Chehab, Bingbu Cao,
Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel
In-Reply-To: <20260417073939.2686170-1-hhhuang@smu.edu.sg>
On Fri, Apr 17, 2026 at 03:39:39PM +0800, Huihui Huang wrote:
> There is a memory leak in drivers/staging/media/ipu7/ipu7-boot.c.
>
> In ipu7_boot_init_boot_config(), boot_config is allocated by
> ipu7_dma_alloc(). If the second ipu7_dma_alloc() for queue_mem fails,
> the function returns -ENOMEM without freeing the previously allocated
> boot_config.
>
> Add the missing ipu7_dma_free() call before returning on the error
> path.
>
> Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
> ---
> v2: Reword commit message in imperative mood. Remove unnecessary
> NULL assignment on the error path.
> ---
> drivers/staging/media/ipu7/ipu7-boot.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/media/ipu7/ipu7-boot.c b/drivers/staging/media/ipu7/ipu7-boot.c
> index d7901ff78b38..495b3e05a9b1 100644
> --- a/drivers/staging/media/ipu7/ipu7-boot.c
> +++ b/drivers/staging/media/ipu7/ipu7-boot.c
> @@ -263,6 +263,8 @@ int ipu7_boot_init_boot_config(struct ipu7_bus_device *adev,
> GFP_KERNEL, 0);
> if (!syscom->queue_mem) {
> dev_err(dev, "Failed to allocate queue memory.\n");
> + ipu7_dma_free(adev, adev->boot_config_size,
> + adev->boot_config, adev->boot_config_dma_addr, 0);
> return -ENOMEM;
Adding a free here leads to a double free. It's the same issue.
One magical cleanup function in the caller.
I haven't looked at this but I bet there are bugs in the error handling
since magical cleanup functions are always buggy.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH RFC 2/7] media: qcom: iris: gen2: add support for 10bit decoding
From: Neil Armstrong @ 2026-04-17 8:03 UTC (permalink / raw)
To: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: linux-media, linux-arm-msm, linux-kernel
In-Reply-To: <d27e1500-ac4a-01ac-084e-bb53aa1f63b0@oss.qualcomm.com>
On 4/17/26 09:22, Dikshita Agarwal wrote:
>
>
> On 4/8/2026 10:13 PM, Neil Armstrong wrote:
>> Add the necessary plumbing into the HFi Gen2 to signal the decoder
>> the right 10bit pixel format and stride when in compressed mode.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>> .../platform/qcom/iris/iris_hfi_gen2_command.c | 71 +++++++++++++++++++++-
>> .../platform/qcom/iris/iris_hfi_gen2_defines.h | 1 +
>> drivers/media/platform/qcom/iris/iris_utils.c | 4 +-
>> 3 files changed, 72 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> index 30bfd90d423b..8e547e390fa3 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> @@ -481,8 +481,20 @@ static int iris_hfi_gen2_set_colorformat(struct iris_inst *inst, u32 plane)
>>
>> if (inst->domain == DECODER) {
>> pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
>> - hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
>> - HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
>> + switch (pixelformat) {
>> + case V4L2_PIX_FMT_NV12:
>> + hfi_colorformat = HFI_COLOR_FMT_NV12;
>> + break;
>> + case V4L2_PIX_FMT_QC08C:
>> + hfi_colorformat = HFI_COLOR_FMT_NV12_UBWC;
>> + break;
>> + case V4L2_PIX_FMT_P010:
>> + hfi_colorformat = HFI_COLOR_FMT_P010;
>> + break;
>> + case V4L2_PIX_FMT_QC10C:
>> + hfi_colorformat = HFI_COLOR_FMT_TP10_UBWC;
>> + break;
>> + };
>> } else {
>> pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
>> hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
>> @@ -517,7 +529,8 @@ static int iris_hfi_gen2_set_linear_stride_scanline(struct iris_inst *inst, u32
>> stride_uv = stride_y;
>> scanline_uv = scanline_y / 2;
>>
>> - if (pixelformat != V4L2_PIX_FMT_NV12)
>> + if (pixelformat != V4L2_PIX_FMT_NV12 &&
>> + pixelformat != V4L2_PIX_FMT_P010)
>> return 0;
>>
>> payload[0] = stride_y << 16 | scanline_y;
>> @@ -532,6 +545,57 @@ static int iris_hfi_gen2_set_linear_stride_scanline(struct iris_inst *inst, u32
>> sizeof(u64));
>> }
>>
>> +static int iris_hfi_gen2_set_ubwc_stride_scanline(struct iris_inst *inst, u32 plane)
>> +{
>> + u32 meta_stride_y, meta_scanline_y, meta_stride_uv, meta_scanline_uv;
>> + u32 stride_y, scanline_y, stride_uv, scanline_uv;
>> + u32 port = iris_hfi_gen2_get_port(inst, plane);
>> + u32 pixelformat, width, height;
>> + u32 payload[4];
>> +
>> + pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
>> + width = inst->fmt_dst->fmt.pix_mp.width;
>> + height = inst->fmt_dst->fmt.pix_mp.height;
>
> This HFI is only applicable to AV1, you might see some corruption due to
> this. Please check.
This is what I saw looking at donwstream, but I had not implemented this initially
but I got some corruption with some different width/height, which was solved adding
this command.
I guess this in-firmware calculations are not exactly the same the DRM driver expects.
Honestly I think it's preferable to sync the stride/scanlines calculations between the
driver and the firmware.
For example I used the same "ALIGN(width, 192)" for the v2 iris patchset which was wrong
because ALIGN only works on Power Of Two number but it still worked because this HFI
HFI_PROP_UBWC_STRIDE_SCANLINE command was called with the wrong aligned width.
Neil
>
> Thanks,
> Dikshita
^ permalink raw reply
* Re: [PATCH RFC 1/7] media: qcom: iris: add QC10C & P010 buffer size calculations
From: Neil Armstrong @ 2026-04-17 8:03 UTC (permalink / raw)
To: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: linux-media, linux-arm-msm, linux-kernel
In-Reply-To: <0afa05c9-1cbe-88b6-32d6-f65813391817@oss.qualcomm.com>
On 4/17/26 08:47, Dikshita Agarwal wrote:
>
>
> On 4/8/2026 10:13 PM, Neil Armstrong wrote:
>> The P010 (YUV format with 16-bits per pixel with interleaved UV)
>> and QC10C (P010 compressed mode similar to QC08C) requires specific
>> buffer calculations to allocate the right buffer size for DPB frames
>> and frames consumed by userspace.
>>
>> Similar to 8bit, the 10bit DPB frames uses QC10C format.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>> drivers/media/platform/qcom/iris/iris_buffer.c | 81 +++++++++++++++++++++++++-
>> 1 file changed, 80 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
>> index 9151f43bc6b9..a0e31bff8f26 100644
>> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
>> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
>> @@ -15,7 +15,9 @@
>> #define MAX_WIDTH 4096
>> #define MAX_HEIGHT 2304
>> #define Y_STRIDE_ALIGN 128
>> +#define Y_STRIDE_ALIGN_P010 256
>> #define UV_STRIDE_ALIGN 128
>> +#define UV_STRIDE_ALIGN_P010 256
>> #define Y_SCANLINE_ALIGN 32
>> #define UV_SCANLINE_ALIGN 16
>> #define UV_SCANLINE_ALIGN_QC08C 32
>> @@ -80,6 +82,26 @@ static u32 iris_yuv_buffer_size_nv12(struct iris_inst *inst)
>> return ALIGN(y_plane + uv_plane, PIXELS_4K);
>> }
>>
>> +static u32 iris_yuv_buffer_size_p010(struct iris_inst *inst)
>> +{
>> + u32 y_plane, uv_plane, y_stride, uv_stride, y_scanlines, uv_scanlines;
>> + struct v4l2_format *f;
>> +
>> + if (inst->domain == DECODER)
>> + f = inst->fmt_dst;
>> + else
>> + f = inst->fmt_src;
>> +
>> + y_stride = ALIGN(f->fmt.pix_mp.width * 2, Y_STRIDE_ALIGN_P010);
>> + uv_stride = ALIGN(f->fmt.pix_mp.width * 2, UV_STRIDE_ALIGN_P010);
>> + y_scanlines = ALIGN(f->fmt.pix_mp.height, Y_SCANLINE_ALIGN);
>> + uv_scanlines = ALIGN((f->fmt.pix_mp.height + 1) >> 1, UV_SCANLINE_ALIGN);
>> + y_plane = y_stride * y_scanlines;
>> + uv_plane = uv_stride * uv_scanlines;
>> +
>> + return ALIGN(y_plane + uv_plane, PIXELS_4K);
>> +}
>> +
>> /*
>> * QC08C:
>> * Compressed Macro-tile format for NV12.
>> @@ -204,6 +226,55 @@ static u32 iris_yuv_buffer_size_qc08c(struct iris_inst *inst)
>> return ALIGN(y_meta_plane + y_plane + uv_meta_plane + uv_plane, PIXELS_4K);
>> }
>>
>> +/*
>> + * QC10C:
>> + * Compressed Macro-tile format for TP10.
>> + */
>> +static u32 iris_yuv_buffer_size_qc10c(struct iris_inst *inst)
>> +{
>> + u32 y_stride, y_buf_height;
>> + u32 uv_stride, uv_buf_height;
>> + u32 y_md_stride, y_md_height;
>> + u32 uv_md_stride, uv_md_height;
>> + u32 y_data_size, uv_data_size;
>> + u32 y_md_size, uv_md_size;
>> + struct v4l2_format *f = NULL;
>> +
>> + if (inst->domain == DECODER)
>> + f = inst->fmt_dst;
>> + else
>> + f = inst->fmt_src;
>> +
>> + y_stride = ALIGN(ALIGN(f->fmt.pix_mp.width, 192) * 4 / 3,
>> + Y_STRIDE_ALIGN_P010);
>
> Y_STRIDE_ALIGN_P010 is being used for both P010 and QC10C, lets keep it
> Y_STRIDE_ALIGN_10_BIT ? or something similar ?
>
>> + y_buf_height = ALIGN(f->fmt.pix_mp.height, UV_SCANLINE_ALIGN);
>
> why not call it y_scanlines only?
>
>> +
>> + y_data_size = ALIGN(y_stride * y_buf_height, PIXELS_4K);
>
> s/y_data_size/y_plane ?
>
>> +
>> + uv_stride = ALIGN(ALIGN(f->fmt.pix_mp.width, 192) * 4 / 3,
>> + UV_STRIDE_ALIGN_P010);
>> + uv_buf_height = ALIGN((f->fmt.pix_mp.height + 1) / 2,
>> + UV_SCANLINE_ALIGN);
>
> s/uv_buf_height/uv_scanline?
>
>> +
>> + uv_data_size = ALIGN(uv_stride * uv_buf_height, PIXELS_4K);
>
> s/uv_data_size/uv_plane?
>
> Pls keep all these names consistent with other functions, applies to below
> variables as well.
>
>
> Thanks,
> Dikshita
>
>> +
>> + y_md_stride = ALIGN(DIV_ROUND_UP(f->fmt.pix_mp.width, 48),
>> + META_STRIDE_ALIGNED);
>> + y_md_height = ALIGN(DIV_ROUND_UP(f->fmt.pix_mp.height, 4),
>> + META_SCANLINE_ALIGNED);
>> +
>> + y_md_size = ALIGN(y_md_stride * y_md_height, PIXELS_4K);
>> +
>> + uv_md_stride = ALIGN(DIV_ROUND_UP((f->fmt.pix_mp.width + 1) / 2, 24),
>> + META_STRIDE_ALIGNED);
>> + uv_md_height = ALIGN(DIV_ROUND_UP((f->fmt.pix_mp.height + 1) / 2, 4),
>> + META_SCANLINE_ALIGNED);
>> +
>> + uv_md_size = ALIGN(uv_md_stride * uv_md_height, PIXELS_4K);
>> +
>> + return y_data_size + uv_data_size + y_md_size + uv_md_size;
>> +}
>> +
>> static u32 iris_dec_bitstream_buffer_size(struct iris_inst *inst)
>> {
>> struct platform_inst_caps *caps = inst->core->iris_platform_data->inst_caps;
>> @@ -268,10 +339,18 @@ int iris_get_buffer_size(struct iris_inst *inst,
>> case BUF_OUTPUT:
>> if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C)
>> return iris_yuv_buffer_size_qc08c(inst);
>> + else if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC10C)
>> + return iris_yuv_buffer_size_qc10c(inst);
>> + else if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_P010)
>> + return iris_yuv_buffer_size_p010(inst);
>> else
>> return iris_yuv_buffer_size_nv12(inst);
>> case BUF_DPB:
>> - return iris_yuv_buffer_size_qc08c(inst);
>> + if (inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC10C ||
>> + inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_P010)
>
> Please introduce one API, is_10bit_format and use that here instead.
Ack
>
> Thanks,
> Dikshita
>> + return iris_yuv_buffer_size_qc10c(inst);
>> + else
>> + return iris_yuv_buffer_size_qc08c(inst);
>> default:
>> return 0;
>> }
>>
^ permalink raw reply
* Re: [PATCH RFC 3/7] media: qcom: iris: add helpers for 8bit and 10bit formats
From: Neil Armstrong @ 2026-04-17 8:03 UTC (permalink / raw)
To: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
Bryan O'Donoghue, Mauro Carvalho Chehab
Cc: linux-media, linux-arm-msm, linux-kernel
In-Reply-To: <7ca67466-51d7-0003-47c6-9d0e6518745e@oss.qualcomm.com>
On 4/17/26 09:23, Dikshita Agarwal wrote:
>
>
> On 4/8/2026 10:13 PM, Neil Armstrong wrote:
>> To simplify code checking for pixel formats, add helpers to
>> check for 8bit and 10bit formats.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>> drivers/media/platform/qcom/iris/iris_utils.c | 12 ++++++++++++
>> drivers/media/platform/qcom/iris/iris_utils.h | 2 ++
>> 2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_utils.c b/drivers/media/platform/qcom/iris/iris_utils.c
>> index 26f51a0ccd04..c75dcb8e671e 100644
>> --- a/drivers/media/platform/qcom/iris/iris_utils.c
>> +++ b/drivers/media/platform/qcom/iris/iris_utils.c
>> @@ -40,6 +40,18 @@ bool iris_split_mode_enabled(struct iris_inst *inst)
>> inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC10C;
>> }
>>
>> +bool iris_fmt_is_8bit(__u32 pixelformat)
>> +{
>> + return pixelformat == V4L2_PIX_FMT_NV12 ||
>> + pixelformat == V4L2_PIX_FMT_QC08C;
>> +}
>> +
>> +bool iris_fmt_is_10bit(__u32 pixelformat)
>> +{
>> + return pixelformat == V4L2_PIX_FMT_P010 ||
>> + pixelformat == V4L2_PIX_FMT_QC10C;
>> +}
>
> A similar check is required in the current first patch of the series. So
> you can move this patch earlier in the series?
Ack
>
> Thanks,
> Dikshita
>
^ permalink raw reply
* Re: [PATCH v2] staging: media: ipu7: fix boot_config leak on queue_mem failure
From: Dan Carpenter @ 2026-04-17 8:05 UTC (permalink / raw)
To: Huihui Huang
Cc: Sakari Ailus, Mauro Carvalho Chehab, Bingbu Cao,
Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel
In-Reply-To: <aeHo2WOF1Rcp9zwf@stanley.mountain>
On Fri, Apr 17, 2026 at 11:01:29AM +0300, Dan Carpenter wrote:
> On Fri, Apr 17, 2026 at 03:39:39PM +0800, Huihui Huang wrote:
> > There is a memory leak in drivers/staging/media/ipu7/ipu7-boot.c.
> >
> > In ipu7_boot_init_boot_config(), boot_config is allocated by
> > ipu7_dma_alloc(). If the second ipu7_dma_alloc() for queue_mem fails,
> > the function returns -ENOMEM without freeing the previously allocated
> > boot_config.
> >
> > Add the missing ipu7_dma_free() call before returning on the error
> > path.
> >
> > Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
> > ---
> > v2: Reword commit message in imperative mood. Remove unnecessary
> > NULL assignment on the error path.
> > ---
> > drivers/staging/media/ipu7/ipu7-boot.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/staging/media/ipu7/ipu7-boot.c b/drivers/staging/media/ipu7/ipu7-boot.c
> > index d7901ff78b38..495b3e05a9b1 100644
> > --- a/drivers/staging/media/ipu7/ipu7-boot.c
> > +++ b/drivers/staging/media/ipu7/ipu7-boot.c
> > @@ -263,6 +263,8 @@ int ipu7_boot_init_boot_config(struct ipu7_bus_device *adev,
> > GFP_KERNEL, 0);
> > if (!syscom->queue_mem) {
> > dev_err(dev, "Failed to allocate queue memory.\n");
> > + ipu7_dma_free(adev, adev->boot_config_size,
> > + adev->boot_config, adev->boot_config_dma_addr, 0);
> > return -ENOMEM;
>
> Adding a free here leads to a double free. It's the same issue.
> One magical cleanup function in the caller.
>
> I haven't looked at this but I bet there are bugs in the error handling
> since magical cleanup functions are always buggy.
Btw, if you had kept the "adev->boot_config = NULL;" assignment that
you had in v1 then that would have prevented the double free since
ipu7_boot_release_boot_config() tests for that... This information is
not useful to you at this point but I'm sure you will find it
frustrating. :P
regards,
dan carpenter
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox