RE: [PATCH] drm/amdgpu: remove asymmetrical irq disabling in jpeg 4.0.5 suspend

2024-02-01 Thread Gopalakrishnan, Veerabadhran (Veera)
[AMD Official Use Only - General]

Looks good to me.

Reviewed-by: Veerabadhran Gopalakrishnan 

Regards,
Veera

-Original Message-
From: Jamadar, Saleemkhan 
Sent: Thursday, February 1, 2024 5:29 PM
To: Ma, Li ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Gopalakrishnan, 
Veerabadhran (Veera) ; Zhang, Yifan 

Subject: RE: [PATCH] drm/amdgpu: remove asymmetrical irq disabling in jpeg 
4.0.5 suspend

[AMD Official Use Only - General]

Acked-By: Saleemkhan Jamadar 

-Original Message-
From: Ma, Li 
Sent: Thursday, February 1, 2024 5:25 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Jamadar, Saleemkhan 
; Gopalakrishnan, Veerabadhran (Veera) 
; Zhang, Yifan ; Ma, 
Li 
Subject: [PATCH] drm/amdgpu: remove asymmetrical irq disabling in jpeg 4.0.5 
suspend

A supplement to commit: 45fa6f32276f7ce571227f8368cf17378b804aa1
There is an irq warning of jpeg during resume in s2idle process. No irq enabled 
in jpeg 4.0.5 resume.

Signed-off-by: Li Ma 
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c   |  9 -
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c | 10 --
 2 files changed, 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
index bc38b90f8cf8..88ea58d5c4ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
@@ -674,14 +674,6 @@ static int jpeg_v4_0_set_powergating_state(void *handle,
return ret;
 }

-static int jpeg_v4_0_set_interrupt_state(struct amdgpu_device *adev,
-   struct amdgpu_irq_src *source,
-   unsigned type,
-   enum amdgpu_interrupt_state state)
-{
-   return 0;
-}
-
 static int jpeg_v4_0_set_ras_interrupt_state(struct amdgpu_device *adev,
struct amdgpu_irq_src *source,
unsigned int type, @@ -765,7 +757,6 @@ 
static void jpeg_v4_0_set_dec_ring_funcs(struct amdgpu_device *adev)  }

 static const struct amdgpu_irq_src_funcs jpeg_v4_0_irq_funcs = {
-   .set = jpeg_v4_0_set_interrupt_state,
.process = jpeg_v4_0_process_interrupt,  };

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c 
b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
index 6ede85b28cc8..78b74daf4eeb 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
@@ -181,7 +181,6 @@ static int jpeg_v4_0_5_hw_fini(void *handle)
RREG32_SOC15(JPEG, 0, regUVD_JRBC_STATUS))
jpeg_v4_0_5_set_powergating_state(adev, 
AMD_PG_STATE_GATE);
}
-   amdgpu_irq_put(adev, >jpeg.inst->irq, 0);

return 0;
 }
@@ -516,14 +515,6 @@ static int jpeg_v4_0_5_set_powergating_state(void *handle,
return ret;
 }

-static int jpeg_v4_0_5_set_interrupt_state(struct amdgpu_device *adev,
-   struct amdgpu_irq_src *source,
-   unsigned type,
-   enum amdgpu_interrupt_state state)
-{
-   return 0;
-}
-
 static int jpeg_v4_0_5_process_interrupt(struct amdgpu_device *adev,
  struct amdgpu_irq_src *source,
  struct amdgpu_iv_entry *entry) @@ -603,7 
+594,6 @@ static void jpeg_v4_0_5_set_dec_ring_funcs(struct amdgpu_device 
*adev)  }

 static const struct amdgpu_irq_src_funcs jpeg_v4_0_5_irq_funcs = {
-   .set = jpeg_v4_0_5_set_interrupt_state,
.process = jpeg_v4_0_5_process_interrupt,  };

--
2.25.1




RE: [PATCH] drm/amdgpu: remove asymmetrical irq disabling in jpeg 4.0.5 suspend

2024-02-01 Thread Zhang, Yifan
[AMD Official Use Only - General]

Reviewed-by: Yifan Zhang 

Best Regards,
Yifan

-Original Message-
From: Ma, Li 
Sent: Thursday, February 1, 2024 7:55 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Jamadar, Saleemkhan 
; Gopalakrishnan, Veerabadhran (Veera) 
; Zhang, Yifan ; Ma, 
Li 
Subject: [PATCH] drm/amdgpu: remove asymmetrical irq disabling in jpeg 4.0.5 
suspend

A supplement to commit: 45fa6f32276f7ce571227f8368cf17378b804aa1
There is an irq warning of jpeg during resume in s2idle process. No irq enabled 
in jpeg 4.0.5 resume.

Signed-off-by: Li Ma 
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c   |  9 -
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c | 10 --
 2 files changed, 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
index bc38b90f8cf8..88ea58d5c4ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
@@ -674,14 +674,6 @@ static int jpeg_v4_0_set_powergating_state(void *handle,
return ret;
 }

-static int jpeg_v4_0_set_interrupt_state(struct amdgpu_device *adev,
-   struct amdgpu_irq_src *source,
-   unsigned type,
-   enum amdgpu_interrupt_state state)
-{
-   return 0;
-}
-
 static int jpeg_v4_0_set_ras_interrupt_state(struct amdgpu_device *adev,
struct amdgpu_irq_src *source,
unsigned int type,
@@ -765,7 +757,6 @@ static void jpeg_v4_0_set_dec_ring_funcs(struct 
amdgpu_device *adev)  }

 static const struct amdgpu_irq_src_funcs jpeg_v4_0_irq_funcs = {
-   .set = jpeg_v4_0_set_interrupt_state,
.process = jpeg_v4_0_process_interrupt,  };

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c 
b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
index 6ede85b28cc8..78b74daf4eeb 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
@@ -181,7 +181,6 @@ static int jpeg_v4_0_5_hw_fini(void *handle)
RREG32_SOC15(JPEG, 0, regUVD_JRBC_STATUS))
jpeg_v4_0_5_set_powergating_state(adev, 
AMD_PG_STATE_GATE);
}
-   amdgpu_irq_put(adev, >jpeg.inst->irq, 0);

return 0;
 }
@@ -516,14 +515,6 @@ static int jpeg_v4_0_5_set_powergating_state(void *handle,
return ret;
 }

-static int jpeg_v4_0_5_set_interrupt_state(struct amdgpu_device *adev,
-   struct amdgpu_irq_src *source,
-   unsigned type,
-   enum amdgpu_interrupt_state state)
-{
-   return 0;
-}
-
 static int jpeg_v4_0_5_process_interrupt(struct amdgpu_device *adev,
  struct amdgpu_irq_src *source,
  struct amdgpu_iv_entry *entry) @@ -603,7 
+594,6 @@ static void jpeg_v4_0_5_set_dec_ring_funcs(struct amdgpu_device 
*adev)  }

 static const struct amdgpu_irq_src_funcs jpeg_v4_0_5_irq_funcs = {
-   .set = jpeg_v4_0_5_set_interrupt_state,
.process = jpeg_v4_0_5_process_interrupt,  };

--
2.25.1



[PATCH v3 0/5] Add support for fetching EDID from ACPI _DDC

2024-02-01 Thread Mario Limonciello
Some laptops ship an EDID in the BIOS encoded in the _DDC method that
differs than the EDID directly on the laptop panel for $REASONS.

This is the EDID that is used by the AMD Windows driver, and so sometimes
different results are found in different operating systems.

This series adds a new DRM helper that will use acpi_video to fetch the
EDID.

On amdgpu when an eDP panel is found the BIOS
is checked first for an EDID and that used as a preference if found.

On nouveau it replaces the previous local function doing a similar role.

This does *not* use struct drm_edid as this will require more involved
amdgpu display driver work that will come separately as part of follow-ups
to: https://lore.kernel.org/amd-gfx/20240126163429.56714-1-m...@igalia.com/

v2-v3:
 * Clean up some of the 'select ACPI_VIDEO' kconfig spaghetti reported by LKP
 * Drop the 'select ACPI_VIDEO' from the DRM drivers

Mario Limonciello (5):
  ACPI: video: Handle fetching EDID that is longer than 256 bytes
  drm: Add drm_get_acpi_edid() helper
  drm/amd: Fetch the EDID from _DDC if available for eDP
  drm/nouveau: Use drm_get_acpi_edid() helper
  drm: Drop unneeded selects in DRM drivers

 drivers/acpi/acpi_video.c | 25 +++
 drivers/gpu/drm/Kconfig   |  5 ++
 drivers/gpu/drm/amd/amdgpu/Kconfig|  7 --
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  1 +
 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c|  4 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  8 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 ++-
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  9 ++-
 drivers/gpu/drm/drm_edid.c| 73 +++
 drivers/gpu/drm/gma500/Kconfig|  6 --
 drivers/gpu/drm/i915/Kconfig  |  7 --
 drivers/gpu/drm/nouveau/nouveau_acpi.c| 27 ---
 drivers/gpu/drm/nouveau/nouveau_acpi.h|  2 -
 drivers/gpu/drm/nouveau/nouveau_connector.c   |  2 +-
 drivers/gpu/drm/radeon/Kconfig|  7 --
 drivers/gpu/drm/xe/Kconfig|  6 --
 include/drm/drm_edid.h|  1 +
 17 files changed, 116 insertions(+), 84 deletions(-)

-- 
2.34.1



[PATCH v3 2/5] drm: Add drm_get_acpi_edid() helper

2024-02-01 Thread Mario Limonciello
Some manufacturers have intentionally put an EDID that differs from
the EDID on the internal panel on laptops.  Drivers can call this
helper to attempt to fetch the EDID from the BIOS's ACPI _DDC method.

Signed-off-by: Mario Limonciello 
---
v1->v2:
 * Split code from previous amdgpu specific helper to generic drm helper.
v2->v3:
 * Add an extra select to fix a variety of randconfig errors found from
   LKP robot.
---
 drivers/gpu/drm/Kconfig|  5 +++
 drivers/gpu/drm/drm_edid.c | 73 ++
 include/drm/drm_edid.h |  1 +
 3 files changed, 79 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2520db0b776e..14df907c96c8 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -21,6 +21,11 @@ menuconfig DRM
select KCMP
select VIDEO_CMDLINE
select VIDEO_NOMODESET
+   select ACPI_VIDEO if ACPI
+   select BACKLIGHT_CLASS_DEVICE if ACPI
+   select INPUT if ACPI
+   select X86_PLATFORM_DEVICES if ACPI && X86
+   select ACPI_WMI if ACPI && X86
help
  Kernel-level support for the Direct Rendering Infrastructure (DRI)
  introduced in XFree86 4.0. If you say Y here, you need to select
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 69c68804023f..1fbbeaa664b2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -28,6 +28,7 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -2188,6 +2189,47 @@ drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int 
block, size_t len)
return ret == xfers ? 0 : -1;
 }
 
