[PATCH 1/6] drm/xe/guc: Add VF2GUC_MATCH_VERSION to ABI

[Piotr Piórkowski]

Piotr Piórkowski <piotr.piorkowski at intel.com> wrote on wto [2024-maj-14 14:47:03 +0200]:
> Michal Wajdeczko <michal.wajdeczko at intel.com> wrote on nie [2024-maj-12 17:49:10 +0200]:
> > In upcoming patches we will add a version negotiation between
> > the VF driver and the GuC firmware. Add necessary definitions
> > to our GuC firmware ABI header.
> > 
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > ---
> >  .../gpu/drm/xe/abi/guc_actions_sriov_abi.h    | 67 +++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/abi/guc_actions_sriov_abi.h b/drivers/gpu/drm/xe/abi/guc_actions_sriov_abi.h
> > index c1ad09b36453..49ed5fbd4ced 100644
> > --- a/drivers/gpu/drm/xe/abi/guc_actions_sriov_abi.h
> > +++ b/drivers/gpu/drm/xe/abi/guc_actions_sriov_abi.h
> > @@ -213,6 +213,73 @@
> >  #define   GUC_PF_NOTIFY_VF_PAUSE_DONE			3u
> >  #define   GUC_PF_NOTIFY_VF_FIXUP_DONE			4u
> >  
> > +/**
> > + * DOC: VF2GUC_MATCH_VERSION
> > + *
> > + * This action is used to match VF interface version used by VF and GuC.
> > + *
> > + * This action must be sent over MMIO.

One more thing:
In the next patch you use the phrase:

+ * This message must be sent as `MMIO HXG Message`_.

Maybe use it here too.

Thanks
Piotr

> > + *
> > + *  +---+-------+--------------------------------------------------------------+
> > + *  |   | Bits  | Description                                                  |
> > + *  +===+=======+==============================================================+
> > + *  | 0 |    31 | ORIGIN = GUC_HXG_ORIGIN_HOST_                                |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   | 30:28 | TYPE = GUC_HXG_TYPE_REQUEST_                                 |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   | 27:16 | DATA0 = MBZ                                                  |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   |  15:0 | ACTION = _`GUC_ACTION_VF2GUC_MATCH_VERSION` = 0x5500         |
> > + *  +---+-------+--------------------------------------------------------------+
> > + *  | 1 | 31:24 | **BRANCH** - branch ID of the VF interface                   |
> > + *  |   |       | (use BRANCH_ANY to request latest version supported by GuC)  |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   | 23:16 | **MAJOR** - major version of the VF interface                |
> > + *  |   |       | (use MAJOR_ANY to request latest version supported by GuC)   |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   |  15:8 | **MINOR** - minor version of the VF interface                |
> > + *  |   |       | (use MINOR_ANY to request latest version supported by GuC)   |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   |   7:0 | **MBZ**                                                      |
> > + *  +---+-------+--------------------------------------------------------------+
> > + *
> > + *  +---+-------+--------------------------------------------------------------+
> > + *  |   | Bits  | Description                                                  |
> > + *  +===+=======+==============================================================+
> > + *  | 0 |    31 | ORIGIN = GUC_HXG_ORIGIN_GUC_                                 |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   | 30:28 | TYPE = GUC_HXG_TYPE_RESPONSE_SUCCESS_                        |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   |  27:0 | DATA0 = MBZ                                                  |
> > + *  +---+-------+--------------------------------------------------------------+
> > + *  | 1 | 31:24 | **BRANCH** - branch ID of the VF interface                   |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   | 23:16 | **MAJOR** - major version of the VF interface                |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   |  15:8 | **MINOR** - minor version of the VF interface                |
> > + *  |   +-------+--------------------------------------------------------------+
> > + *  |   |   7:0 | **PATCH** - patch version of the VF interface                |
> > + *  +---+-------+--------------------------------------------------------------+
> > + */
> > +#define GUC_ACTION_VF2GUC_MATCH_VERSION			0x5500u
> > +
> > +#define VF2GUC_MATCH_VERSION_REQUEST_MSG_LEN		(GUC_HXG_REQUEST_MSG_MIN_LEN + 1u)
> > +#define VF2GUC_MATCH_VERSION_REQUEST_MSG_0_MBZ		GUC_HXG_REQUEST_MSG_0_DATA0
> > +#define VF2GUC_MATCH_VERSION_REQUEST_MSG_1_BRANCH	(0xffu << 24)
> > +#define   GUC_VERSION_BRANCH_ANY			0
> > +#define VF2GUC_MATCH_VERSION_REQUEST_MSG_1_MAJOR	(0xffu << 16)
> > +#define   GUC_VERSION_MAJOR_ANY				0
> > +#define VF2GUC_MATCH_VERSION_REQUEST_MSG_1_MINOR	(0xffu << 8)
> > +#define   GUC_VERSION_MINOR_ANY				0
> > +#define VF2GUC_MATCH_VERSION_REQUEST_MSG_1_MBZ		(0xffu << 0)
> 
> NIT: It seems to me that GENMASK would be more appropriate here
> 
> > +
> > +#define VF2GUC_MATCH_VERSION_RESPONSE_MSG_LEN		(GUC_HXG_RESPONSE_MSG_MIN_LEN + 1u)
> > +#define VF2GUC_MATCH_VERSION_RESPONSE_MSG_0_MBZ		GUC_HXG_RESPONSE_MSG_0_DATA0
> > +#define VF2GUC_MATCH_VERSION_RESPONSE_MSG_1_BRANCH	(0xffu << 24)
> > +#define VF2GUC_MATCH_VERSION_RESPONSE_MSG_1_MAJOR	(0xffu << 16)
> > +#define VF2GUC_MATCH_VERSION_RESPONSE_MSG_1_MINOR	(0xffu << 8)
> > +#define VF2GUC_MATCH_VERSION_RESPONSE_MSG_1_PATCH	(0xffu << 0)
> > +
> >  /**
> >   * DOC: PF2GUC_UPDATE_VGT_POLICY
> >   *
> 
> LGTM:
> Reviewed-by: Piotr Piórkowski <piotr.piorkowski at intel.com>
> 
> > -- 
> > 2.43.0
> > 
> 
> -- 

--