[Intel-xe] [PATCH v2 05/17] drm/xe: Rename instruction field to avoid confusion

Lucas De Marchi lucas.demarchi at intel.com
Sat Apr 22 14:57:48 UTC 2023


On Fri, Apr 21, 2023 at 04:41:41PM -0700, Matt Roper wrote:
>On Fri, Apr 21, 2023 at 03:32:46PM -0700, Lucas De Marchi wrote:
>> There was both BLT_DEPTH_32 and XY_FAST_COLOR_BLT_DEPTH_32 - also add
>> the prefix to the first to make it clear this is about the FAST_**COPY**
>> operation. While at it, remove the GEN9_ prefix.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>
>Looks like i915 (correctly) uses the same definition for a bunch of
>different instructions (XY_SRC_COPY_BLT, XY_COLOR_BLT, XY_FAST_COPY_BLT,
>etc.).  But Xe only uses it for XY_FAST_COPY_BLT, so this rename matches
>the usage.
>
>Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
>
>I do wonder if we should think about changing how we emit GPU
>instructions at some point.  Even when we get the bit shifting correct,
>there's still potential mistakes if you OR the definitions into the
>wrong dword (which has happened at least a few times in i915 history).
>Maybe we could do something like create a C structure per instruction
>(e.g., "struct xy_fast_copy_blt") and fill in its fields to build up
>these batchbuffers.


I agree. When I'm updating these parts I always thing I'm doing the job
of a compiler or something.  It's very error prone.

thanks
Lucas De Marchi


More information about the Intel-xe mailing list