Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <83fb974f-2c5a-0792-30f7-a317813f0eed@linaro.org>
Date:   Wed, 12 Apr 2023 21:56:44 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [Freedreno] [PATCH] drm/msm/dpu: add DSC range checking during
 resource reservation
Content-Language: en-GB
To:     Marijn Suijten <marijn.suijten@somainline.org>,
        Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc:     Kuogee Hsieh <quic_khsieh@quicinc.com>,
        freedreno@lists.freedesktop.org, quic_sbillaka@quicinc.com,
        airlied@gmail.com, andersson@kernel.org, robdclark@gmail.com,
        dri-devel@lists.freedesktop.org, dianders@chromium.org,
        vkoul@kernel.org, agross@kernel.org, daniel@ffwll.ch,
        linux-arm-msm@vger.kernel.org, swboyd@chromium.org,
        sean@poorly.run, linux-kernel@vger.kernel.org
References: <1681247380-1607-1-git-send-email-quic_khsieh@quicinc.com>
 <qvgbm3wimai3jytnikbcixipvwqn2uywqpg4mn6mjh5atergfx@wa4edsrp7y22>
 <96416911-bca3-b007-b036-1c4463e83aaa@quicinc.com>
 <24c5aa23-9b3c-787c-10aa-e9d5ad91512b@linaro.org>
 <49479b93-b364-d882-7a77-08223a94ed36@quicinc.com>
 <tczt5alqbadkodgorqm4pljpqkn5bc4efpxiy3em7bgu7gqaka@3cdszu4k6rhk>
 <8310d7ce-7ac0-05a6-b95a-c18a498f7644@quicinc.com>
 <szwu75yxcfxeyvfvrsyuoc3jeoaylydwtlzm3cevmpr3zpmfpo@wrdgbf3w3de2>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
In-Reply-To: <szwu75yxcfxeyvfvrsyuoc3jeoaylydwtlzm3cevmpr3zpmfpo@wrdgbf3w3de2>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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