Re: [PATCH v4 1/7] accel/ivpu: Introduce a new DRM driver for Intel VPU

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/14/2022 6:57 AM, Oded Gabbay wrote:
On Thu, Dec 8, 2022 at 1:08 PM Jacek Lawrynowicz
<jacek.lawrynowicz@xxxxxxxxxxxxxxx> wrote:
+
+static inline int ivpu_hw_info_init(struct ivpu_device *vdev)
+{
+       return vdev->hw->ops->info_init(vdev);
+};
+
+static inline int ivpu_hw_power_up(struct ivpu_device *vdev)
+{
+       ivpu_dbg(vdev, PM, "HW power up\n");
+
+       return vdev->hw->ops->power_up(vdev);
+};
+
+static inline int ivpu_hw_boot_fw(struct ivpu_device *vdev)
+{
+       return vdev->hw->ops->boot_fw(vdev);
+};
+
+static inline bool ivpu_hw_is_idle(struct ivpu_device *vdev)
+{
+       return vdev->hw->ops->is_idle(vdev);
+};
+
+static inline int ivpu_hw_power_down(struct ivpu_device *vdev)
+{
+       ivpu_dbg(vdev, PM, "HW power down\n");
+
+       return vdev->hw->ops->power_down(vdev);
+};
+
+static inline void ivpu_hw_wdt_disable(struct ivpu_device *vdev)
+{
+       vdev->hw->ops->wdt_disable(vdev);
+};
+
+/* Register indirect accesses */
+static inline u32 ivpu_hw_reg_pll_freq_get(struct ivpu_device *vdev)
+{
+       return vdev->hw->ops->reg_pll_freq_get(vdev);
+};
+
+static inline u32 ivpu_hw_reg_telemetry_offset_get(struct ivpu_device *vdev)
+{
+       return vdev->hw->ops->reg_telemetry_offset_get(vdev);
+};
+
+static inline u32 ivpu_hw_reg_telemetry_size_get(struct ivpu_device *vdev)
+{
+       return vdev->hw->ops->reg_telemetry_size_get(vdev);
+};
+
+static inline u32 ivpu_hw_reg_telemetry_enable_get(struct ivpu_device *vdev)
+{
+       return vdev->hw->ops->reg_telemetry_enable_get(vdev);
+};
+
+static inline void ivpu_hw_reg_db_set(struct ivpu_device *vdev, u32 db_id)
+{
+       vdev->hw->ops->reg_db_set(vdev, db_id);
+};
+
+static inline u32 ivpu_hw_reg_ipc_rx_addr_get(struct ivpu_device *vdev)
+{
+       return vdev->hw->ops->reg_ipc_rx_addr_get(vdev);
+};
+
+static inline u32 ivpu_hw_reg_ipc_rx_count_get(struct ivpu_device *vdev)
+{
+       return vdev->hw->ops->reg_ipc_rx_count_get(vdev);
+};
+
+static inline void ivpu_hw_reg_ipc_tx_set(struct ivpu_device *vdev, u32 vpu_addr)
+{
+       vdev->hw->ops->reg_ipc_tx_set(vdev, vpu_addr);
+};
+
+static inline void ivpu_hw_irq_clear(struct ivpu_device *vdev)
+{
+       vdev->hw->ops->irq_clear(vdev);
+};
+
+static inline void ivpu_hw_irq_enable(struct ivpu_device *vdev)
+{
+       vdev->hw->ops->irq_enable(vdev);
+};
+
+static inline void ivpu_hw_irq_disable(struct ivpu_device *vdev)
+{
+       vdev->hw->ops->irq_disable(vdev);
+};
Why hide all the calls to the hw ops functions inside inline functions ?
I mean, it just makes it one step longer to read the code...
Why not directly call vdev->hw->ops->operation ?

diff --git a/include/uapi/drm/ivpu_drm.h b/include/uapi/drm/ivpu_drm.h
new file mode 100644
index 000000000000..922cbf30ce34
--- /dev/null
+++ b/include/uapi/drm/ivpu_drm.h
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
+/*
+ * Copyright (C) 2020-2022 Intel Corporation
+ */
+
+#ifndef __UAPI_IVPU_DRM_H__
+#define __UAPI_IVPU_DRM_H__
+
+#include "drm.h"
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+#define DRM_IVPU_DRIVER_MAJOR 1
+#define DRM_IVPU_DRIVER_MINOR 0
I would prefer not to define versions for the driver. Don't get in
this trap of trying to keep a version
number updated over time.  It breaks down the moment you get the code
merged into the kernel tree as the driver is what is in the kernel, not
separate from it.

Also think of the stable kernels, you will backport fixes to those for
the driver as part of the development process.  So what "version" are
they now?  They are some mis-match of the old one with new fixes, and as
such, the version number now means nothing.

Just stick to the version number of the kernel itself, that's the only
sane way.

This seems to assume that the user is running whole kernels and not a backport of the driver (similar to the backports project that is popular with net).

Lets assume this gets merged into 6.3. End user is on say Ubuntu 20.04 which GA'd with a 5.4 kernel and is maintaining that. Intel here backports the 6.3 driver to 5.4 to support this end user and tries to do the "right thing" by using the upstream code instead of some downstream fork. Now the kernel version is meaningless because 5.4 never had this driver. Of course if the user reports an issue to Intel, it would be handy to know exactly what version of the driver the user has.

The src version hash that modinfo provides is meaningless because it changes based on the entirety of the build, which includes things like the compiler used.

If there is an alternative solution for this, I'd love to hear it.



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux