| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a45d0c97-9e31-42a9-aeed-6f874f4f7321@kwiboo.se>
Date: Sun, 17 Aug 2025 18:18:42 +0200
From: Jonas Karlman <jonas@...boo.se>
To: Nicolas Dufresne <nicolas.dufresne@...labora.com>
Cc: Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
Detlev Casanova <detlev.casanova@...labora.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>, Heiko Stuebner
<heiko@...ech.de>, Alex Bee <knaerzche@...il.com>,
Sebastian Fricke <sebastian.fricke@...labora.com>,
linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/7] media: rkvdec: Disable QoS for HEVC and VP9 on
RK3328
On 8/12/2025 3:00 PM, Nicolas Dufresne wrote:
> Le mardi 12 août 2025 à 01:08 +0200, Jonas Karlman a écrit :
>> Hi Nicolas,
>>
>> Missed some comments in my last mail.
>>
>> On 8/11/2025 11:25 PM, Nicolas Dufresne wrote:
>>> Le dimanche 10 août 2025 à 21:24 +0000, Jonas Karlman a écrit :
>>>> From: Alex Bee <knaerzche@...il.com>
>>>>
>>>> The RK3328 VDEC has a HW quirk that require QoS to be disabled when HEVC
>>>> or VP9 is decoded, otherwise the decoded picture may become corrupted.
>>>>
>>>> Add a RK3328 variant with a quirk flag to disable QoS when before
>>>> decoding is started.
>>>>
>>>> Signed-off-by: Alex Bee <knaerzche@...il.com>
>>>> Signed-off-by: Jonas Karlman <jonas@...boo.se>
>>>> ---
>>>> Changes in v2:
>>>> - No change
>>>> ---
>>>> drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c | 9 +++++++++
>>>> drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h | 2 ++
>>>> drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c | 10 ++++++++++
>>>> drivers/media/platform/rockchip/rkvdec/rkvdec.c | 12 ++++++++++++
>>>> drivers/media/platform/rockchip/rkvdec/rkvdec.h | 4 ++++
>>>> 5 files changed, 37 insertions(+)
>>>>
>>>> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
>>>> b/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
>>>> index 1994ea24f0be..f8bb8c4264f7 100644
>>>> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
>>>> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
>>>> @@ -789,6 +789,15 @@ static int rkvdec_hevc_run(struct rkvdec_ctx *ctx)
>>>> writel(1, rkvdec->regs + RKVDEC_REG_PREF_LUMA_CACHE_COMMAND);
>>>> writel(1, rkvdec->regs + RKVDEC_REG_PREF_CHR_CACHE_COMMAND);
>>>>
>>>> + if (rkvdec->quirks & RKVDEC_QUIRK_DISABLE_QOS) {
>>>> + u32 reg;
>>>> +
>>>> + reg = readl(rkvdec->regs + RKVDEC_REG_QOS_CTRL);
>>>> + reg |= 0xFFFF;
>>>> + reg &= ~BIT(12);
>>>
>>> I wonder if there is a better way to express that, if not, a comment for
>>> future
>>> readers would be nice. If read it will, we keep the upper 16bit, and
>>> replaced
>>> the lower bits with 0xEFFF (all bits set except 12) ? I'd rather not spend
>>> time
>>> thinking if I walk by this code again.
>>
>> Vendor kernel use following comment to describe the purpose of this [1]:
>>
>> HW defeat workaround: VP9 and H.265 power save optimization cause
>> decoding corruption, disable optimization here.
>>
>> From the TRM we can see following for rkvdec_swreg99_qos_ctrl:
>>
>> 27:26 sw_axi_wr_hurry_level
>> 00: hurry off
>> 01~11: hurry level
>> 25:24 sw_axi_rd_hurry_level
>> 00: hurry off
>> 01~11: hurry level
>> 23:16 sw_bus2mc_buffer_qos_level
>> range is: 0~255
>> the value is means that left space <=
>> sw_bus2mc_buffer_qos_level, it will give hurry
>> 15:0 swreg_block_gating_e
>>
>> So yes this set swreg_block_gating_e to 0xEFFF. Possible this configure
>> hw to not auto gate most internal clocks?
>>
>> Could add a comment and possible use something like following:
>>
>> reg &= GENMASK(31, 16);
>> reg |= 0xEFFF;
>
> Thanks for the information, I think this form is somewhat nicer indeed, and a
> little comment, its fine to say that the QOS bits are undocumented.
Sure, I will update this in a v3.
Regards,
Jonas
>
> Nicolas
>
>>
>> [1]
>> https://github.com/Kwiboo/linux-rockchip/blob/linux-6.1-stan-rkr6.1/drivers/video/rockchip/mpp/mpp_rkvdec.c#L857-L867
>>
>>>
>>>> + writel(reg, rkvdec->regs + RKVDEC_REG_QOS_CTRL);
>>>> + }
>>>> +
>>>> /* Start decoding! */
>>>> reg = (run.pps->flags & V4L2_HEVC_PPS_FLAG_TILES_ENABLED) ?
>>>> 0 : RKVDEC_WR_DDR_ALIGN_EN;
>>>> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
>>>> b/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
>>>> index 540c8bdf24e4..c627b6b6f53a 100644
>>>> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
>>>> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
>>>> @@ -219,6 +219,8 @@
>>>> #define RKVDEC_REG_H264_ERR_E 0x134
>>>> #define RKVDEC_H264_ERR_EN_HIGHBITS(x) ((x) &
>>>> 0x3fffffff)
>>>>
>>>> +#define RKVDEC_REG_QOS_CTRL 0x18C
>>>> +
>>>> #define RKVDEC_REG_PREF_LUMA_CACHE_COMMAND 0x410
>>>> #define RKVDEC_REG_PREF_CHR_CACHE_COMMAND 0x450
>>>>
>>>> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
>>>> b/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
>>>> index 0e7e16f20eeb..cadb9d592308 100644
>>>> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
>>>> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
>>>> @@ -824,6 +824,16 @@ static int rkvdec_vp9_run(struct rkvdec_ctx *ctx)
>>>> writel(1, rkvdec->regs + RKVDEC_REG_PREF_CHR_CACHE_COMMAND);
>>>>
>>>> writel(0xe, rkvdec->regs + RKVDEC_REG_STRMD_ERR_EN);
>>>> +
>>>> + if (rkvdec->quirks & RKVDEC_QUIRK_DISABLE_QOS) {
>>>> + u32 reg;
>>>> +
>>>> + reg = readl(rkvdec->regs + RKVDEC_REG_QOS_CTRL);
>>>> + reg |= 0xFFFF;
>>>> + reg &= ~BIT(12);
>>>> + writel(reg, rkvdec->regs + RKVDEC_REG_QOS_CTRL);
>>>
>>> Can we deduplicate that ?
>>
>> Guess so, any suggestion on how to best do that?
>>
>> One possible way that comes to mind:
>>
>> if (rkvdec->quirks & RKVDEC_QUIRK_DISABLE_QOS)
>> rkvdec_quirk_disable_qos(rkvdec);
>>
>>>
>>>> + }
>>>> +
>>>> /* Start decoding! */
>>>> writel(RKVDEC_INTERRUPT_DEC_E | RKVDEC_CONFIG_DEC_CLK_GATE_E |
>>>> RKVDEC_TIMEOUT_E | RKVDEC_BUF_EMPTY_E,
>>
>> [snip]
Powered by blists - more mailing lists