[PATCH] drm/msm: Revert "drm/msm: Stop using iommu_present()"

[Dmitry Baryshkov]

On 20/04/2022 00:04, Robin Murphy wrote:
> On 2022-04-19 14:04, Dmitry Baryshkov wrote:
>> This reverts commit e2a88eabb02410267519b838fb9b79f5206769be. The commit
>> in question makes msm_use_mmu() check whether the DRM 'component master'
>> device is translated by the IOMMU. At this moment it is the 'mdss'
>> device.
>> However on platforms using the MDP5 driver (e.g. MSM8916/APQ8016,
>> MSM8996/APQ8096) it's the mdp5 device, which has the iommus property
>> (and thus is "translated by the IOMMU"). This results in these devices
>> being broken with the following lines in the dmesg.
>>
>> [drm] Initialized msm 1.9.0 20130625 for 1a00000.mdss on minor 0
>> msm 1a00000.mdss: [drm:adreno_request_fw] loaded qcom/a300_pm4.fw from 
>> new location
>> msm 1a00000.mdss: [drm:adreno_request_fw] loaded qcom/a300_pfp.fw from 
>> new location
>> msm 1a00000.mdss: [drm:get_pages] *ERROR* could not get pages: -28
>> msm 1a00000.mdss: could not allocate stolen bo
>> msm 1a00000.mdss: [drm:get_pages] *ERROR* could not get pages: -28
>> msm 1a00000.mdss: [drm:msm_alloc_stolen_fb] *ERROR* failed to allocate 
>> buffer object
>> msm 1a00000.mdss: [drm:msm_fbdev_create] *ERROR* failed to allocate fb
>>
>> Getting the mdp5 device pointer from this function is not that easy at
>> this moment. Thus this patch is reverted till the MDSS rework [1] lands.
>> It will make the mdp5/dpu1 device component master and the check will be
>> legit.
>>
>> [1] https://patchwork.freedesktop.org/series/98525/
> 
> Oh, DRM...
> 
> If that series is going to land got 5.19, could you please implement the 
> correct equivalent of this patch within it?

Yes, that's the plan. I'm sending a reworked version of your patch 
shortly (but it still depends on [1]).

> 
> I'm fine with the revert for now if this patch doesn't work properly in 
> all cases, but I have very little sympathy left for DRM drivers riding 
> roughshod over all the standard driver model abstractions because 
> they're "special". iommu_present() *needs* to go away, so if it's left 
> to me to have a second go at fixing this driver next cycle, you're 
> liable to get some abomination based on of_find_compatible_node() or 
> similar, and I'll probably be demanding an ack to take it through the 
> IOMMU tree ;)

No need for such measures :-)

> 
> Thanks,
> Robin.
> 
>> Fixes: e2a88eabb024 ("drm/msm: Stop using iommu_present()")
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>>   drivers/gpu/drm/msm/msm_drv.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_drv.c 
>> b/drivers/gpu/drm/msm/msm_drv.c
>> index b6702b0fafcb..e2b5307b2360 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.c
>> +++ b/drivers/gpu/drm/msm/msm_drv.c
>> @@ -263,7 +263,7 @@ bool msm_use_mmu(struct drm_device *dev)
>>       struct msm_drm_private *priv = dev->dev_private;
>>       /* a2xx comes with its own MMU */
>> -    return priv->is_a2xx || device_iommu_mapped(dev->dev);
>> +    return priv->is_a2xx || iommu_present(&platform_bus_type);
>>   }
>>   static int msm_init_vram(struct drm_device *dev)


-- 
With best wishes
Dmitry