Message-Id: <DC0NUGDTCUYD.QT1BLJEGXYFG@cknow.org>
Date: Tue, 12 Aug 2025 20:28:43 +0200
From: "Diederik de Haas" <didi.debian@...ow.org>
To: "Jonas Karlman" <jonas@...boo.se>
Cc: "Ezequiel Garcia" <ezequiel@...guardiasur.com.ar>, "Detlev Casanova"
 <detlev.casanova@...labora.com>, "Mauro Carvalho Chehab"
 <mchehab@...nel.org>, "Alex Bee" <knaerzche@...il.com>, "Nicolas Dufresne"
 <nicolas.dufresne@...labora.com>, "linux-media@...r.kernel.org"
 <linux-media@...r.kernel.org>, "linux-rockchip@...ts.infradead.org"
 <linux-rockchip@...ts.infradead.org>, "devicetree@...r.kernel.org"
 <devicetree@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 0/7] media: rkvdec: Add HEVC backend

Hi Jonas,

On Tue Aug 12, 2025 at 7:11 PM CEST, Jonas Karlman wrote:
> On 8/12/2025 2:11 PM, Diederik de Haas wrote:
>> On Sun Aug 10, 2025 at 11:24 PM CEST, Jonas Karlman wrote:
>>> This series add a HEVC backend to the Rockchip Video Decoder driver.
>>>
>>> With the dependent H.264 High 10 and 4:2:2 profile support series
>>> finally merged there is finally time to send a v2 with minor changes and
>>> a suggested code style fix of this series. v1 of this series has been
>>> fully functional up until recent unstaging of the rkvdec driver.
>>>
>>> Patch 1 add the new HEVC backend.
>>> Patch 2-3 add variants support to the driver.
>>> Patch 4 add support for a rk3288 variant.
>>> Patch 5 add a rk3328 variant to work around hw quirks.
>>> Patch 6-7 add device tree node for rk3288.
>> 
>> It looks like I had a previous version of linuxtv-rkvdec-hevc-v2 branch
>> locally and that also had this commit:
>> - media: rkvdec: Keep decoder clocks gated
>> 
>> Is that one no longer needed/useful/etc ?
>
> I do not think it is, could possible be to keep power consumption at
> minimum while decoding. Some parts enable auto gating and then we
> disable it when decoding is complete. With auto-suspend the entire block
> is disabled anyway so this probably did not make any noticeable
> difference and could instead introduce new possible issues.

Makes sense, thanks.
 
>> And 'chewitt' also had a commit to fix 8/10-bit selection:
>> https://github.com/chewitt/linux/commit/4b93b05d2ca608bc23f1d52bcc32df926d435c7c
>> "WIP: media: rkvdec: fix 8-bit/10-bit format selection"
>> 
>> I haven't tried that one (yet), but did  try an other variant with
>> changing the ordering in rkvdec_hevc_decoded_fmts but that didn't work
>> in my tests. (Can ofc be PEBKAC)
>
> The format selection in kernel for this series should be correct,
> however to ensure 10-bit works you need following for ffmpeg-v4l2request
> to select and use 10-bit pixel formats:
>
> libdrm 2.4.104+ (NV15) / 2.4.118+ (NV20)
> - 10-bit drm formats, ffmpeg v4l2request test with a #ifdef
>
> linux headers v6.16-rc1+ (NV15/NV20)
> - 10-bit v4l2 pix fmt, ffmpeg v4l2request test with a #ifdef
>
> FFmpeg v4l2request will not negotiate use of 10-bit formats without
> DRM_FORMAT_NV15/NV20 and V4L2_PIX_FMT_NV15/NV20 defined when ffmpeg was
> compiled.
>
> That would be the most likely issue if only 8-bit formats is working.

Thanks so much for the detailed explanation with which I can check where
my stack wasn't doing what I hoped it would :-)

>> Would that be useful? I do/did have consistent problems with playing
>> 10-bit encoded video files.
>
> Looking quickly at the 'fix 8/10-bit selection' commit the issue is that
> rkvdec_hevc_get_image_fmt() was incomplete to begin with. The
> rkvdec_hevc_get_image_fmt() in this series has been correct since v1.

Thanks :)

Cheers,
  Diederik

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists