[PATCH v13 6/6] iommu/arm-smmu: add support for PRR bit setup

[Bibek Kumar Patro]

On 7/16/2024 5:44 PM, Konrad Dybcio wrote:
> On 15.07.2024 10:07 PM, Rob Clark wrote:
>> On Mon, Jul 15, 2024 at 4:00 AM Bibek Kumar Patro
>> <quic_bibekkum at quicinc.com> wrote:
> 
> [...]
> 
>>>>> As I checked gpu_prr_mem reserved mem section is not used for mobile
>>>>> targets hence not present for other DT only compute targets like
>>>>> x1e80100.dtsi has the same. PRR looks to be smmu version specific
>>>>> property.
>>>>
>>>> I only see it in gpu_prr_mem in x1e80100.dtsi, but not documented
>>>> anywhere.  I'm only assuming based on the name that it is intended to
>>>> be for PRR (but not sure why it is larger than 0x1000).  Are the
>>>> PRR_CFG_*ADDR regs programmed by the fw (and access blocked in EL1) on
>>>> this device?
>>>>
>>>
>>> As I checked, if the drm/gfx driver allocates the page for drm, then
>>> this reserved-memory region is not required.
>>>
>>> PRR_CFG_*ADDR regs have read and write access in EL1 only for this
>>> device, behavior is same as other devices as well. These are not
>>> programmed by fw.
>>
>> If there is any device which _doesn't_ have EL1 access to these regs,
>> I think going the reserved memory route seems more future proof?
>> Otherwise we later on have to deal with two different ways to do
>> things.  But I'm not sure if there is any such device or risk.
> 
> We can have our cake and eat it too, if we keep the check for a
> reserved memory node handle, but make it a dynamic allocation (see
> [1] for example), this way there's a way to opt into using this from
> the DT and there's no need for adding more properties
> 

Coming to allocation methodology, here the client/user(drm/gpu
driver in this case) would be allocating the page and pass the address 
to our SMMU interface which would just configure it in the PRR_CFG_*ADDR 
register.
So I believe the choice of allocation method should be better left to 
the client, rather than restricting them to a specific strategy. (?)
If dynamically allocated without a reserved pool, this region might only 
be present in some targets and not others, which could make it look a 
bit ugly [1].

Thanks & regards,
Bibek

[1]:https://lore.kernel.org/all/CAF6AEGvZWdN+CC9O3tq7kjYPq424U6__KgAnFNCV0bCqE8wPuQ@mail.gmail.com/#:~:text=That%0Aseems%20a%20bit%20ugly%20to%20me.

> Konrad
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/msm8916.dtsi?h=v6.10#n109