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

[Robin Murphy <robin.murphy@arm.com>]

On 2022-04-19 22:08, Dmitry Baryshkov wrote:
> 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 :-)

Awesome, thanks!

Robin.