+/**
+ * drm_do_probe_acpi_edid() - get EDID information via ACPI _DDC
+ * @data: struct drm_device
+ * @buf: EDID data buffer to be filled
+ * @block: 128 byte EDID block to start fetching from
+ * @len: EDID data buffer length to fetch
+ *
+ * Try to fetch EDID information by calling acpi_video_get_edid() function.
+ *
+ * Return: 0 on success or error code on failure.
+ */
+static int
+drm_do_probe_acpi_edid(void *data, u8 *buf, unsigned int block, size_t len)
+{
+   struct drm_device *ddev = data;
+   struct acpi_device *acpidev = ACPI_COMPANION(ddev->dev);
+   unsigned char start = block * EDID_LENGTH;
+   void *edid;
+   int r;
+
+   if (!acpidev)
+   return -ENODEV;
+
+   /* fetch the entire edid from BIOS */
+   r = acpi_video_get_edid(acpidev, ACPI_VIDEO_DISPLAY_LCD, -1, );
+   if (r < 0) {
+   DRM_DEBUG_KMS("Failed to get EDID from ACPI: %d\n", r);
+   return -EINVAL;
+   }
+   if (len > r || start > r || start + len > r) {
+   r = EINVAL;
+   goto cleanup;
+   }
+
+   memcpy(buf, edid + start, len);
+   r = 0;
+cleanup:
+   kfree(edid);
+   return r;
+}
+
 static void connector_bad_edid(struct drm_connector *connector,
   const struct edid *edid, int num_blocks)
 {
@@ -2643,6 +2685,37 @@ struct edid *drm_get_edid(struct drm_connector 
*connector,
 }
 EXPORT_SYMBOL(drm_get_edid);
 
+/**
+ * drm_get_acpi_edid - get EDID data, if available
+ * @connector: connector we're probing
+ *
+ * Use the BIOS to attempt to grab EDID data if possible.  If found,
+ * attach it to the connector.
+ *
+ * Return: Pointer to valid EDID or NULL if we couldn't find any.
+ */
+struct edid *drm_get_acpi_edid(struct drm_connector *connector)
+{
+   struct edid *edid = NULL;
+
+   switch (connector->connector_type) {
+   case DRM_MODE_CONNECTOR_LVDS:
+   case DRM_MODE_CONNECTOR_eDP:
+   break;
+   default:
+   return NULL;
+   }
+
+   if (connector->force == DRM_FORCE_OFF)
+   return NULL;
+
+   edid = _drm_do_get_edid(connector, drm_do_probe_acpi_edid, 
connector->dev, NULL);
+
+   drm_connector_update_edid_property(connector, edid);
+   return edid;
+}
+EXPORT_SYMBOL(drm_get_acpi_edid);
+
 /**
  * drm_edid_read_custom - Read EDID data using given EDID block read function
  * @connector: Connector to use
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 518d1b8106c7..60fbdc06badc 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -412,6 +412,7 @@ struct edid *drm_do_get_edid(struct drm_connector 
*connector,
void *data);
 struct edid *drm_get_edid(struct drm_connector *connector,
  struct i2c_adapter *adapter);
+struct edid *drm_get_acpi_edid(struct drm_connector *connector);
 u32 drm_edid_get_panel_id(struct i2c_adapter *adapter);
 struct edid *drm_get_edid_switcheroo(struct drm_connector *connector,
 struct i2c_adapter *adapter);
-- 
2.34.1



[PATCH v3 5/5] drm: Drop unneeded selects in DRM drivers

2024-02-01 Thread Mario Limonciello
All of the selects on ACPI_VIDEO are unnecessary when DRM does the
select for ACPI_VIDEO as it provides a helper for acpi based EDID.

Signed-off-by: Mario Limonciello 
---
v2->v3:
 * new patch
---
 drivers/gpu/drm/amd/amdgpu/Kconfig | 7 ---
 drivers/gpu/drm/gma500/Kconfig | 6 --
 drivers/gpu/drm/i915/Kconfig   | 7 ---
 drivers/gpu/drm/radeon/Kconfig | 7 ---
 drivers/gpu/drm/xe/Kconfig | 6 --
 5 files changed, 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
b/drivers/gpu/drm/amd/amdgpu/Kconfig
index 22d88f8ef527..b2178a5a947c 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -22,13 +22,6 @@ config DRM_AMDGPU
select DRM_BUDDY
select DRM_SUBALLOC_HELPER
select DRM_EXEC
-   # amdgpu depends on ACPI_VIDEO when ACPI is enabled, for select to work
-   # ACPI_VIDEO's dependencies must also be selected.
-   select INPUT if ACPI
-   select ACPI_VIDEO if ACPI
-   # On x86 ACPI_VIDEO also needs ACPI_WMI
-   select X86_PLATFORM_DEVICES if ACPI && X86
-   select ACPI_WMI if ACPI && X86
help
  Choose this option if you have a recent AMD Radeon graphics card.
 
diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
index efb4a2dd2f80..6921ef67b256 100644
--- a/drivers/gpu/drm/gma500/Kconfig
+++ b/drivers/gpu/drm/gma500/Kconfig
@@ -6,12 +6,6 @@ config DRM_GMA500
select FB_IOMEM_HELPERS if DRM_FBDEV_EMULATION
select I2C
select I2C_ALGOBIT
-   # GMA500 depends on ACPI_VIDEO when ACPI is enabled, just like i915
-   select ACPI_VIDEO if ACPI
-   select BACKLIGHT_CLASS_DEVICE if ACPI
-   select INPUT if ACPI
-   select X86_PLATFORM_DEVICES if ACPI
-   select ACPI_WMI if ACPI
help
  Say yes for an experimental 2D KMS framebuffer driver for the
  Intel GMA500 (Poulsbo), Intel GMA600 (Moorestown/Oak Trail) and
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index b5d6e3352071..476da09433bb 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -22,13 +22,6 @@ config DRM_I915
select I2C
select I2C_ALGOBIT
select IRQ_WORK
-   # i915 depends on ACPI_VIDEO when ACPI is enabled
-   # but for select to work, need to select ACPI_VIDEO's dependencies, ick
-   select BACKLIGHT_CLASS_DEVICE if ACPI
-   select INPUT if ACPI
-   select X86_PLATFORM_DEVICES if ACPI
-   select ACPI_WMI if ACPI
-   select ACPI_VIDEO if ACPI
select ACPI_BUTTON if ACPI
select SYNC_FILE
select IOSF_MBI if X86
diff --git a/drivers/gpu/drm/radeon/Kconfig b/drivers/gpu/drm/radeon/Kconfig
index f98356be0af2..12149d594100 100644
--- a/drivers/gpu/drm/radeon/Kconfig
+++ b/drivers/gpu/drm/radeon/Kconfig
@@ -19,13 +19,6 @@ config DRM_RADEON
select INTERVAL_TREE
select I2C
select I2C_ALGOBIT
-   # radeon depends on ACPI_VIDEO when ACPI is enabled, for select to work
-   # ACPI_VIDEO's dependencies must also be selected.
-   select INPUT if ACPI
-   select ACPI_VIDEO if ACPI
-   # On x86 ACPI_VIDEO also needs ACPI_WMI
-   select X86_PLATFORM_DEVICES if ACPI && X86
-   select ACPI_WMI if ACPI && X86
help
  Choose this option if you have an ATI Radeon graphics card.  There
  are both PCI and AGP versions.  You don't need to choose this to
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index e36ae1f0d885..cf60bdcafb0c 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -19,13 +19,7 @@ config DRM_XE
select DRM_MIPI_DSI
select RELAY
select IRQ_WORK
-   # xe depends on ACPI_VIDEO when ACPI is enabled
-   # but for select to work, need to select ACPI_VIDEO's dependencies, ick
-   select BACKLIGHT_CLASS_DEVICE if ACPI
-   select INPUT if ACPI
-   select ACPI_VIDEO if X86 && ACPI
select ACPI_BUTTON if ACPI
-   select ACPI_WMI if X86 && ACPI
select SYNC_FILE
select IOSF_MBI
select CRC32
-- 
2.34.1



[PATCH v3 4/5] drm/nouveau: Use drm_get_acpi_edid() helper

2024-02-01 Thread Mario Limonciello
Rather than inventing a wrapper to acpi_video_get_edid() use the
one provided by drm. This fixes two problems:
1. A memory leak that the memory provided by the ACPI call was
   never freed.
2. Validation of the BIOS provided blob.

Signed-off-by: Mario Limonciello 
---
v1->v2:
 * New patch
---
 drivers/gpu/drm/nouveau/nouveau_acpi.c  | 27 -
 drivers/gpu/drm/nouveau/nouveau_acpi.h  |  2 --
 drivers/gpu/drm/nouveau/nouveau_connector.c |  2 +-
 3 files changed, 1 insertion(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c 
b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index 8f0c69aad248..de9daafb3fbb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -360,33 +360,6 @@ void nouveau_unregister_dsm_handler(void) {}
 void nouveau_switcheroo_optimus_dsm(void) {}
 #endif
 
-void *
-nouveau_acpi_edid(struct drm_device *dev, struct drm_connector *connector)
-{
-   struct acpi_device *acpidev;
-   int type, ret;
-   void *edid;
-
-   switch (connector->connector_type) {
-   case DRM_MODE_CONNECTOR_LVDS:
-   case DRM_MODE_CONNECTOR_eDP:
-   type = ACPI_VIDEO_DISPLAY_LCD;
-   break;
-   default:
-   return NULL;
-   }
-
-   acpidev = ACPI_COMPANION(dev->dev);
-   if (!acpidev)
-   return NULL;
-
-   ret = acpi_video_get_edid(acpidev, type, -1, );
-   if (ret < 0)
-   return NULL;
-
-   return kmemdup(edid, EDID_LENGTH, GFP_KERNEL);
-}
-
 bool nouveau_acpi_video_backlight_use_native(void)
 {
return acpi_video_backlight_use_native();
diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.h 
b/drivers/gpu/drm/nouveau/nouveau_acpi.h
index e39dd8b94b8b..6a3def8e6cca 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.h
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.h
@@ -10,7 +10,6 @@ bool nouveau_is_v1_dsm(void);
 void nouveau_register_dsm_handler(void);
 void nouveau_unregister_dsm_handler(void);
 void nouveau_switcheroo_optimus_dsm(void);
-void *nouveau_acpi_edid(struct drm_device *, struct drm_connector *);
 bool nouveau_acpi_video_backlight_use_native(void);
 void nouveau_acpi_video_register_backlight(void);
 #else
@@ -19,7 +18,6 @@ static inline bool nouveau_is_v1_dsm(void) { return false; };
 static inline void nouveau_register_dsm_handler(void) {}
 static inline void nouveau_unregister_dsm_handler(void) {}
 static inline void nouveau_switcheroo_optimus_dsm(void) {}
-static inline void *nouveau_acpi_edid(struct drm_device *dev, struct 
drm_connector *connector) { return NULL; }
 static inline bool nouveau_acpi_video_backlight_use_native(void) { return 
true; }
 static inline void nouveau_acpi_video_register_backlight(void) {}
 #endif
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 856b3ef5edb8..746571d4cac0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -713,7 +713,7 @@ nouveau_connector_detect_lvds(struct drm_connector 
*connector, bool force)
 * valid - it's not (rh#613284)
 */
if (nv_encoder->dcb->lvdsconf.use_acpi_for_edid) {
-   edid = nouveau_acpi_edid(dev, connector);
+   edid = drm_get_acpi_edid(connector);
if (edid) {
status = connector_status_connected;
goto out;
-- 
2.34.1



[PATCH v3 1/5] ACPI: video: Handle fetching EDID that is longer than 256 bytes

2024-02-01 Thread Mario Limonciello
The ACPI specification allows for an EDID to be up to 512 bytes but
the _DDC EDID fetching code will only try up to 256 bytes.

Modify the code to instead start at 512 bytes and work it's way
down instead.

As _DDC is now called up to 4 times on a machine debugging messages
are noisier than necessary.  Decrease from info to debug.

Link: 
https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device
Signed-off-by: Mario Limonciello 
---
v1->v2:
 * Use for loop for acpi_video_get_edid()
 * Use one of Rafael's suggestions for acpi_video_device_EDID()
 * Decrease message level too
---
 drivers/acpi/acpi_video.c | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 4afdda9db019..3bfd013e09d2 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -625,12 +625,9 @@ acpi_video_device_EDID(struct acpi_video_device *device,
 
if (!device)
return -ENODEV;
-   if (length == 128)
-   arg0.integer.value = 1;
-   else if (length == 256)
-   arg0.integer.value = 2;
-   else
+   if (!length || (length % 128))
return -EINVAL;
+   arg0.integer.value = length / 128;
 
status = acpi_evaluate_object(device->dev->handle, "_DDC", , 
);
if (ACPI_FAILURE(status))
@@ -641,7 +638,8 @@ acpi_video_device_EDID(struct acpi_video_device *device,
if (obj && obj->type == ACPI_TYPE_BUFFER)
*edid = obj;
else {
-   acpi_handle_info(device->dev->handle, "Invalid _DDC data\n");
+   acpi_handle_debug(device->dev->handle,
+"Invalid _DDC data for length %ld\n", length);
status = -EFAULT;
kfree(obj);
}
@@ -1447,7 +1445,6 @@ int acpi_video_get_edid(struct acpi_device *device, int 
type, int device_id,
 
for (i = 0; i < video->attached_count; i++) {
video_device = video->attached_array[i].bind_info;
-   length = 256;
 
if (!video_device)
continue;
@@ -1478,18 +1475,14 @@ int acpi_video_get_edid(struct acpi_device *device, int 
type, int device_id,
continue;
}
 
-   status = acpi_video_device_EDID(video_device, , length);
-
-   if (ACPI_FAILURE(status) || !buffer ||
-   buffer->type != ACPI_TYPE_BUFFER) {
-   length = 128;
+   for (length = 512; length > 0; length -= 128) {
status = acpi_video_device_EDID(video_device, ,
length);
-   if (ACPI_FAILURE(status) || !buffer ||
-   buffer->type != ACPI_TYPE_BUFFER) {
-   continue;
-   }
+   if (ACPI_SUCCESS(status))
+   break;
}
+   if (!length)
+   continue;
 
*edid = buffer->buffer.pointer;
return length;
-- 
2.34.1



[PATCH v3 3/5] drm/amd: Fetch the EDID from _DDC if available for eDP

2024-02-01 Thread Mario Limonciello
Some manufacturers have intentionally put an EDID that differs from
the EDID on the internal panel on laptops.

Attempt to fetch this EDID if it exists and prefer it over the EDID
that is provided by the panel.

Signed-off-by: Mario Limonciello 
---
v2:
 * Use drm helper which will run more validation
 * Move eDP check to DRM helper
 * Add module parameter
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c |  4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  8 
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 10 --
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  |  9 ++---
 5 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 3d8a48f46b01..5d5be3e20687 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -217,6 +217,7 @@ extern int amdgpu_smartshift_bias;
 extern int amdgpu_use_xgmi_p2p;
 extern int amdgpu_mtype_local;
 extern bool enforce_isolation;
+extern bool acpi_edid;
 #ifdef CONFIG_HSA_AMD
 extern int sched_policy;
 extern bool debug_evictions;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 9caba10315a8..6aa8cc431abe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -278,6 +278,10 @@ static void amdgpu_connector_get_edid(struct drm_connector 
*connector)
struct amdgpu_device *adev = drm_to_adev(dev);
struct amdgpu_connector *amdgpu_connector = 
to_amdgpu_connector(connector);
 
+   /* if the BIOS specifies the EDID via _DDC, prefer this */
+   if (acpi_edid && !amdgpu_connector->edid)
+   amdgpu_connector->edid = drm_get_acpi_edid(connector);
+
if (amdgpu_connector->edid)
return;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index cc69005f5b46..be7a4da85a8b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -166,6 +166,7 @@ uint amdgpu_sdma_phase_quantum = 32;
 char *amdgpu_disable_cu;
 char *amdgpu_virtual_display;
 bool enforce_isolation;
+bool acpi_edid = true;
 /*
  * OverDrive(bit 14) disabled by default
  * GFX DCS(bit 19) disabled by default
@@ -990,6 +991,13 @@ MODULE_PARM_DESC(wbrf,
"Enable Wifi RFI interference mitigation (0 = disabled, 1 = enabled, -1 
= auto(default)");
 module_param_named(wbrf, amdgpu_wbrf, int, 0444);
 
+/**
+ * DOC: acpi_edid (bool)
+ * Try to fetch EDID for eDP display from BIOS using ACPI _DDC method.
+ */
+module_param(acpi_edid, bool, 0444);
+MODULE_PARM_DESC(acpi_edid, "Fetch EDID for eDP display from BIOS");
+
 /* These devices are not supported by amdgpu.
  * They are supported by the mach64, r128, radeon drivers
  */
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 202c6ad443a3..688d615c6687 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6589,7 +6589,11 @@ static void amdgpu_dm_connector_funcs_force(struct 
drm_connector *connector)
struct amdgpu_connector *amdgpu_connector = 
to_amdgpu_connector(connector);
struct dc_link *dc_link = aconnector->dc_link;
struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
-   struct edid *edid;
+   struct edid *edid = NULL;
+
+   /* prefer ACPI over panel for eDP */
+   if (acpi_edid)
+   edid = drm_get_acpi_edid(connector);
 
/*
 * Note: drm_get_edid gets edid in the following order:
@@ -6597,7 +6601,9 @@ static void amdgpu_dm_connector_funcs_force(struct 
drm_connector *connector)
 * 2) firmware EDID if set via edid_firmware module parameter
 * 3) regular DDC read.
 */
-   edid = drm_get_edid(connector, _connector->ddc_bus->aux.ddc);
+   if (!edid)
+   edid = drm_get_edid(connector, 
_connector->ddc_bus->aux.ddc);
+
if (!edid) {
DRM_ERROR("No EDID found on connector: %s.\n", connector->name);
return;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 85b7f58a7f35..cc39b1c14aa8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -899,7 +899,7 @@ enum dc_edid_status dm_helpers_read_local_edid(
struct i2c_adapter *ddc;
int retry = 3;
enum dc_edid_status edid_status;
-   struct edid *edid;
+   struct edid *edid = NULL;
 
if (link->aux_mode)
ddc = >dm_dp_aux.aux.ddc;
@@ -910,8 +910,11 @@ enum dc_edid_status dm_helpers_read_local_edid(
 * do check 

[PATCH v2] amdkfd: pass debug exceptions to second-level trap handler

2024-02-01 Thread Laurent Morichetti
Call the 2nd level trap handler if the cwsr handler is entered with any
one of wave_start, wave_end, or trap_after_inst exceptions.

Signed-off-by: Laurent Morichetti 
Tested-by: Lancelot Six 
Reviewed-by: Jay Cornwall 
---
 drivers/gpu/drm/amd/amdkfd/cwsr_trap_handler.h  |  2 +-
 .../drm/amd/amdkfd/cwsr_trap_handler_gfx10.asm  | 17 -
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/cwsr_trap_handler.h 
b/drivers/gpu/drm/amd/amdkfd/cwsr_trap_handler.h
index d1caaf0e6a7c..2e9b64edb8d2 100644
--- a/drivers/gpu/drm/amd/amdkfd/cwsr_trap_handler.h
+++ b/drivers/gpu/drm/amd/amdkfd/cwsr_trap_handler.h
@@ -2518,7 +2518,7 @@ static const uint32_t cwsr_trap_gfx11_hex[] = {
0x8b6eff7b, 0x0400,
0xbfa20045, 0xbf830010,
0xb8fbf803, 0xbfa0fffa,
-   0x8b6eff7b, 0x0900,
+   0x8b6eff7b, 0x00160900,
0xbfa20015, 0x8b6eff7b,
0x71ff, 0xbfa10008,
0x8b6fff7b, 0x7080,
diff --git a/drivers/gpu/drm/amd/amdkfd/cwsr_trap_handler_gfx10.asm 
b/drivers/gpu/drm/amd/amdkfd/cwsr_trap_handler_gfx10.asm
index 71b3dc0c7363..7568ff3af978 100644
--- a/drivers/gpu/drm/amd/amdkfd/cwsr_trap_handler_gfx10.asm
+++ b/drivers/gpu/drm/amd/amdkfd/cwsr_trap_handler_gfx10.asm
@@ -81,6 +81,11 @@ var SQ_WAVE_TRAPSTS_POST_SAVECTX_SHIFT   = 11
 var SQ_WAVE_TRAPSTS_POST_SAVECTX_SIZE  = 21
 var SQ_WAVE_TRAPSTS_ILLEGAL_INST_MASK  = 0x800
 var SQ_WAVE_TRAPSTS_EXCP_HI_MASK   = 0x7000
+#if ASIC_FAMILY >= CHIP_PLUM_BONITO
+var SQ_WAVE_TRAPSTS_WAVE_START_MASK= 0x2
+var SQ_WAVE_TRAPSTS_WAVE_END_MASK  = 0x4
+var SQ_WAVE_TRAPSTS_TRAP_AFTER_INST_MASK   = 0x10
+#endif
 
 var SQ_WAVE_MODE_EXCP_EN_SHIFT = 12
 var SQ_WAVE_MODE_EXCP_EN_ADDR_WATCH_SHIFT  = 19
@@ -92,6 +97,16 @@ var SQ_WAVE_IB_STS_RCNT_FIRST_REPLAY_MASK= 0x003F8000
 
 var SQ_WAVE_MODE_DEBUG_EN_MASK = 0x800
 
+#if ASIC_FAMILY < CHIP_PLUM_BONITO
+var S_TRAPSTS_NON_MASKABLE_EXCP_MASK   = 
SQ_WAVE_TRAPSTS_MEM_VIOL_MASK|SQ_WAVE_TRAPSTS_ILLEGAL_INST_MASK
+#else
+var S_TRAPSTS_NON_MASKABLE_EXCP_MASK   = SQ_WAVE_TRAPSTS_MEM_VIOL_MASK 
|\
+ 
SQ_WAVE_TRAPSTS_ILLEGAL_INST_MASK |\
+ 
SQ_WAVE_TRAPSTS_WAVE_START_MASK   |\
+ SQ_WAVE_TRAPSTS_WAVE_END_MASK 
|\
+ 
SQ_WAVE_TRAPSTS_TRAP_AFTER_INST_MASK
+#endif
+
 // bits [31:24] unused by SPI debug data
 var TTMP11_SAVE_REPLAY_W64H_SHIFT  = 31
 var TTMP11_SAVE_REPLAY_W64H_MASK   = 0x8000
@@ -224,7 +239,7 @@ L_NOT_HALTED:
// Check non-maskable exceptions. memory_violation, illegal_instruction
// and xnack_error exceptions always cause the wave to enter the trap
// handler.
-   s_and_b32   ttmp2, s_save_trapsts, 
SQ_WAVE_TRAPSTS_MEM_VIOL_MASK|SQ_WAVE_TRAPSTS_ILLEGAL_INST_MASK
+   s_and_b32   ttmp2, s_save_trapsts, S_TRAPSTS_NON_MASKABLE_EXCP_MASK
s_cbranch_scc1  L_FETCH_2ND_TRAP
 
// Check for maskable exceptions in trapsts.excp and trapsts.excp_hi.

base-commit: c4b562a17829454713e45219fa754be1bfda9004
-- 
2.25.1



[PATCH] drm/amdkfd: update SIMD distribution algo for GFXIP 9.4.2 onwards

2024-02-01 Thread Rajneesh Bhardwaj
In certain cooperative group dispatch scenarios the default SPI resource
allocation may cause reduced per-CU workgroup occupancy. Set
COMPUTE_RESOURCE_LIMITS.FORCE_SIMD_DIST=1 to mitigate soft hang
scenarions.

Suggested-by: Joseph Greathouse 
Signed-off-by: Rajneesh Bhardwaj 
---
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
index 42d881809dc7..4b28e7dcb62f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -303,6 +303,10 @@ static void update_mqd(struct mqd_manager *mm, void *mqd,
update_cu_mask(mm, mqd, minfo, 0);
set_priority(m, q);
 
+   if (KFD_GC_VERSION(mm->dev) >= IP_VERSION(9, 4, 2))
+   m->compute_resource_limits = q->is_gws ?
+   COMPUTE_RESOURCE_LIMITS__FORCE_SIMD_DIST_MASK : 0;
+
q->is_active = QUEUE_IS_ACTIVE(*q);
 }
 
-- 
2.34.1



Re: [PATCH v3 3/3] drm/amdgpu: Implement check_async_props for planes

2024-02-01 Thread André Almeida

Hi Sima,

Em 30/01/2024 07:56, Daniel Vetter escreveu:

On Sun, Jan 28, 2024 at 06:25:15PM -0300, André Almeida wrote:

AMD GPUs can do async flips with changes on more properties than just
the FB ID, so implement a custom check_async_props for AMD planes.

Allow amdgpu to do async flips with overlay planes as well.

Signed-off-by: André Almeida 
---
v3: allow overlay planes


This comment very much written with a lack of clearly better ideas, but:

Do we really need this much flexibility, especially for the first driver
adding the first few additional properties?

A simple bool on struct drm_plane to indicate whether async flips are ok
or not should also do this job here? Maybe a bit of work to roll that out
to the primary planes for current drivers, but not much. And wouldn't need
drivers to implement some very uapi-marshalling atomic code ...


Right, perhaps I can first write in the way you suggest, and later 
expand to the form I have proposed here if/when new properties arise.




Also we could probably remove the current drm_mode_config.async_flip flag
and entirely replace it with the per-plane one.
-Sima


  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 29 +++
  1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 116121e647ca..ed75b69636b4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -25,6 +25,7 @@
   */
  
  #include 

+#include 
  #include 
  #include 
  #include 
@@ -1430,6 +1431,33 @@ static void 
amdgpu_dm_plane_drm_plane_destroy_state(struct drm_plane *plane,
drm_atomic_helper_plane_destroy_state(plane, state);
  }
  
+static int amdgpu_dm_plane_check_async_props(struct drm_property *prop,

+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = >dev->mode_config;
+   int ret;
+
+   if (prop != config->prop_fb_id &&
+   prop != config->prop_in_fence_fd) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, _val);
+   return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);
+   }
+
+   if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY &&
+   plane_state->plane->type != DRM_PLANE_TYPE_OVERLAY) {
+   drm_dbg_atomic(prop->dev,
+  "[OBJECT:%d] Only primary or overlay planes can be 
changed during async flip\n",
+  obj->id);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
  static const struct drm_plane_funcs dm_plane_funcs = {
.update_plane   = drm_atomic_helper_update_plane,
.disable_plane  = drm_atomic_helper_disable_plane,
@@ -1438,6 +1466,7 @@ static const struct drm_plane_funcs dm_plane_funcs = {
.atomic_duplicate_state = amdgpu_dm_plane_drm_plane_duplicate_state,
.atomic_destroy_state = amdgpu_dm_plane_drm_plane_destroy_state,
.format_mod_supported = amdgpu_dm_plane_format_mod_supported,
+   .check_async_props = amdgpu_dm_plane_check_async_props,
  };
  
  int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,

--
2.43.0





Re: [PATCH v3 1/3] drm/atomic: Allow drivers to write their own plane check for async flips

2024-02-01 Thread André Almeida

Hi Pekka,

Em 29/01/2024 05:49, Pekka Paalanen escreveu:

On Sun, 28 Jan 2024 18:25:13 -0300
André Almeida  wrote:


Some hardware are more flexible on what they can flip asynchronously, so
rework the plane check so drivers can implement their own check, lifting
up some of the restrictions.

Signed-off-by: André Almeida 
---
v3: no changes

  drivers/gpu/drm/drm_atomic_uapi.c | 62 ++-
  include/drm/drm_atomic_uapi.h | 12 ++
  include/drm/drm_plane.h   |  5 +++
  3 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index aee4a65d4959..6d5b9fec90c7 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -620,7 +620,7 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
return 0;
  }
  
-static int

+int
  drm_atomic_plane_get_property(struct drm_plane *plane,
const struct drm_plane_state *state,
struct drm_property *property, uint64_t *val)
@@ -683,6 +683,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
  
  	return 0;

  }
+EXPORT_SYMBOL(drm_atomic_plane_get_property);
  
  static int drm_atomic_set_writeback_fb_for_connector(

struct drm_connector_state *conn_state,
@@ -1026,18 +1027,54 @@ int drm_atomic_connector_commit_dpms(struct 
drm_atomic_state *state,
return ret;
  }
  
-static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t prop_value,

+int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t 
prop_value,


Hi,

should the word "async" be somewhere in the function name?


 struct drm_property *prop)
  {
if (ret != 0 || old_val != prop_value) {
drm_dbg_atomic(prop->dev,
-  "[PROP:%d:%s] No prop can be changed during async 
flip\n",
+  "[PROP:%d:%s] This prop cannot be changed during 
async flip\n",
   prop->base.id, prop->name);
return -EINVAL;
}
  
  	return 0;

  }
+EXPORT_SYMBOL(drm_atomic_check_prop_changes);
+
+/* plane changes may have exceptions, so we have a special function for them */
+static int drm_atomic_check_plane_changes(struct drm_property *prop,
+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+   struct drm_mode_config *config = >dev->mode_config;
+   int ret;
+
+   if (plane->funcs->check_async_props)
+   return plane->funcs->check_async_props(prop, plane, plane_state,
+obj, prop_value, 
old_val);


Is it really ok to allow drivers to opt-in to also *reject* the FB_ID
and IN_FENCE_FD props on async commits?

Either intentionally or by accident.



Right, perhaps I should write this function in a way that you can only 
lift restrictions, and not add new ones.



+
+   /*
+* if you are trying to change something other than the FB ID, your
+* change will be either rejected or ignored, so we can stop the check
+* here
+*/
+   if (prop != config->prop_fb_id) {
+   ret = drm_atomic_plane_get_property(plane, plane_state,
+   prop, _val);
+   return drm_atomic_check_prop_changes(ret, old_val, prop_value, 
prop);


When I first read this code, it seemed to say: "If the prop is not
FB_ID, then do the usual prop change checking that happens on all
changes, not only async.". Reading this patch a few more times over, I
finally realized drm_atomic_check_prop_changes() is about async
specifically.



I see that the lack of the async word is giving some confusion, so I'll 
add it to the functions.


Thanks,
André


+   }
+
+   if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) {
+   drm_dbg_atomic(prop->dev,
+  "[OBJECT:%d] Only primary planes can be changed 
during async flip\n",
+  obj->id);
+   return -EINVAL;
+   }
+
+   return 0;
+}
  
  int drm_atomic_set_property(struct drm_atomic_state *state,

struct drm_file *file_priv,
@@ -1100,7 +1137,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
*state,
case DRM_MODE_OBJECT_PLANE: {
struct drm_plane *plane = obj_to_plane(obj);
struct drm_plane_state *plane_state;
-   struct drm_mode_config *config = >dev->mode_config;
  
  		plane_state = drm_atomic_get_plane_state(state, plane);

if (IS_ERR(plane_state)) {
@@ -1108,19 +1144,11 @@ int 

[pull] amdgpu, amdkfd drm-fixes-6.8

2024-02-01 Thread Alex Deucher
Hi Dave, Sima,

Fixes for 6.8.

The following changes since commit 41bccc98fb7931d63d03f326a746ac4d429c1dd3:

  Linux 6.8-rc2 (2024-01-28 17:01:12 -0800)

are available in the Git repository at:

  https://gitlab.freedesktop.org/agd5f/linux.git 
tags/amd-drm-fixes-6.8-2024-02-01

for you to fetch changes up to 6813cdca4ab94a238f8eb0cef3d3f3fcbdfb0ee0:

  drm/amdgpu/pm: Use inline function for IP version check (2024-02-01 09:11:38 
-0500)


amd-drm-fixes-6.8-2024-02-01:

amdgpu:
- Fix reboot issue seen on some 7000 series dGPUs
- Fix client init order for KFD
- Misc display fixes
- USB-C fix
- DCN 3.5 fixes
- Fix issues with GPU scheduler and GPU reset
- GPU firmware loading fix
- Misc fixes
- GC 11.5 fix
- VCN 4.0.5 fix
- IH overflow fix

amdkfd:
- SVM fixes
- Trap handler fix
- Fix device permission lookup
- Properly reserve BO before validating it


Charlene Liu (2):
  Revert "drm/amd/display: initialize all the dpm level's stutter latency"
  drm/amd/display: fix USB-C flag update after enc10 feature init

David McFarland (1):
  drm/amd: Don't init MEC2 firmware when it fails to load

Dmytro Laktyushkin (1):
  drm/amd/display: Fix DPSTREAM CLK on and off sequence

Fangzhi Zuo (1):
  drm/amd/display: Fix dcn35 8k30 Underflow/Corruption Issue

Friedrich Vock (1):
  drm/amdgpu: Reset IH OVERFLOW_CLEAR bit

Jay Cornwall (1):
  drm/amdkfd: Use S_ENDPGM_SAVED in trap handler

Lang Yu (1):
  drm/amdkfd: reserve the BO before validating it

Le Ma (1):
  drm/amdgpu: move the drm client creation behind drm device registration

Ma Jun (2):
  drm/amdgpu: Fix the warning info in mode1 reset
  drm/amdgpu/pm: Use inline function for IP version check

Mario Limonciello (1):
  Revert "drm/amd/pm: fix the high voltage and temperature issue"

Mukul Joshi (1):
  drm/amdkfd: Use correct drm device for cgroup permission check

Nicholas Susanto (1):
  drm/amd/display: Underflow workaround by increasing SR exit latency

Philip Yang (1):
  drm/amdkfd: Correct partial migration virtual addr

Sohaib Nadeem (1):
  drm/amd/display: increased min_dcfclk_mhz and min_fclk_mhz

Srinivasan Shanmugam (3):
  drm/amd/display: Add NULL check for kzalloc in 
'amdgpu_dm_atomic_commit_tail()'
  drm/amd/display: Fix buffer overflow in 
'get_host_router_total_dp_tunnel_bw()'
  drm/amdgpu: Fix missing error code in 'gmc_v6/7/8/9_0_hw_init()'

Wenjing Liu (1):
  drm/amd/display: fix incorrect mpc_combine array size

Yifan Zhang (2):
  drm/amdgpu: drm/amdgpu: remove golden setting for gfx 11.5.0
  drm/amdgpu: remove asymmetrical irq disabling in vcn 4.0.5 suspend

 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 32 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h |  4 ++-
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c   | 20 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c|  8 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  4 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   | 12 
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h   |  2 +-
 drivers/gpu/drm/amd/amdgpu/cik_ih.c|  6 
 drivers/gpu/drm/amd/amdgpu/cz_ih.c |  5 +++
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c |  2 --
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 22 -
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c  |  4 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  4 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  4 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  |  4 +--
 drivers/gpu/drm/amd/amdgpu/iceland_ih.c|  5 +++
 drivers/gpu/drm/amd/amdgpu/ih_v6_0.c   |  6 
 drivers/gpu/drm/amd/amdgpu/ih_v6_1.c   |  7 +
 drivers/gpu/drm/amd/amdgpu/navi10_ih.c |  6 
 drivers/gpu/drm/amd/amdgpu/si_ih.c |  6 
 drivers/gpu/drm/amd/amdgpu/tonga_ih.c  |  6 
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c  | 17 --
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c| 19 
 drivers/gpu/drm/amd/amdgpu/vega10_ih.c |  6 
 drivers/gpu/drm/amd/amdgpu/vega20_ih.c |  6 
 drivers/gpu/drm/amd/amdkfd/cwsr_trap_handler.h | 14 -
 .../gpu/drm/amd/amdkfd/cwsr_trap_handler_gfx10.asm |  2 +-
 .../gpu/drm/amd/amdkfd/cwsr_trap_handler_gfx9.asm  |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c   |  4 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c   |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h  |  9 --
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  4 +++
 .../amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c   

[PATCH] drm/amdgpu: Fix potential out-of-bounds access in 'amdgpu_discovery_reg_base_init()'

2024-02-01 Thread Srinivasan Shanmugam
The issue arises when the array 'adev->vcn.vcn_config' is accessed
before checking if the index 'adev->vcn.num_vcn_inst' is within the
bounds of the array.

The fix involves moving the bounds check before the array access. This
ensures that 'adev->vcn.num_vcn_inst' is within the bounds of the array
before it is used as an index.

Fixes the below:
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1289 
amdgpu_discovery_reg_base_init() error: testing array offset 
'adev->vcn.num_vcn_inst' after use.

Cc: Christian König 
Cc: Alex Deucher 
Signed-off-by: Srinivasan Shanmugam 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index ef800590c1ab..83da46d73f70 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -1282,11 +1282,11 @@ static int amdgpu_discovery_reg_base_init(struct 
amdgpu_device *adev)
 * 0b10 : encode is disabled
 * 0b01 : decode is disabled
 */
-   adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
-   ip->revision & 0xc0;
-   ip->revision &= ~0xc0;
if (adev->vcn.num_vcn_inst <
AMDGPU_MAX_VCN_INSTANCES) {
+   
adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
+   ip->revision & 0xc0;
+   ip->revision &= ~0xc0;
adev->vcn.num_vcn_inst++;
adev->vcn.inst_mask |=
(1U << ip->instance_number);
-- 
2.34.1



[PATCH 2/2] drm/amdgpu: Improve huge page mapping update

2024-02-01 Thread Philip Yang
Update huge page mapping, ex 2MB address and size aligned, we alloc PTB
bo, and then free the PTB bo after updating PDE0 as PTE.

If fragment size >= parent_shift, don't alloc PT bo, because we will
update PDE entry, this will improve the huge page mapping update
by removing the extra PTB bo alloc and free.

Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index a3d609655ce3..ef3ef03e50ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -916,7 +916,11 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params 
*params,
uint64_t incr, entry_end, pe_start;
struct amdgpu_bo *pt;
 
-   if (!params->unlocked) {
+   shift = amdgpu_vm_pt_level_shift(adev, cursor.level);
+   parent_shift = amdgpu_vm_pt_level_shift(adev, cursor.level - 1);
+
+   if (!params->unlocked &&
+   (adev->asic_type < CHIP_VEGA10 || frag < parent_shift)) {
/* make sure that the page tables covering the
 * address range are actually allocated
 */
@@ -926,8 +930,6 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params 
*params,
return r;
}
 
-   shift = amdgpu_vm_pt_level_shift(adev, cursor.level);
-   parent_shift = amdgpu_vm_pt_level_shift(adev, cursor.level - 1);
if (params->unlocked) {
/* Unlocked updates are only allowed on the leaves */
if (amdgpu_vm_pt_descendant(adev, ))
-- 
2.35.1



[PATCH 1/2] drm/amdgpu: Unmap only clear the page table leaves

2024-02-01 Thread Philip Yang
SVM migration unmap pages from GPU and then update mapping to GPU to
recover page fault. Currently unmap clears the PDE entry for range
length >= huge page and free PTB bo, update mapping to alloc new PT bo.
There is race bug that the freed entry bo maybe still on the pt_free
list, reused when updating mapping and then freed, leave invalid PDE
entry and cause GPU page fault.

By setting the update to clear only one PDE entry or clear PTB, to
avoid unmap to free PTE bo. This fixes the race bug and improve the
unmap and map to GPU performance. Update mapping to huge page will
still free the PTB bo.

With this change, the vm->pt_freed list and work is not needed. Add
WARN_ON(unlocked) in amdgpu_vm_pt_free_dfs to catch if unmap to free the
PTB.

Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c|  4 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h|  4 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 43 ++-
 3 files changed, 10 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 82e5fd66a10d..3bde77dfc63f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2256,8 +2256,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
spin_lock_init(>status_lock);
INIT_LIST_HEAD(>freed);
INIT_LIST_HEAD(>done);
-   INIT_LIST_HEAD(>pt_freed);
-   INIT_WORK(>pt_free_work, amdgpu_vm_pt_free_work);
INIT_KFIFO(vm->faults);
 
r = amdgpu_vm_init_entities(adev, vm);
@@ -2446,8 +2444,6 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
 
amdgpu_amdkfd_gpuvm_destroy_cb(adev, vm);
 
-   flush_work(>pt_free_work);
-
root = amdgpu_bo_ref(vm->root.bo);
amdgpu_bo_reserve(root, true);
amdgpu_vm_set_pasid(adev, vm, 0);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index cdb61f1e7c35..74fe211b9ecd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -316,10 +316,6 @@ struct amdgpu_vm {
/* BOs which are invalidated, has been updated in the PTs */
struct list_headdone;
 
-   /* PT BOs scheduled to free and fill with zero if vm_resv is not hold */
-   struct list_headpt_freed;
-   struct work_struct  pt_free_work;
-
/* contains the page directory */
struct amdgpu_vm_bo_base root;
struct dma_fence*last_update;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index a160265ddc07..a3d609655ce3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -657,27 +657,6 @@ static void amdgpu_vm_pt_free(struct amdgpu_vm_bo_base 
*entry)
amdgpu_bo_unref(>bo);
 }
 
-void amdgpu_vm_pt_free_work(struct work_struct *work)
-{
-   struct amdgpu_vm_bo_base *entry, *next;
-   struct amdgpu_vm *vm;
-   LIST_HEAD(pt_freed);
-
-   vm = container_of(work, struct amdgpu_vm, pt_free_work);
-
-   spin_lock(>status_lock);
-   list_splice_init(>pt_freed, _freed);
-   spin_unlock(>status_lock);
-
-   /* flush_work in amdgpu_vm_fini ensure vm->root.bo is valid. */
-   amdgpu_bo_reserve(vm->root.bo, true);
-
-   list_for_each_entry_safe(entry, next, _freed, vm_status)
-   amdgpu_vm_pt_free(entry);
-
-   amdgpu_bo_unreserve(vm->root.bo);
-}
-
 /**
  * amdgpu_vm_pt_free_dfs - free PD/PT levels
  *
@@ -696,17 +675,7 @@ static void amdgpu_vm_pt_free_dfs(struct amdgpu_device 
*adev,
struct amdgpu_vm_pt_cursor cursor;
struct amdgpu_vm_bo_base *entry;
 
-   if (unlocked) {
-   spin_lock(>status_lock);
-   for_each_amdgpu_vm_pt_dfs_safe(adev, vm, start, cursor, entry)
-   list_move(>vm_status, >pt_freed);
-
-   if (start)
-   list_move(>entry->vm_status, >pt_freed);
-   spin_unlock(>status_lock);
-   schedule_work(>pt_free_work);
-   return;
-   }
+   WARN_ON(unlocked);
 
for_each_amdgpu_vm_pt_dfs_safe(adev, vm, start, cursor, entry)
amdgpu_vm_pt_free(entry);
@@ -1009,7 +978,15 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params 
*params,
incr = (uint64_t)AMDGPU_GPU_PAGE_SIZE << shift;
mask = amdgpu_vm_pt_entries_mask(adev, cursor.level);
pe_start = ((cursor.pfn >> shift) & mask) * 8;
-   entry_end = ((uint64_t)mask + 1) << shift;
+
+   if (cursor.level < AMDGPU_VM_PTB && params->unlocked)
+   /*
+* Unmap to clear one PDE entry, to avoid unmap to free
+* PTB using pt_free work which has race condition.
+*/
+

RE: [PATCH] drm/amdgpu: Only create mes event log debugfs when mes is enabled

2024-02-01 Thread Kasiviswanathan, Harish
[AMD Official Use Only - General]

Reviewed-by: Harish Kasiviswanathan 


-Original Message-
From: amd-gfx  On Behalf Of Liu, Shaoyun
Sent: Thursday, February 1, 2024 10:58 AM
To: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amdgpu: Only create mes event log debugfs when mes is 
enabled

[AMD Official Use Only - General]

[AMD Official Use Only - General]

ping

-Original Message-
From: Liu, Shaoyun 
Sent: Wednesday, January 31, 2024 9:26 AM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun 
Subject: [PATCH] drm/amdgpu: Only create mes event log debugfs when mes is 
enabled

Skip the debugfs file creation for mes event log if the GPU doesn't use MES. 
This to prevent potential kernel oops when user try to read the event log in 
debugfs on a GPU without MES

Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index 0626ac0192a8..dd2b8f3fa2f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -1565,9 +1565,9 @@ void amdgpu_debugfs_mes_event_log_init(struct 
amdgpu_device *adev)  #if defined(CONFIG_DEBUG_FS)
struct drm_minor *minor = adev_to_drm(adev)->primary;
struct dentry *root = minor->debugfs_root;
-
-   debugfs_create_file("amdgpu_mes_event_log", 0444, root,
-   adev, _debugfs_mes_event_log_fops);
+   if (adev->enable_mes)
+   debugfs_create_file("amdgpu_mes_event_log", 0444, root,
+   adev, _debugfs_mes_event_log_fops);

 #endif
 }
--
2.34.1



RE: [PATCH] drm/amdgpu: Only create mes event log debugfs when mes is enabled

2024-02-01 Thread Liu, Shaoyun
[AMD Official Use Only - General]

ping

-Original Message-
From: Liu, Shaoyun 
Sent: Wednesday, January 31, 2024 9:26 AM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun 
Subject: [PATCH] drm/amdgpu: Only create mes event log debugfs when mes is 
enabled

Skip the debugfs file creation for mes event log if the GPU doesn't use MES. 
This to prevent potential kernel oops when user try to read the event log in 
debugfs on a GPU without MES

Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index 0626ac0192a8..dd2b8f3fa2f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -1565,9 +1565,9 @@ void amdgpu_debugfs_mes_event_log_init(struct 
amdgpu_device *adev)  #if defined(CONFIG_DEBUG_FS)
struct drm_minor *minor = adev_to_drm(adev)->primary;
struct dentry *root = minor->debugfs_root;
-
-   debugfs_create_file("amdgpu_mes_event_log", 0444, root,
-   adev, _debugfs_mes_event_log_fops);
+   if (adev->enable_mes)
+   debugfs_create_file("amdgpu_mes_event_log", 0444, root,
+   adev, _debugfs_mes_event_log_fops);

 #endif
 }
--
2.34.1



RE: [PATCH v2] drm/amd/display: Add NULL test for 'timing generator' in 'dcn21_set_pipe()'

2024-02-01 Thread Koo, Anthony
[AMD Official Use Only - General]

Reviewed-by: Anthony Koo 

Thanks,
Anthony

-Original Message-
From: SHANMUGAM, SRINIVASAN 
Sent: Thursday, February 1, 2024 4:59 AM
To: Siqueira, Rodrigo ; Pillai, Aurabindo 
; Koo, Anthony 
Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN 
; Sun, Yongqiang 
Subject: [PATCH v2] drm/amd/display: Add NULL test for 'timing generator' in 
'dcn21_set_pipe()'

In "u32 otg_inst = pipe_ctx->stream_res.tg->inst;"
pipe_ctx->stream_res.tg could be NULL, it is relying on the caller to ensure 
the tg is not NULL.

Fixes: 474ac4a875ca ("drm/amd/display: Implement some asic specific abm call 
backs.")
Cc: Yongqiang Sun 
Cc: Anthony Koo 
Cc: Rodrigo Siqueira 
Cc: Aurabindo Pillai 
Signed-off-by: Srinivasan Shanmugam 
---
v2:
  - s/u32/uint32_t for consistency (Anthony)

 .../amd/display/dc/hwss/dcn21/dcn21_hwseq.c   | 24 +++
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
index 8e88dcaf88f5..8323077bba15 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
@@ -206,28 +206,32 @@ void dcn21_set_abm_immediate_disable(struct pipe_ctx 
*pipe_ctx)  void dcn21_set_pipe(struct pipe_ctx *pipe_ctx)  {
struct abm *abm = pipe_ctx->stream_res.abm;
-   uint32_t otg_inst = pipe_ctx->stream_res.tg->inst;
+   struct timing_generator *tg = pipe_ctx->stream_res.tg;
struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
struct dmcu *dmcu = pipe_ctx->stream->ctx->dc->res_pool->dmcu;
+   uint32_t otg_inst;
+
+   if (!abm && !tg && !panel_cntl)
+   return;
+
+   otg_inst = tg->inst;

if (dmcu) {
dce110_set_pipe(pipe_ctx);
return;
}

-   if (abm && panel_cntl) {
-   if (abm->funcs && abm->funcs->set_pipe_ex) {
-   abm->funcs->set_pipe_ex(abm,
+   if (abm->funcs && abm->funcs->set_pipe_ex) {
+   abm->funcs->set_pipe_ex(abm,
otg_inst,
SET_ABM_PIPE_NORMAL,
panel_cntl->inst,
panel_cntl->pwrseq_inst);
-   } else {
-   dmub_abm_set_pipe(abm, otg_inst,
-   SET_ABM_PIPE_NORMAL,
-   panel_cntl->inst,
-   panel_cntl->pwrseq_inst);
-   }
+   } else {
+   dmub_abm_set_pipe(abm, otg_inst,
+ SET_ABM_PIPE_NORMAL,
+ panel_cntl->inst,
+ panel_cntl->pwrseq_inst);
}
 }

--
2.34.1



RE: [PATCH v3] drm/amd/display: Fix 'panel_cntl' could be null in 'dcn21_set_backlight_level()'

2024-02-01 Thread Koo, Anthony
[AMD Official Use Only - General]

Reviewed-by: Anthony Koo 

Thanks,
Anthony

-Original Message-
From: SHANMUGAM, SRINIVASAN 
Sent: Thursday, February 1, 2024 4:57 AM
To: Siqueira, Rodrigo ; Pillai, Aurabindo 
; Koo, Anthony 
Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN 
; Sun, Yongqiang 
Subject: [PATCH v3] drm/amd/display: Fix 'panel_cntl' could be null in 
'dcn21_set_backlight_level()'

'panel_cntl' structure used to control the display panel could be null, 
dereferencing it could lead to a null pointer access.

Fixes the below:
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn21/dcn21_hwseq.c:269 
dcn21_set_backlight_level() error: we previously assumed 'panel_cntl' could be 
null (see line 250)

Fixes: 474ac4a875ca ("drm/amd/display: Implement some asic specific abm call 
backs.")
Cc: Yongqiang Sun 
Cc: Anthony Koo 
Cc: Rodrigo Siqueira 
Cc: Aurabindo Pillai 
Signed-off-by: Srinivasan Shanmugam 
---
v3:
 - s/u32/uint32_t for consistency (Anthony)

 .../amd/display/dc/hwss/dcn21/dcn21_hwseq.c   | 39 ++-
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
index 8323077bba15..5c7f380a84f9 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
@@ -241,34 +241,35 @@ bool dcn21_set_backlight_level(struct pipe_ctx *pipe_ctx, 
 {
struct dc_context *dc = pipe_ctx->stream->ctx;
struct abm *abm = pipe_ctx->stream_res.abm;
+   struct timing_generator *tg = pipe_ctx->stream_res.tg;
struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
+   uint32_t otg_inst;
+
+   if (!abm && !tg && !panel_cntl)
+   return false;
+
+   otg_inst = tg->inst;

if (dc->dc->res_pool->dmcu) {
dce110_set_backlight_level(pipe_ctx, backlight_pwm_u16_16, 
frame_ramp);
return true;
}

-   if (abm != NULL) {
-   uint32_t otg_inst = pipe_ctx->stream_res.tg->inst;
-
-   if (abm && panel_cntl) {
-   if (abm->funcs && abm->funcs->set_pipe_ex) {
-   abm->funcs->set_pipe_ex(abm,
-   otg_inst,
-   SET_ABM_PIPE_NORMAL,
-   panel_cntl->inst,
-   panel_cntl->pwrseq_inst);
-   } else {
-   dmub_abm_set_pipe(abm,
-   otg_inst,
-   SET_ABM_PIPE_NORMAL,
-   panel_cntl->inst,
-   
panel_cntl->pwrseq_inst);
-   }
-   }
+   if (abm->funcs && abm->funcs->set_pipe_ex) {
+   abm->funcs->set_pipe_ex(abm,
+   otg_inst,
+   SET_ABM_PIPE_NORMAL,
+   panel_cntl->inst,
+   panel_cntl->pwrseq_inst);
+   } else {
+   dmub_abm_set_pipe(abm,
+ otg_inst,
+ SET_ABM_PIPE_NORMAL,
+ panel_cntl->inst,
+ panel_cntl->pwrseq_inst);
}

-   if (abm && abm->funcs && abm->funcs->set_backlight_level_pwm)
+   if (abm->funcs && abm->funcs->set_backlight_level_pwm)
abm->funcs->set_backlight_level_pwm(abm, backlight_pwm_u16_16,
frame_ramp, 0, panel_cntl->inst);
else
--
2.34.1



Re: [PATCH 3/6] drm/amdgpu: prefer snprintf over sprintf

2024-02-01 Thread Jani Nikula
On Fri, 12 Jan 2024, Alex Deucher  wrote:
> On Wed, Jan 10, 2024 at 12:39 PM Jani Nikula  wrote:
>>
>> This will trade the W=1 warning -Wformat-overflow to
>> -Wformat-truncation. This lets us enable -Wformat-overflow subsystem
>> wide.
>>
>> Cc: Alex Deucher 
>> Cc: Christian König 
>> Cc: Pan, Xinhui 
>> Cc: amd-gfx@lists.freedesktop.org
>> Signed-off-by: Jani Nikula 
>
> Acked-by: Alex Deucher 
>
> Feel free to take this via whichever tree makes sense.

Thanks, pushed this one patch to drm-misc-next as prep work.

BR,
Jani.

>
> Alex
>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> index b9674c57c436..82b4b2019fca 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> @@ -329,7 +329,8 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>>
>> ring->eop_gpu_addr = kiq->eop_gpu_addr;
>> ring->no_scheduler = true;
>> -   sprintf(ring->name, "kiq_%d.%d.%d.%d", xcc_id, ring->me, ring->pipe, 
>> ring->queue);
>> +   snprintf(ring->name, sizeof(ring->name), "kiq_%d.%d.%d.%d",
>> +xcc_id, ring->me, ring->pipe, ring->queue);
>> r = amdgpu_ring_init(adev, ring, 1024, irq, 
>> AMDGPU_CP_KIQ_IRQ_DRIVER0,
>>  AMDGPU_RING_PRIO_DEFAULT, NULL);
>> if (r)
>> --
>> 2.39.2
>>

-- 
Jani Nikula, Intel


Re: [bug report] drm/amd/display: Simplify the per-CPU usage.

2024-02-01 Thread Sebastian Andrzej Siewior
On 2024-02-01 15:18:04 [+0300], Dan Carpenter wrote:
> Hello Sebastian Andrzej Siewior,
Hi Dan,

> The patch de5e73dc6baf: "drm/amd/display: Simplify the per-CPU
> usage." from Sep 21, 2023 (linux-next), leads to the following Smatch
> static checker warning:

Did I introduce that or has it been made visible?
That change adds preempt_disable() to DC_FP_START() but this was there
already, just hidden. For x86 it is done within kernel_fpu_begin().

> drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn30/dcn30_resource.c:2385 
> dcn30_resource_construct() warn: sleeping in atomic context
> drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn32/dcn32_resource.c:2136 
> dcn32_resource_construct() warn: sleeping in atomic context
> 
> drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn30/dcn30_resource.c
> 2263 static bool dcn30_resource_construct(
> 2264 uint8_t num_virtual_links,
> 2265 struct dc *dc,
> 2266 struct dcn30_resource_pool *pool)
> 2267 {
…
> 2281 
> 2282 DC_FP_START();
>  ^^
> Preempt disabled here.
…
> 2383 /* Clock Sources for Pixel Clock*/
> 2384 pool->base.clock_sources[DCN30_CLK_SRC_PLL0] =
> --> 2385 dcn30_clock_source_create(ctx, ctx->dc_bios,
>  ^^
> sleeping allocation here.
> 

Correct you are. But there is more. Later is also dccg30_create() and
dcn30_hubbub_create() and probably a few other, too.
Could we please restructure the init-phase so that the memory allocation
happen outside of DC_FP_START(). Is DC_FP_START() even needed here?

> regards,
> dan carpenter

Sebastian


Re: [bug report] drm/amd/display: Simplify the per-CPU usage.

2024-02-01 Thread Dan Carpenter
On Thu, Feb 01, 2024 at 02:53:42PM +0100, Sebastian Andrzej Siewior wrote:
> On 2024-02-01 15:18:04 [+0300], Dan Carpenter wrote:
> > Hello Sebastian Andrzej Siewior,
> Hi Dan,
> 
> > The patch de5e73dc6baf: "drm/amd/display: Simplify the per-CPU
> > usage." from Sep 21, 2023 (linux-next), leads to the following Smatch
> > static checker warning:
> 
> Did I introduce that or has it been made visible?
> That change adds preempt_disable() to DC_FP_START() but this was there
> already, just hidden. For x86 it is done within kernel_fpu_begin().
> 

Sorry, yeah, the bug was there before.  I don't know why this shows up
as a new warning.  Probably it's because AMD driver files were renamed...
Smatch parses kernel_fpu_begin() correctly and sees the preempt_disable()
but I didn't know it disables preemption so it's likely human error on
my part.

regards,
dan carpenter



[bug report] drm/amd/display: Simplify the per-CPU usage.

2024-02-01 Thread Dan Carpenter
Hello Sebastian Andrzej Siewior,

The patch de5e73dc6baf: "drm/amd/display: Simplify the per-CPU
usage." from Sep 21, 2023 (linux-next), leads to the following Smatch
static checker warning:

drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn30/dcn30_resource.c:2385 
dcn30_resource_construct() warn: sleeping in atomic context
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn32/dcn32_resource.c:2136 
dcn32_resource_construct() warn: sleeping in atomic context

drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn30/dcn30_resource.c
2263 static bool dcn30_resource_construct(
2264 uint8_t num_virtual_links,
2265 struct dc *dc,
2266 struct dcn30_resource_pool *pool)
2267 {
2268 int i;
2269 struct dc_context *ctx = dc->ctx;
2270 struct irq_service_init_data init_data;
2271 struct ddc_service_init_data ddc_init_data = {0};
2272 uint32_t pipe_fuses = read_pipe_fuses(ctx);
2273 uint32_t num_pipes = 0;
2274 
2275 if (!(pipe_fuses == 0 || pipe_fuses == 0x3e)) {
2276 BREAK_TO_DEBUGGER();
2277 dm_error("DC: Unexpected fuse recipe for navi2x !\n");
2278 /* fault to single pipe */
2279 pipe_fuses = 0x3e;
2280 }
2281 
2282 DC_FP_START();
 ^^
Preempt disabled here.

2283 
2284 ctx->dc_bios->regs = _regs;
2285 
2286 pool->base.res_cap = _cap_dcn3;
2287 
2288 pool->base.funcs = _res_pool_funcs;
2289 
2290 /*
2291  *  Resource + asic cap harcoding*
2292  */
2293 pool->base.underlay_pipe_index = NO_UNDERLAY_PIPE;
2294 pool->base.pipe_count = 
pool->base.res_cap->num_timing_generator;
2295 pool->base.mpcc_count = 
pool->base.res_cap->num_timing_generator;
2296 dc->caps.max_downscale_ratio = 600;
2297 dc->caps.i2c_speed_in_khz = 100;
2298 dc->caps.i2c_speed_in_khz_hdcp = 100; /*1.4 w/a not applied by 
default*/
2299 dc->caps.max_cursor_size = 256;
2300 dc->caps.min_horizontal_blanking_period = 80;
2301 dc->caps.dmdata_alloc_size = 2048;
2302 dc->caps.mall_size_per_mem_channel = 8;
2303 /* total size = mall per channel * num channels * 1024 * 1024 
*/
2304 dc->caps.mall_size_total = dc->caps.mall_size_per_mem_channel 
* dc->ctx->dc_bios->vram_info.num_chans * 1048576;
2305 dc->caps.cursor_cache_size = dc->caps.max_cursor_size * 
dc->caps.max_cursor_size * 8;
2306 
2307 dc->caps.max_slave_planes = 2;
2308 dc->caps.max_slave_yuv_planes = 2;
2309 dc->caps.max_slave_rgb_planes = 2;
2310 dc->caps.post_blend_color_processing = true;
2311 dc->caps.force_dp_tps4_for_cp2520 = true;
2312 dc->caps.extended_aux_timeout_support = true;
2313 dc->caps.dmcub_support = true;
2314 
2315 /* Color pipeline capabilities */
2316 dc->caps.color.dpp.dcn_arch = 1;
2317 dc->caps.color.dpp.input_lut_shared = 0;
2318 dc->caps.color.dpp.icsc = 1;
2319 dc->caps.color.dpp.dgam_ram = 0; // must use gamma_corr
2320 dc->caps.color.dpp.dgam_rom_caps.srgb = 1;
2321 dc->caps.color.dpp.dgam_rom_caps.bt2020 = 1;
2322 dc->caps.color.dpp.dgam_rom_caps.gamma2_2 = 1;
2323 dc->caps.color.dpp.dgam_rom_caps.pq = 1;
2324 dc->caps.color.dpp.dgam_rom_caps.hlg = 1;
2325 dc->caps.color.dpp.post_csc = 1;
2326 dc->caps.color.dpp.gamma_corr = 1;
2327 dc->caps.color.dpp.dgam_rom_for_yuv = 0;
2328 
2329 dc->caps.color.dpp.hw_3d_lut = 1;
2330 dc->caps.color.dpp.ogam_ram = 1;
2331 // no OGAM ROM on DCN3
2332 dc->caps.color.dpp.ogam_rom_caps.srgb = 0;
2333 dc->caps.color.dpp.ogam_rom_caps.bt2020 = 0;
2334 dc->caps.color.dpp.ogam_rom_caps.gamma2_2 = 0;
2335 dc->caps.color.dpp.ogam_rom_caps.pq = 0;
2336 dc->caps.color.dpp.ogam_rom_caps.hlg = 0;
2337 dc->caps.color.dpp.ocsc = 0;
2338 
2339 dc->caps.color.mpc.gamut_remap = 1;
2340 dc->caps.color.mpc.num_3dluts = 
pool->base.res_cap->num_mpc_3dlut; //3
2341 dc->caps.color.mpc.ogam_ram = 1;
2342 dc->caps.color.mpc.ogam_rom_caps.srgb = 0;
2343 dc->caps.color.mpc.ogam_rom_caps.bt2020 = 0;
2344 dc->caps.color.mpc.ogam_rom_caps.gamma2_2 = 0;
2345 dc->caps.color.mpc.ogam_rom_caps.pq = 0;
2346 dc->caps.color.mpc.ogam_rom_caps.hlg = 0;
2347 dc->caps.color.mpc.ocsc = 

Re: [PATCH] drm/amdgpu: fix typo in parameter description

2024-02-01 Thread Christian König

Am 11.01.24 um 16:58 schrieb Alex Deucher:

Missing space.

Signed-off-by: Alex Deucher 


Reviewed-by: Christian König 

And sorry that this took so long. I'm still trying to catch up to my mails.

Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 5c9caf5fa075..0712d5867849 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -366,7 +366,7 @@ module_param_named(aspm, amdgpu_aspm, int, 0444);
   * Setting the value to 0 disables this functionality.
   * Setting the value to -2 is auto enabled with power down when displays are 
attached.
   */
-MODULE_PARM_DESC(runpm, "PX runtime pm (2 = force enable with BAMACO, 1 = force 
enable with BACO, 0 = disable, -1 = auto, -2 = autowith displays)");
+MODULE_PARM_DESC(runpm, "PX runtime pm (2 = force enable with BAMACO, 1 = force 
enable with BACO, 0 = disable, -1 = auto, -2 = auto with displays)");
  module_param_named(runpm, amdgpu_runtime_pm, int, 0444);
  
  /**




Re: [PATCH v2 3/3] drm/amdgpu: sync page table freeing with tlb flush

2024-02-01 Thread Christian König




Am 31.01.24 um 18:14 schrieb Shashank Sharma:

This patch:
- Attaches the TLB flush fence to the PT objects being freed
- Adds a new ptr in VM to save this last TLB flush fence
- Adds a new lock in VM to prevent out-of-context update of saved
   TLB flush fence
- Adds a new ptr in tlb_flush structure to save vm

The idea is to delay freeing of page table objects until we have the
respective TLB entries flushed.

V2: rebase

Cc: Christian König 
Cc: Alex Deucher 
Cc: Felix Kuehling 
Cc: Rajneesh Bhardwaj 
Signed-off-by: Shashank Sharma 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c|  3 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h|  4 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 27 +++
  .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  | 13 +++--
  4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 67c690044b97..b0e81c249e3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2245,6 +2245,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
vm->generation = 0;
  
  	mutex_init(>eviction_lock);

+   mutex_init(>tlb_flush_lock);
vm->evicting = false;
vm->tlb_fence_context = dma_fence_context_alloc(1);
  
@@ -2360,7 +2361,9 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)

}
  
  	dma_fence_put(vm->last_update);

+   dma_fence_put(vm->tlb_fence_last);
vm->last_update = dma_fence_get_stub();
+   vm->tlb_fence_last = dma_fence_get_stub();
vm->is_compute_context = true;
  
  	/* Free the shadow bo for compute VM */

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 8e6fd25d07b7..b05bc586237f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -334,6 +334,10 @@ struct amdgpu_vm {
uint64_t*tlb_seq_cpu_addr;
uint64_ttlb_fence_context;
  
+	/* Ptr and lock to maintain tlb flush sync */

+   struct mutextlb_flush_lock;
+   struct dma_fence*tlb_fence_last;
+
atomic64_t  kfd_last_flushed_seq;
  
  	/* How many times we had to re-generate the page tables */

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index 95dc0afdaffb..f1c4418c4d63 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -631,6 +631,18 @@ static int amdgpu_vm_pt_alloc(struct amdgpu_device *adev,
return r;
  }
  
+static inline

+void amdgpu_vm_attach_tlb_fence(struct amdgpu_bo *bo, struct dma_fence *fence)
+{
+   if (!bo || !fence)
+   return;
+
+   if (!dma_resv_reserve_fences(bo->tbo.base.resv, 1)) {
+   dma_resv_add_fence(bo->tbo.base.resv, fence,
+  DMA_RESV_USAGE_BOOKKEEP);
+   }
+}
+
  /**
   * amdgpu_vm_pt_free - free one PD/PT
   *
@@ -638,6 +650,7 @@ static int amdgpu_vm_pt_alloc(struct amdgpu_device *adev,
   */
  static void amdgpu_vm_pt_free(struct amdgpu_vm_bo_base *entry)
  {
+   struct amdgpu_vm *vm;
struct amdgpu_bo *shadow;
  
  	if (!entry->bo)

@@ -646,9 +659,23 @@ static void amdgpu_vm_pt_free(struct amdgpu_vm_bo_base 
*entry)
entry->bo->vm_bo = NULL;
shadow = amdgpu_bo_shadowed(entry->bo);
if (shadow) {
+   vm = shadow->vm_bo->vm;
+
+   mutex_lock(>tlb_flush_lock);
+   if (vm->tlb_fence_last)
+   amdgpu_vm_attach_tlb_fence(shadow, vm->tlb_fence_last);
+   mutex_unlock(>tlb_flush_lock);
+
ttm_bo_set_bulk_move(>tbo, NULL);
amdgpu_bo_unref();
}
+
+   vm = entry->vm;
+   mutex_lock(>tlb_flush_lock);
+   if (vm->tlb_fence_last)
+   amdgpu_vm_attach_tlb_fence(entry->bo, vm->tlb_fence_last);
+   mutex_unlock(>tlb_flush_lock);
+


That approach doesn't make sense. Instead add the freed PT/PDs to a 
linked list in the parameters structure and only really free them after 
adding the fence to the root PD.




ttm_bo_set_bulk_move(>bo->tbo, NULL);
  
  	spin_lock(>vm->status_lock);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
index 569681badd7c..54ec81d30034 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
@@ -31,6 +31,7 @@
  struct amdgpu_tlb_fence {
struct dma_fencebase;
struct amdgpu_device*adev;
+   struct amdgpu_vm*vm;


Big NAK to that. The VM might not live long enough to see the end of the 
TLB flush.


Regards,
Christian.


struct dma_fence*dependency;
struct work_struct  work;

RE: [PATCH] drm/amdgpu: remove asymmetrical irq disabling in jpeg 4.0.5 suspend

2024-02-01 Thread Jamadar, Saleemkhan
[AMD Official Use Only - General]

Acked-By: Saleemkhan Jamadar 

-Original Message-
From: Ma, Li 
Sent: Thursday, February 1, 2024 5:25 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Jamadar, Saleemkhan 
; Gopalakrishnan, Veerabadhran (Veera) 
; Zhang, Yifan ; Ma, 
Li 
Subject: [PATCH] drm/amdgpu: remove asymmetrical irq disabling in jpeg 4.0.5 
suspend

A supplement to commit: 45fa6f32276f7ce571227f8368cf17378b804aa1
There is an irq warning of jpeg during resume in s2idle process. No irq enabled 
in jpeg 4.0.5 resume.

Signed-off-by: Li Ma 
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c   |  9 -
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c | 10 --
 2 files changed, 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
index bc38b90f8cf8..88ea58d5c4ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
@@ -674,14 +674,6 @@ static int jpeg_v4_0_set_powergating_state(void *handle,
return ret;
 }

-static int jpeg_v4_0_set_interrupt_state(struct amdgpu_device *adev,
-   struct amdgpu_irq_src *source,
-   unsigned type,
-   enum amdgpu_interrupt_state state)
-{
-   return 0;
-}
-
 static int jpeg_v4_0_set_ras_interrupt_state(struct amdgpu_device *adev,
struct amdgpu_irq_src *source,
unsigned int type,
@@ -765,7 +757,6 @@ static void jpeg_v4_0_set_dec_ring_funcs(struct 
amdgpu_device *adev)  }

 static const struct amdgpu_irq_src_funcs jpeg_v4_0_irq_funcs = {
-   .set = jpeg_v4_0_set_interrupt_state,
.process = jpeg_v4_0_process_interrupt,  };

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c 
b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
index 6ede85b28cc8..78b74daf4eeb 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
@@ -181,7 +181,6 @@ static int jpeg_v4_0_5_hw_fini(void *handle)
RREG32_SOC15(JPEG, 0, regUVD_JRBC_STATUS))
jpeg_v4_0_5_set_powergating_state(adev, 
AMD_PG_STATE_GATE);
}
-   amdgpu_irq_put(adev, >jpeg.inst->irq, 0);

return 0;
 }
@@ -516,14 +515,6 @@ static int jpeg_v4_0_5_set_powergating_state(void *handle,
return ret;
 }

-static int jpeg_v4_0_5_set_interrupt_state(struct amdgpu_device *adev,
-   struct amdgpu_irq_src *source,
-   unsigned type,
-   enum amdgpu_interrupt_state state)
-{
-   return 0;
-}
-
 static int jpeg_v4_0_5_process_interrupt(struct amdgpu_device *adev,
  struct amdgpu_irq_src *source,
  struct amdgpu_iv_entry *entry) @@ -603,7 
+594,6 @@ static void jpeg_v4_0_5_set_dec_ring_funcs(struct amdgpu_device 
*adev)  }

 static const struct amdgpu_irq_src_funcs jpeg_v4_0_5_irq_funcs = {
-   .set = jpeg_v4_0_5_set_interrupt_state,
.process = jpeg_v4_0_5_process_interrupt,  };

--
2.25.1



[PATCH] drm/amdgpu: remove asymmetrical irq disabling in jpeg 4.0.5 suspend

2024-02-01 Thread Li Ma
A supplement to commit: 45fa6f32276f7ce571227f8368cf17378b804aa1
There is an irq warning of jpeg during resume in s2idle process. No irq enabled 
in jpeg 4.0.5 resume.

Signed-off-by: Li Ma 
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c   |  9 -
 drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c | 10 --
 2 files changed, 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
index bc38b90f8cf8..88ea58d5c4ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c
@@ -674,14 +674,6 @@ static int jpeg_v4_0_set_powergating_state(void *handle,
return ret;
 }
 
-static int jpeg_v4_0_set_interrupt_state(struct amdgpu_device *adev,
-   struct amdgpu_irq_src *source,
-   unsigned type,
-   enum amdgpu_interrupt_state state)
-{
-   return 0;
-}
-
 static int jpeg_v4_0_set_ras_interrupt_state(struct amdgpu_device *adev,
struct amdgpu_irq_src *source,
unsigned int type,
@@ -765,7 +757,6 @@ static void jpeg_v4_0_set_dec_ring_funcs(struct 
amdgpu_device *adev)
 }
 
 static const struct amdgpu_irq_src_funcs jpeg_v4_0_irq_funcs = {
-   .set = jpeg_v4_0_set_interrupt_state,
.process = jpeg_v4_0_process_interrupt,
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c 
b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
index 6ede85b28cc8..78b74daf4eeb 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c
@@ -181,7 +181,6 @@ static int jpeg_v4_0_5_hw_fini(void *handle)
RREG32_SOC15(JPEG, 0, regUVD_JRBC_STATUS))
jpeg_v4_0_5_set_powergating_state(adev, 
AMD_PG_STATE_GATE);
}
-   amdgpu_irq_put(adev, >jpeg.inst->irq, 0);
 
return 0;
 }
@@ -516,14 +515,6 @@ static int jpeg_v4_0_5_set_powergating_state(void *handle,
return ret;
 }
 
-static int jpeg_v4_0_5_set_interrupt_state(struct amdgpu_device *adev,
-   struct amdgpu_irq_src *source,
-   unsigned type,
-   enum amdgpu_interrupt_state state)
-{
-   return 0;
-}
-
 static int jpeg_v4_0_5_process_interrupt(struct amdgpu_device *adev,
  struct amdgpu_irq_src *source,
  struct amdgpu_iv_entry *entry)
@@ -603,7 +594,6 @@ static void jpeg_v4_0_5_set_dec_ring_funcs(struct 
amdgpu_device *adev)
 }
 
 static const struct amdgpu_irq_src_funcs jpeg_v4_0_5_irq_funcs = {
-   .set = jpeg_v4_0_5_set_interrupt_state,
.process = jpeg_v4_0_5_process_interrupt,
 };
 
-- 
2.25.1



[PATCH v2] drm/amd/display: Add NULL test for 'timing generator' in 'dcn21_set_pipe()'

2024-02-01 Thread Srinivasan Shanmugam
In "u32 otg_inst = pipe_ctx->stream_res.tg->inst;"
pipe_ctx->stream_res.tg could be NULL, it is relying on the caller to
ensure the tg is not NULL.

Fixes: 474ac4a875ca ("drm/amd/display: Implement some asic specific abm call 
backs.")
Cc: Yongqiang Sun 
Cc: Anthony Koo 
Cc: Rodrigo Siqueira 
Cc: Aurabindo Pillai 
Signed-off-by: Srinivasan Shanmugam 
---
v2:
  - s/u32/uint32_t for consistency (Anthony)

 .../amd/display/dc/hwss/dcn21/dcn21_hwseq.c   | 24 +++
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
index 8e88dcaf88f5..8323077bba15 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
@@ -206,28 +206,32 @@ void dcn21_set_abm_immediate_disable(struct pipe_ctx 
*pipe_ctx)
 void dcn21_set_pipe(struct pipe_ctx *pipe_ctx)
 {
struct abm *abm = pipe_ctx->stream_res.abm;
-   uint32_t otg_inst = pipe_ctx->stream_res.tg->inst;
+   struct timing_generator *tg = pipe_ctx->stream_res.tg;
struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
struct dmcu *dmcu = pipe_ctx->stream->ctx->dc->res_pool->dmcu;
+   uint32_t otg_inst;
+
+   if (!abm && !tg && !panel_cntl)
+   return;
+
+   otg_inst = tg->inst;
 
if (dmcu) {
dce110_set_pipe(pipe_ctx);
return;
}
 
-   if (abm && panel_cntl) {
-   if (abm->funcs && abm->funcs->set_pipe_ex) {
-   abm->funcs->set_pipe_ex(abm,
+   if (abm->funcs && abm->funcs->set_pipe_ex) {
+   abm->funcs->set_pipe_ex(abm,
otg_inst,
SET_ABM_PIPE_NORMAL,
panel_cntl->inst,
panel_cntl->pwrseq_inst);
-   } else {
-   dmub_abm_set_pipe(abm, otg_inst,
-   SET_ABM_PIPE_NORMAL,
-   panel_cntl->inst,
-   panel_cntl->pwrseq_inst);
-   }
+   } else {
+   dmub_abm_set_pipe(abm, otg_inst,
+ SET_ABM_PIPE_NORMAL,
+ panel_cntl->inst,
+ panel_cntl->pwrseq_inst);
}
 }
 
-- 
2.34.1



[PATCH v3] drm/amd/display: Fix 'panel_cntl' could be null in 'dcn21_set_backlight_level()'

2024-02-01 Thread Srinivasan Shanmugam
'panel_cntl' structure used to control the display panel could be null,
dereferencing it could lead to a null pointer access.

Fixes the below:
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn21/dcn21_hwseq.c:269 
dcn21_set_backlight_level() error: we previously assumed 'panel_cntl' could be 
null (see line 250)

Fixes: 474ac4a875ca ("drm/amd/display: Implement some asic specific abm call 
backs.")
Cc: Yongqiang Sun 
Cc: Anthony Koo 
Cc: Rodrigo Siqueira 
Cc: Aurabindo Pillai 
Signed-off-by: Srinivasan Shanmugam 
---
v3:
 - s/u32/uint32_t for consistency (Anthony)

 .../amd/display/dc/hwss/dcn21/dcn21_hwseq.c   | 39 ++-
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
index 8323077bba15..5c7f380a84f9 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c
@@ -241,34 +241,35 @@ bool dcn21_set_backlight_level(struct pipe_ctx *pipe_ctx,
 {
struct dc_context *dc = pipe_ctx->stream->ctx;
struct abm *abm = pipe_ctx->stream_res.abm;
+   struct timing_generator *tg = pipe_ctx->stream_res.tg;
struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
+   uint32_t otg_inst;
+
+   if (!abm && !tg && !panel_cntl)
+   return false;
+
+   otg_inst = tg->inst;
 
if (dc->dc->res_pool->dmcu) {
dce110_set_backlight_level(pipe_ctx, backlight_pwm_u16_16, 
frame_ramp);
return true;
}
 
-   if (abm != NULL) {
-   uint32_t otg_inst = pipe_ctx->stream_res.tg->inst;
-
-   if (abm && panel_cntl) {
-   if (abm->funcs && abm->funcs->set_pipe_ex) {
-   abm->funcs->set_pipe_ex(abm,
-   otg_inst,
-   SET_ABM_PIPE_NORMAL,
-   panel_cntl->inst,
-   panel_cntl->pwrseq_inst);
-   } else {
-   dmub_abm_set_pipe(abm,
-   otg_inst,
-   SET_ABM_PIPE_NORMAL,
-   panel_cntl->inst,
-   
panel_cntl->pwrseq_inst);
-   }
-   }
+   if (abm->funcs && abm->funcs->set_pipe_ex) {
+   abm->funcs->set_pipe_ex(abm,
+   otg_inst,
+   SET_ABM_PIPE_NORMAL,
+   panel_cntl->inst,
+   panel_cntl->pwrseq_inst);
+   } else {
+   dmub_abm_set_pipe(abm,
+ otg_inst,
+ SET_ABM_PIPE_NORMAL,
+ panel_cntl->inst,
+ panel_cntl->pwrseq_inst);
}
 
-   if (abm && abm->funcs && abm->funcs->set_backlight_level_pwm)
+   if (abm->funcs && abm->funcs->set_backlight_level_pwm)
abm->funcs->set_backlight_level_pwm(abm, backlight_pwm_u16_16,
frame_ramp, 0, panel_cntl->inst);
else
-- 
2.34.1



[PATCH 1/2] drm/amdgpu: skip to program GFXDEC registers for suspend abort

2024-02-01 Thread Prike Liang
In the suspend abort cases, the gfx power rail doesn't turn off so
some GFXDEC registers/CSB can't reset to default value and at this
moment reinitialize GFXDEC/CSB will result in an unexpected error.
So let skip those program sequence for the suspend abort case.

Signed-off-by: Prike Liang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 8 
 3 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index c5f3859fd682..312dfaec7b4a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1079,6 +1079,8 @@ struct amdgpu_device {
boolin_s3;
boolin_s4;
boolin_s0ix;
+   /* indicate amdgpu suspension status */
+   boolsuspend_complete;
 
enum pp_mp1_state   mp1_state;
struct amdgpu_doorbell_index doorbell_index;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 475bd59c9ac2..59254144916c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2472,6 +2472,7 @@ static int amdgpu_pmops_suspend(struct device *dev)
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(drm_dev);
 
+   adev->suspend_complete = false;
if (amdgpu_acpi_is_s0ix_active(adev))
adev->in_s0ix = true;
else if (amdgpu_acpi_is_s3_active(adev))
@@ -2486,6 +2487,7 @@ static int amdgpu_pmops_suspend_noirq(struct device *dev)
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(drm_dev);
 
+   adev->suspend_complete = true;
if (amdgpu_acpi_should_gpu_reset(adev))
return amdgpu_asic_reset(adev);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 57808be6e3ec..169d45268ef6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -3034,6 +3034,14 @@ static int gfx_v9_0_cp_gfx_start(struct amdgpu_device 
*adev)
 
gfx_v9_0_cp_gfx_enable(adev, true);
 
+   /* Now only limit the quirk on the APU gfx9 series and already
+* confirmed that the APU gfx10/gfx11 needn't such update.
+*/
+   if (adev->flags & AMD_IS_APU &&
+   adev->in_s3 && !adev->suspend_complete) {
+   DRM_INFO(" Will skip the CSB packet resubmit\n");
+   return 0;
+   }
r = amdgpu_ring_alloc(ring, gfx_v9_0_get_csb_size(adev) + 4 + 3);
if (r) {
DRM_ERROR("amdgpu: cp failed to lock ring (%d).\n", r);
-- 
2.34.1



[PATCH 2/2] drm/amdgpu: reset gpu for s3 suspend abort case

2024-02-01 Thread Prike Liang
In the s3 suspend abort case some type of gfx9 power
rail not turn off from FCH side and this will put the
GPU in an unknown power status, so let's reset the gpu
to a known good power state before reinitialize gpu
device.

Signed-off-by: Prike Liang 
---
 drivers/gpu/drm/amd/amdgpu/soc15.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 15033efec2ba..c64c01e2944a 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -1298,10 +1298,32 @@ static int soc15_common_suspend(void *handle)
return soc15_common_hw_fini(adev);
 }
 
+static bool soc15_need_reset_on_resume(struct amdgpu_device *adev)
+{
+   u32 sol_reg;
+
+   sol_reg = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_81);
+
+   /* Will reset for the following suspend abort cases.
+* 1) Only reset limit on APU side, dGPU hasn't checked yet.
+* 2) S3 suspend abort and TOS already launched.
+*/
+   if (adev->flags & AMD_IS_APU && adev->in_s3 &&
+   !adev->suspend_complete &&
+   sol_reg)
+   return true;
+
+   return false;
+}
+
 static int soc15_common_resume(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+   if (soc15_need_reset_on_resume(adev)) {
+   dev_info(adev->dev, "S3 suspend abort case, let's reset 
ASIC.\n");
+   soc15_asic_reset(adev);
+   }
return soc15_common_hw_init(adev);
 }
 
-- 
2.34.1