[PATCH v6 1/2] drm/xe: add a new debugfs file - mmio

[Koby Elbaz]

On 15/02/2024 1:21, Ville Syrjälä wrote:
> On Tue, Feb 06, 2024 at 05:16:23PM +0000, Koby Elbaz wrote:
>> On 31/01/2024 22:14, Summers, Stuart wrote:
>>> On Wed, 2024-01-31 at 21:59 +0200, Koby Elbaz wrote:
>>>> Now that the former mmio handlers are fully removed, this
>>>> debugfs file will be used for mmio access (read/write)
>>>> through the debugfs ioctl file operation.
>>> Sorry I don't know the history here of v0-v5, but one question, with
>>> igt-gpu-tools/tools/intel_reg we can read and write MMIO registers
>>> without needing to have the driver loaded at all (i.e. straight through
>>> the BARs). Why do we want to also put this capability in the xe debugfs
>>> then?
>>>
>>> Thanks,
>>> Stuart
>> Hi Stuart,
>> Sorry for the late response,
>> To some extent we can use it, however, it does not satisfy our
>> development needs.
>> Moreover, this change was recently agreed upon throughout the removal of
>> the former mmio-ioctl code:
>> https://patchwork.freedesktop.org/series/123403/
> If someone intends to hook this up into intel_reg then please
> make sure it's _not_ the default path. When debugging I want my
> register accesses to go directly to hardware and not be
> potentially mucked up by a busted driver.

There's no intention whatsoever to do so. As a matter of fact, the
debugfs mmio ioctl is parallel/independent interface.

Thanks,
Koby
>>
>>>> Signed-off-by: Koby Elbaz <kelbaz at habana.ai>
>>>> ---
>>>>    drivers/gpu/drm/xe/xe_debugfs.c | 12 ++++++++++++
>>>>    1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c
>>>> b/drivers/gpu/drm/xe/xe_debugfs.c
>>>> index 01db5b27bec5..21b2257aa3a8 100644
>>>> --- a/drivers/gpu/drm/xe/xe_debugfs.c
>>>> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
>>>> @@ -98,12 +98,22 @@ static int forcewake_release(struct inode *inode,
>>>> struct file *file)
>>>>           return 0;
>>>>    }
>>>>
>>>> +static long xe_debugfs_mmio_ioctl(struct file *f, unsigned int cmd,
>>>> unsigned long arg)
>>>> +{
>>>> +       return 0;
>>>> +}
>>>> +
>>>>    static const struct file_operations forcewake_all_fops = {
>>>>           .owner = THIS_MODULE,
>>>>           .open = forcewake_open,
>>>>           .release = forcewake_release,
>>>>    };
>>>>
>>>> +static const struct file_operations mmio_fops = {
>>>> +       .owner = THIS_MODULE,
>>>> +       .unlocked_ioctl = xe_debugfs_mmio_ioctl,
>>>> +};
>>>> +
>>>>    void xe_debugfs_register(struct xe_device *xe)
>>>>    {
>>>>           struct ttm_device *bdev = &xe->ttm;
>>>> @@ -142,6 +152,8 @@ void xe_debugfs_register(struct xe_device *xe)
>>>>           for_each_gt(gt, xe, id)
>>>>                   xe_gt_debugfs_register(gt);
>>>>
>>>> +       debugfs_create_file("mmio", 0644, root, xe, &mmio_fops);
>>>> +
>>>>    #ifdef CONFIG_FAULT_INJECTION
>>>>           fault_create_debugfs_attr("fail_gt_reset", root,
>>>> &gt_reset_failure);
>>>>    #endif
>>