Re: [PATCH] drm/xe: Remove unused "mmio_ext" code

[Maciej Patelczyk <maciej.patelczyk@intel.com>]

On 8.01.2025 04:39, Lucas De Marchi wrote:

> On Mon, Jan 06, 2025 at 03:43:13PM -0800, Matt Roper wrote:
>> The "mmio_ext" and 'REG_EXT" code is currently unused on any existing
>> platform.  Going forward, this also isn't the design we want to use for
>> any future platforms/features either, so we should just go ahead and
>> remove the dead code to avoid confusion.
>>
>> mmio_ext was originally added in an attempt to hack around the early
>> (mis)design of the Xe driver, which used xe_gt as the target for all
>> register MMIO access, even those completely unrelated to the GT subunit
>> of the hardware.  With the introduction of commit 34953ee349dd ("drm/xe:
>> Create dedicated xe_mmio structure") and its follow-up patches, that
>> misdesign has been corrected and access to register MMIO regions
>> specific to hardware units is now done through xe_mmio structures which
>> encapsulate an iomap, region size, and some other metadata.
>>
>> Although all of the registers used by the driver today happen to fall
>> within one specific PCI BAR region, and thus re-use a single device-wide
>> iomap, there's no requirement that this stay true for future platforms
>> or features.  I.e., if a future platform adds a new 'foo' hardware unit
>> that exists at a different area in the BAR, or even in a completely
>> different BAR, then that would be handled by doing a separate iomap of
>> that unit's register region and wrapping it in its own 'struct xe_mmio
>> foo_regs' structure.  The pointer to the new 'foo_regs' could be placed
>> within the xe_device, xe_tile, xe_gt, etc., according to where the new
>> hardware unit falls within the current hardware hierarchy.
>>
>> This effectively reverts the following commits, although parts of these
>> commits had already vanished or changed with the earlier xe_mmio
>> refactor work:
>>
>> - commit 399a13323f0d ("drm/xe: add 28-bit address support in struct
>>   xe_reg")
>> - commit fdef72e02e20 ("drm/xe: add a flag to bypass multi-tile config
>>   from MTCFG reg")
>> - commit 866b2b176434 ("drm/xe: add MMIO extension support flags")
>> - commit ef29b390c734 ("drm/xe: map MMIO BAR according to the num of
>>   tiles in device desc")
>> - commit a4e2f3a299ea ("drm/xe: refactor xe_mmio_probe_tiles to support
>>   MMIO extension")
>>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Koby Elbaz <kelbaz@habana.ai>
>> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>, but let's wait
> a day or 2 to give a chance for others to take another look.
>
> Koby, can you ack this?
>
> thanks
> Lucas De Marchi


Also looked from EU Debugger perspective 
(https://patchwork.freedesktop.org/series/142295/).

Just wanted to double check we don't rely on mmio_ext there. We don't. Good.

Acked-by: Maciej Patelczyk <maciej.patelczyk@intel.com>


Regards,

Maciej