lkml.org 
[lkml]   [2023]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [Freedreno] [PATCH] drm/msm/dpu: add DSC range checking during resource reservation
From
On 12/04/2023 21:50, Marijn Suijten wrote:
> On 2023-04-12 10:48:18, Abhinav Kumar wrote:
> [..]
>>> The only way to trigger this newly introduced range check is by omitting
>>> the DSC_x constants and manually writing e.g. an out-of-range value 10
>>> here, or setting DSC_NONE. This is only allowed for interfaces.
>>>
>>
>> Correct, its just working on an implicit understanding that the indices
>> in the catalog
>
> .. this sentence appears to be incomplete: what did you want to say? ..
>
>> which might still be right stick to the RM limits.
>>
>> Thats why this is not bad to have.
>
> What do you mean by "RM limits"? We have constants in the kernel that
> both define the maximum number of blocks in these arrays and a
> predefined set of ids that block can have. These are all used in
> constant structs in the catalog, so there's nothing "software" or
> SoC-specific limiting about this (except what is available in the
> arrays).
>
> [..]
>> I think kuogee just added this to keep it consistent with other checks
>> present in the RM. So I didnt see any harm with that.
>
> Yep, that's the only reason
>
>> If he did see an issue, i will let him report that here.
>
> If so an out-of-bounds constant was hardcoded in dpu_hw_catalog.c.
>
>> Otherwise, I dont want to spend more time discussing this bounds check
>> when other blocks already have it.
>
> I'll whip up a patch to clear out the extraneous lookup (assuming there
> is no other reason/dependency for it to be there...) and can follow that
> up with removing these range checks of known-good values in `const
> struct` fields.

Yes, please.

>
> - Marijn

--
With best wishes
Dmitry

\
 
 \ /
  Last update: 2023-04-12 20:57    [W:0.058 / U:0.428 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site