Re: [Intel-xe] [PATCH] drm/xe/uapi: Remove MMIO ioctl

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 18 Sep 2023, Ofir Bitton <obitton@habana.ai> wrote:
> On 14/09/2023 23:47, Daniel Vetter wrote:
>> On Thu, 14 Sept 2023 at 16:21, Ofir Bitton <obitton@habana.ai> wrote:
>>>
>>> On 14/09/2023 11:35, Jani Nikula wrote:
>>>> On Tue, 12 Sep 2023, Ofir Bitton <obitton@habana.ai> wrote:
>>>>> On 12/09/2023 14:11, Jani Nikula wrote:
>>>>>> On Tue, 12 Sep 2023, Ofir Bitton <obitton@habana.ai> wrote:
>>>>>>> On 12/09/2023 3:25, Matt Roper wrote:
>>>>>>> Hey Matt, I totally undesrstand your concern, I might have another
>>>>>>> suggestion. We can create another FD in debugfs and move this ioctl
>>>>>>> there (I can take ownership on this), This way ABI is not an issue.
>>>>>>
>>>>>> FD or ioctl in debugfs? Or do you just mean adding a debugfs file for
>>>>>> register access?
>>>>>>
>>>>>> BR,
>>>>>> Jani.
>>>>>>
>>>>>
>>>>> Add a new file in debugfs to which we will send debug ioctls such as the
>>>>> mmio ioctl.
>>>>
>>>> It's so rare to do ioctl on debugfs files that I first had to check it's
>>>> possible, and then try to find examples in the kernel. I found one so
>>>> far, though there are probably more.
>>>>
>>>> If it's that rare, usually the question is, does it make sense?
>>>>
>>>>
>>>> BR,
>>>> Jani.
>>>>
>>>>
>>>
>>> I actually got this idea from Daniel few months back during a different
>>> discussion. Daniel any thoughts on this?
>>
>> So the backstory is that some simulation interface for gaudi used a
>> chardev node, for the efficiency/flexibilty of ioctl. Which for
>> upstream is a no-go, we really don't want to make val/sim stuff stable
>> uapi. But in general I'm very much welcome to upstreaming
>> debug/sim/val infrastructure, anything that's reasonable and reduces
>> the delta against internal/downstream trees is good, and the ioctl
>> interface seems like the right fit, and the stable uapi issue can be
>> avoided by moving it all into debugfs.
>>
>> That's how the debugfs-with-ioctl idea was born.
>>
>> Now since it's debugfs I really don't care much (but maybe
>> double-check with Dave Airlie), as long as we don't go overboard and
>> use ioctl for absolutely everything just because we can. Because in
>> general I think debugfs should be human readable and useable with just
>> commandline, very often that's really the most convenient interface.
>> But if we need something where ioctl is just the better fit, then yeah
>> ioctl in debugfs is imo ok.
>>
>> Cheers!
>>
>
> Thanks Daniel for the detailed input! I think in our case we can use
> a debugfs ioctl ONLY for the mmio case, as indeed here it is the best
> fit. Jani, any objection?

Ack on the plan.

And since it's debugfs, we can actually change it afterwards. :)

BR,
Jani.


>
>>> If you are uncomfortable with the ioctl approach we can go with a
>>> different approach, for example what we did in the habanalabs driver:
>>>
>>> setting read/write address:
>>> https://elixir.bootlin.com/linux/v6.6-rc1/source/drivers/accel/habanalabs/common/debugfs.c#L1630
>>>
>>> read32:
>>> https://elixir.bootlin.com/linux/v6.6-rc1/source/drivers/accel/habanalabs/common/debugfs.c#L844
>>>
>>> I liked the ioctl approach so much because it requires a single system
>>> call instead of 2 and the implementation is much cleaner.
>>>
>>> Ofir.
>>>
>>>
>>
>>
>

-- 
Jani Nikula, Intel