Re: [PATCH v7 4/6] media: verisilicon: AV1: Restore IOMMU context before decoding a frame

[Benjamin Gaignard <benjamin.gaignard@collabora.com>]

Le 26/08/2025 à 14:41, Jason Gunthorpe a écrit :
> On Tue, Aug 26, 2025 at 11:52:37AM +0200, Benjamin Gaignard wrote:
>> Le 25/08/2025 à 20:31, Jason Gunthorpe a écrit :
>>> On Mon, Aug 25, 2025 at 01:50:16PM -0400, Nicolas Dufresne wrote:
>>>
>>>> Jason, the point is that the iommu and the VPU are not separate devices, which
>>>> comes with side effects. On RKVDec side, the iommu configuration get resets
>>>> whenever a decoding error leads to a VPU "self reset". I can't remember who from
>>>> the iommu subsystem suggested that, but the empty domain method was agreed to be
>>> IDK, that seems really goofy too me an defiantly needs to be
>>> extensively documented this is restoring the default with some lore
>>> link of the original suggestion.
>>>
>>>> the least invasive way to workaround that issue. I believe Detlev tried multiple
>>>> time to add APIs for that before the discussion lead to this path.
>>> You mean this:
>>>
>>> https://lore.kernel.org/linux-iommu/20250318152049.14781-1-detlev.casanova@collabora.com/
>>>
>>> Which came back with the same remark I would give:
>>>
>>>    Please have some kind of proper reset notifier mechanism - in fact
>>>    with runtime PM could you not already invoke a suspend/resume cycle
>>>    via the device links?
>> when doing parallel decode suspend/resume are not invoked.
> It was a proposal for an error recovery path.
>
>>> Or another reasonable option:
>>>
>>>     Or at worst just export a public interface for the other driver to
>>>     invoke rk_iommu_resume() directly.
>>>
>>> Sigh.
>> An other solution which is working is to call iommu_flush_iotlb_all()
>> before decoding each frame.
> That was already proposed and shot down, it makes no sense at all use
> to use flushing to reset the registers because the HW weirdly lost
> them, and flushing should never happen outside mapping contexts.
>
> If the HW is really resetting the iommu registers after every frame
> that is really just painfully broken, and makes me wonder if it really
> should be an iommu subsystem driver at all if it is so tightly coupled
> to the computing HW. :\
>
> Like we wouldn't try to put a GPU MMU into the iommu subsystem though
> they do fairly similar things.

I will add vsi_iommu_restore_ctx() function where all locks will be
managed and call vsi_iommu_enable(). Decoder driver will call this function
before each frame.
I think it similar to some omap iommu code I have found.

Regards,
Benjamin

> Jason
>