Re: [RFC 0/7] Promote GuC ABI headers to shared location

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Jun 11, 2024 at 11:45:17PM +0200, Michal Wajdeczko wrote:
> 
> 
> On 11.06.2024 22:32, John Harrison wrote:
> > On 6/11/2024 07:30, Michal Wajdeczko wrote:
> >> There are many GuC ABI definitions named in the same way by the i915
> >> and Xe drivers, preventing proper generation of the documentation.
> >>
> >> Promote GuC ABI definitions to shared location that can be used by
> >> both drivers and can be included in documentation.
> > I still very strongly feel that this is the wrong place for such
> > documentation. We do not document any of the hardware registers in the
> > driver, nor the MI_ instructions, etc. Why is this any different? The
> > GuC ABI is not under the control of the Linux kernel driver, either i915
> > or Xe. It is effectively a hardware interface no different to any other
> > hardware interface. It is already fully documented by the owners of that
> > interface. Rather than just copying random chunks of that documentation
> > into the kernel driver, we should just be publishing the official
> > document itself. Same as we do for the rest of the hardware.
> 
> so go publish this official document
> 
> in the meantime IMO it is useful to show, with really little effort, on
> what grounds the communication between i915/Xe and GuC works, so
> everyone, not just selected engineers, can understand and review our
> implementation and check its correctness
> 
> furthermore, if you don't like any hw documentation then we should
> revisit what is already in gpu/i915 section and also reconsider all our
> efforts to document all non-static driver functions, as those functions
> are still internal to the driver, not to be used outside

I second that. This is useful documentation internal to our driver.
Specially with a fw which has an ABI that demands version compatibility
with the driver like this.

> 
> > 
> > John.
> > 
> > 
> >>
> >> Cc: Jani Nikula <jani.nikula@intel.com>
> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >> Cc: Matthew Brost <matthew.brost@intel.com>
> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >> Cc: John Harrison <John.C.Harrison@Intel.com>
> >>
> >> Michal Wajdeczko (7):
> >>    drm/xe/guc: Promote GuC ABI headers to shared location
> >>    Documentation/gpu: Separate GuC ABI section
> >>    Documentation/gpu: Switch to shared GuC ABI definitions
> >>    drm/intel/guc: Update CTB communication ABI
> >>    drm/intel/guc: Add new KLV definitions
> >>    drm/i915: Use shared GuC ABI definitions
> >>    drm/xe: Promote SR-IOV GuC ABI definitions to shared location
> >>
> >>   Documentation/gpu/drivers.rst                 |   1 +
> >>   Documentation/gpu/guc.rst                     |  23 ++
> >>   Documentation/gpu/i915.rst                    |   9 -
> >>   drivers/gpu/drm/i915/Makefile                 |   5 +
> >>   .../gt/uc/abi/guc_communication_ctb_abi.h     | 170 -----------
> >>   .../gt/uc/abi/guc_communication_mmio_abi.h    |  49 ----
> >>   drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h | 112 --------
> >>   .../gpu/drm/i915/gt/uc/abi/guc_messages_abi.h | 264 ------------------
> >>   .../guc}/abi/guc_actions_sriov_abi.h          |   0
> >>   .../guc}/abi/guc_communication_ctb_abi.h      |   2 +
> >>   .../guc}/abi/guc_communication_mmio_abi.h     |   0
> >>   .../drm/{xe => intel/guc}/abi/guc_klvs_abi.h  |  18 +-
> >>   .../{xe => intel/guc}/abi/guc_messages_abi.h  |   0
> >>   .../guc}/abi/guc_relay_actions_abi.h          |   0
> >>   .../guc}/abi/guc_relay_communication_abi.h    |   0
> >>   drivers/gpu/drm/xe/Makefile                   |   5 +
> >>   16 files changed, 49 insertions(+), 609 deletions(-)
> >>   create mode 100644 Documentation/gpu/guc.rst
> >>   delete mode 100644
> >> drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
> >>   delete mode 100644
> >> drivers/gpu/drm/i915/gt/uc/abi/guc_communication_mmio_abi.h
> >>   delete mode 100644 drivers/gpu/drm/i915/gt/uc/abi/guc_klvs_abi.h
> >>   delete mode 100644 drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
> >>   rename drivers/gpu/drm/{xe => intel/guc}/abi/guc_actions_sriov_abi.h
> >> (100%)
> >>   rename drivers/gpu/drm/{xe =>
> >> intel/guc}/abi/guc_communication_ctb_abi.h (98%)
> >>   rename drivers/gpu/drm/{xe =>
> >> intel/guc}/abi/guc_communication_mmio_abi.h (100%)
> >>   rename drivers/gpu/drm/{xe => intel/guc}/abi/guc_klvs_abi.h (97%)
> >>   rename drivers/gpu/drm/{xe => intel/guc}/abi/guc_messages_abi.h (100%)
> >>   rename drivers/gpu/drm/{xe => intel/guc}/abi/guc_relay_actions_abi.h
> >> (100%)
> >>   rename drivers/gpu/drm/{xe =>
> >> intel/guc}/abi/guc_relay_communication_abi.h (100%)
> >>
> >