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@xxxxxxxxx>
> >> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> >> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> >> Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> >> Cc: John Harrison <John.C.Harrison@xxxxxxxxx>
> >>
> >> 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%)
> >>
> >
