[Intel-xe] [PATCH 6/6] drm/xe: Select graphics/media descriptors from GMD_ID

[Lucas De Marchi]

On Wed, Apr 05, 2023 at 04:30:47PM -0700, Matt Roper wrote:
>On Wed, Apr 05, 2023 at 03:36:02PM -0700, Lucas De Marchi wrote:
>> On Mon, Apr 03, 2023 at 01:17:02PM -0700, Matt Roper wrote:
>> > Hook up dummy graphics_desc and media_desc structures on platforms that
>> > use GMD_ID.  When such a platform is probed, the IP version will be read
>> > from the hardware's GMD_ID registers and that version number will be
>> > used to select a graphics/media descriptor with the appropriate settings
>> > for the detected IP.
>> >
>> > If a GMD_ID platform reports a graphics version the driver does not
>> > recognize and support, device probe will be aborted.  If an unrecognized
>> > media version is reported the device probe will leave media
>> > uninitialized and continue.
>> >
>> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>> > ---
>> > drivers/gpu/drm/xe/regs/xe_gt_regs.h |   6 +
>> > drivers/gpu/drm/xe/xe_pci.c          | 161 +++++++++++++++++++++------
>> > 2 files changed, 136 insertions(+), 31 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> > index f45251df5715..2d265dbb7651 100644
>> > --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> > +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> > @@ -22,6 +22,12 @@
>> > #define FORCEWAKE_ACK_MEDIA_VDBOX_GEN11(n)	_MMIO(0xd50 + (n) * 4)
>> > #define FORCEWAKE_ACK_MEDIA_VEBOX_GEN11(n)	_MMIO(0xd70 + (n) * 4)
>> > #define FORCEWAKE_ACK_RENDER_GEN9		_MMIO(0xd84)
>> > +
>> > +#define GMD_ID					_MMIO(0xd8c)
>> > +#define   GMD_ID_ARCH_MASK			REG_GENMASK(31, 22)
>> > +#define   GMD_ID_RELEASE_MASK			REG_GENMASK(21, 14)
>> > +#define   GMD_ID_STEP				REG_GENMASK(5, 0)
>> > +
>> > #define FORCEWAKE_ACK_GT_MTL			_MMIO(0xdfc)
>> >
>> > #define GEN9_LNCFCMOCS(i)			_MMIO(0xb020 + (i) * 4)	/* L3 Cache Control */
>> > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>> > index d634e781858a..62685b14257b 100644
>> > --- a/drivers/gpu/drm/xe/xe_pci.c
>> > +++ b/drivers/gpu/drm/xe/xe_pci.c
>> > @@ -15,6 +15,7 @@
>> > #include <drm/xe_pciids.h>
>> >
>> > #include "regs/xe_regs.h"
>> > +#include "regs/xe_gt_regs.h"
>> > #include "xe_device.h"
>> > #include "xe_display.h"
>> > #include "xe_drv.h"
>> > @@ -99,6 +100,14 @@ __diag_ignore_all("-Woverride-init", "Allow field overrides in table");
>> >
>> > #define NOP(x)	x
>> >
>> > +static const struct xe_graphics_desc graphics_gmdid = {
>> > +	/*
>> > +	 * Unset graphics version implies GMD_ID support; driver will read
>> > +	 * the IP version from hardware and then select a more appropriate
>> > +	 * graphics descriptor.
>> > +	 */
>> > +};
>> > +
>> > static const struct xe_graphics_desc graphics_xelp = {
>> > 	.ver = 12,
>> > 	.rel = 0,
>> > @@ -164,9 +173,6 @@ static const struct xe_graphics_desc graphics_xehpc = {
>> > };
>> >
>> > static const struct xe_graphics_desc graphics_xelpg = {
>> > -	.ver = 12,
>> > -	.rel = 70,
>> > -
>>
>> can we change the logic so we leave the version here?  While reading the
>> rest of the driver it will be horrible not to have in the source code
>> the reference about the IP version.
>
>The 'switch' in handle_gmdid() provides the number(s) that we consider
>to be a match for any of these structures going forward; is that not
>enough?  Note that putting something like 12.70 here gives an incomplete

maybe... It seems we will need to hop a few indirections to get this
answered.

>picture since 12.71 is is also a match (and there could be more numbers
>we associate with it in the future too).
>
>If handle_gmdid()'s numbers aren't clear enough, I could just put a
>comment here listing what numbers we expect to match to this structure?

let's keep it without comments and see if it's too hard. We can add the
comment later if we need it.

thanks
Lucas De Marchi