Re: [PATCH v14 5/6] iommu/arm-smmu: add ACTLR data and support for SC7280

[Bibek Kumar Patro <quic_bibekkum@quicinc.com>]

On 8/23/2024 9:29 PM, Will Deacon wrote:
> On Fri, Aug 16, 2024 at 11:12:58PM +0530, Bibek Kumar Patro wrote:
>> Add ACTLR data table for SC7280 along with support for
>> same including SC7280 specific implementation operations.
>>
>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
>> ---
>>   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 58 +++++++++++++++++++++-
>>   1 file changed, 57 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index dc143b250704..a776c7906c76 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -31,6 +31,55 @@
>>   #define PREFETCH_MODERATE	(2 << PREFETCH_SHIFT)
>>   #define PREFETCH_DEEP		(3 << PREFETCH_SHIFT)
>>
>> +static const struct actlr_config sc7280_apps_actlr_cfg[] = {
>> +	{ 0x0800, 0x04e0, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x0900, 0x0402, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x0901, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x0d01, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x1181, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1182, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1183, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1184, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1185, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1186, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1187, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1188, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1189, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x118b, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x118c, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x118d, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x118e, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x118f, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x2000, 0x0020, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2040, 0x0000, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2062, 0x0000, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2080, 0x0020, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x20c0, 0x0020, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2100, 0x0020, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2140, 0x0000, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2180, 0x0020, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x2181, 0x0004, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x2183, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x2184, 0x0020, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x2187, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +};
>> +
>> +static const struct actlr_config sc7280_gfx_actlr_cfg[] = {
>> +	{ 0x0000, 0x07ff, PREFETCH_DEEP | CPRE | CMTLB },
>> +};
> 
> It's Will "stuck record" Deacon here again to say that I don't think
> this data belongs in the driver.
> 

Hi Will,

It will be difficult to reach a consensus here, with Robin and the DT 
folks okay to keep it in the driver, while you believe it doesn't belong 
there.

Robin, Rob, could you please share your thoughts on concluding the 
placement of this prefetch data?

As discussed earlier [1], the prefetch value for each client doesn’t 
define the hardware topology and is implementation-defined register 
writes used by the software driver.

We're okay with either approach, but these points [2] also raised in the
RFC led us to believe that switching from DT to the driver is the
right approach.


[1]:https://lore.kernel.org/all/2b0d8c5b-7e79-41ff-bc57-003d1b947c16@quicinc.com/
[2]:https://lore.kernel.org/all/a01e7e60-6ead-4a9e-ba90-22a8a6bbd03f@quicinc.com/


> Have a great weekend!
> 
> Will