Re: [PATCH] drm: deprecate driver date

[Hamza Mahfooz]

On 4/29/24 12:43, Jani Nikula wrote:

The driver date serves no useful purpose, because it's hardly ever
updated. The information is misleading at best.

As described in Documentation/gpu/drm-internals.rst:

   The driver date, formatted as MMDD, is meant to identify the date
   of the latest modification to the driver. However, as most drivers
   fail to update it, its value is mostly useless. The DRM core prints it
   to the kernel log at initialization time and passes it to userspace
   through the DRM_IOCTL_VERSION ioctl.

Stop printing the driver date at init, and start returning the empty
string "" as driver date through the DRM_IOCTL_VERSION ioctl.

The driver date initialization in drivers and the struct drm_driver date
member can be removed in follow-up.

Signed-off-by: Jani Nikula 


I would prefer if it was dropped entirely in this patch, but if you feel
that would require too much back and forth, I'm okay with what is
currently proposed.

Reviewed-by: Hamza Mahfooz 



---

The below approximates when each driver's date was last updated.

$ git grepblame "\(\.date = \".*\"\|#define.*DRIVER_DATE\)" -- drivers/gpu 
drivers/accel
fe77368c0f3e0 drivers/accel/habanalabs/common/habanalabs_drv.c 94 (Tomer Tayar 2023-02-19 
11:58:46 +0200 104)   .date = "20190505",
35b137630f08d drivers/accel/ivpu/ivpu_drv.h 20 (Jacek Lawrynowicz 2023-01-17 10:27:17 
+0100 24) #define DRIVER_DATE "20230117"
d38ceaf99ed01 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.h 43 (Alex Deucher 2015-04-20 
16:55:21 -0400 43) #define DRIVER_DATE"20150101"
61f1c4a8ab757 drivers/gpu/drm/arm/display/komeda/komeda_kms.c 51 (james qian wang (Arm 
Technology China) 2019-01-03 11:41:30 + 64)  .date = "20181101",
8e22d79240d95 drivers/gpu/drm/arm/hdlcd_drv.c 343 (Liviu Dudau 2015-04-02 19:48:39 +0100 
234)   .date = "20151021",
ad49f8602fe88 drivers/gpu/drm/arm/malidp_drv.c 232 (Liviu Dudau 2016-03-07 10:00:53 + 
571)  .date = "20160106",
4f2a8f5898ecd drivers/gpu/drm/aspeed/aspeed_gfx_drv.c 208 (Joel Stanley 2019-04-03 
10:49:08 +1030 253)  .date = "20180319",
312fec1405dd5 drivers/gpu/drm/ast/ast_drv.h 46 (Dave Airlie 2012-02-29 13:40:04 + 46) 
#define DRIVER_DATE   "20120228"
1a396789f65a2 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 504 (Boris Brezillon 
2015-01-06 11:13:28 +0100 741)  .date = "20141504",
9913f74fe1570 drivers/gpu/drm/exynos/exynos_drm_drv.c 37 (Marek Szyprowski 2018-05-10 
08:46:36 +0900 37) #define DRIVER_DATE"20180330"
f90cd811ae7a3 drivers/gpu/drm/gma500/psb_drv.h 43 (Arthur Borsboom 2014-03-15 22:12:17 
+0100 29) #define DRIVER_DATE "20140314"
1053d01864937 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 1070 (Xu YiPing 2019-08-20 
23:06:19 + 930).date = "20150718",
76c56a5affeba drivers/gpu/drm/hyperv/hyperv_drm_drv.c 22 (Deepak Rawat 2021-05-27 
04:22:28 -0700 22) #define DRIVER_DATE "2020"
3570bd989acc6 drivers/gpu/drm/i915/i915_driver.h 18 (Jani Nikula 2023-09-29 12:43:23 
+0300 18) #define DRIVER_DATE  "20230929"
4babef0708656 drivers/gpu/drm/imagination/pvr_drv.h 12 (Sarah Walker 2023-11-22 16:34:26 
+ 12) #define PVR_DRIVER_DATE "20230904"
c87e859cdeb5d drivers/gpu/drm/imx/lcdc/imx-lcdc.c 361 (Marian Cichy 2023-03-06 12:52:49 
+0100 353)  .date = "20200716",
eb92830cdbc23 drivers/gpu/drm/kmb/kmb_drv.h 19 (Edmund Dea 2020-08-26 13:17:29 -0700 19) 
#define DRIVER_DATE"20210223"
f39db26c54281 drivers/gpu/drm/loongson/lsdc_drv.c 28 (Sui Jingfeng 2023-06-15 22:36:12 
+0800 28) #define DRIVER_DATE "20220701"
5fc537bfd0003 drivers/gpu/drm/mcde/mcde_drv.c 247 (Linus Walleij 2019-05-24 11:20:19 
+0200 210) .date = "20180529",
119f5173628aa drivers/gpu/drm/mediatek/mtk_drm_drv.c 36 (CK Hu 2016-01-04 18:36:34 +0100 
34) #define DRIVER_DATE "20150513"
414c453106255 drivers/gpu/drm/mgag200/mgag200_drv.h 34 (Dave Airlie 2012-04-17 15:01:25 
+0100 31) #define DRIVER_DATE   "20110418"
77145f1cbdf8d drivers/gpu/drm/nouveau/nouveau_drm.h 9 (Ben Skeggs 2012-07-31 16:16:21 
+1000 10) #define DRIVER_DATE "20120801"
cd5351f4d2b1b drivers/staging/omapdrm/omap_drv.c 27 (Rob Clark 2011-11-12 12:09:40 -0600 
31) #define DRIVER_DATE"20110917"
4bdca11507928 drivers/gpu/drm/panthor/panthor_drv.c 1371 (Boris Brezillon 2024-02-29 
17:22:25 +0100 1386)   .date = "20230801",
bed41005e6174 drivers/gpu/drm/pl111/pl111_drv.c 157 (Tom Cooksey 2017-04-12 20:17:46 
-0700 222) .date = "20170317",
f64122c1f6ade drivers/gpu/drm/qxl/qxl_drv.h 52 (Dave Airlie 2013-02-25 14:47:55 +1000 57) 
#define DRIVER_DATE   "20120117"
c0beb2a723d69 drivers/char/drm/radeon_drv.h 41 (Dave Airlie 2008-05-28 13:52:28

Re: [PATCH 01/12] kbuild: make -Woverride-init warnings more consistent

[Hamza Mahfooz]

Cc: amd-...@lists.freedesktop.org

On 3/26/24 10:47, Arnd Bergmann wrote:

From: Arnd Bergmann 

The -Woverride-init warn about code that may be intentional or not,
but the inintentional ones tend to be real bugs, so there is a bit of
disagreement on whether this warning option should be enabled by default
and we have multiple settings in scripts/Makefile.extrawarn as well as
individual subsystems.

Older versions of clang only supported -Wno-initializer-overrides with
the same meaning as gcc's -Woverride-init, though all supported versions
now work with both. Because of this difference, an earlier cleanup of
mine accidentally turned the clang warning off for W=1 builds and only
left it on for W=2, while it's still enabled for gcc with W=1.

There is also one driver that only turns the warning off for newer
versions of gcc but not other compilers, and some but not all the
Makefiles still use a cc-disable-warning conditional that is no
longer needed with supported compilers here.

Address all of the above by removing the special cases for clang
and always turning the warning off unconditionally where it got
in the way, using the syntax that is supported by both compilers.

Fixes: 2cd3271b7a31 ("kbuild: avoid duplicate warning options")
Signed-off-by: Arnd Bergmann 


Acked-by: Hamza Mahfooz 

For the amdgpu changes.


---
  drivers/gpu/drm/amd/display/dc/dce110/Makefile |  2 +-
  drivers/gpu/drm/amd/display/dc/dce112/Makefile |  2 +-
  drivers/gpu/drm/amd/display/dc/dce120/Makefile |  2 +-
  drivers/gpu/drm/amd/display/dc/dce60/Makefile  |  2 +-
  drivers/gpu/drm/amd/display/dc/dce80/Makefile  |  2 +-
  drivers/gpu/drm/i915/Makefile  |  6 +++---
  drivers/gpu/drm/xe/Makefile|  4 ++--
  drivers/net/ethernet/renesas/sh_eth.c  |  2 +-
  drivers/pinctrl/aspeed/Makefile|  2 +-
  fs/proc/Makefile   |  2 +-
  kernel/bpf/Makefile|  2 +-
  mm/Makefile|  3 +--
  scripts/Makefile.extrawarn | 10 +++---
  13 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/Makefile 
b/drivers/gpu/drm/amd/display/dc/dce110/Makefile
index f0777d61c2cb..c307f040e48f 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dce110/Makefile
@@ -23,7 +23,7 @@
  # Makefile for the 'controller' sub-component of DAL.
  # It provides the control and status of HW CRTC block.
  
-CFLAGS_$(AMDDALPATH)/dc/dce110/dce110_resource.o = $(call cc-disable-warning, override-init)

+CFLAGS_$(AMDDALPATH)/dc/dce110/dce110_resource.o = -Wno-override-init
  
  DCE110 = dce110_timing_generator.o \

  dce110_compressor.o dce110_opp_regamma_v.o \
diff --git a/drivers/gpu/drm/amd/display/dc/dce112/Makefile 
b/drivers/gpu/drm/amd/display/dc/dce112/Makefile
index 7e92effec894..683866797709 100644
--- a/drivers/gpu/drm/amd/display/dc/dce112/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dce112/Makefile
@@ -23,7 +23,7 @@
  # Makefile for the 'controller' sub-component of DAL.
  # It provides the control and status of HW CRTC block.
  
-CFLAGS_$(AMDDALPATH)/dc/dce112/dce112_resource.o = $(call cc-disable-warning, override-init)

+CFLAGS_$(AMDDALPATH)/dc/dce112/dce112_resource.o = -Wno-override-init
  
  DCE112 = dce112_compressor.o
  
diff --git a/drivers/gpu/drm/amd/display/dc/dce120/Makefile b/drivers/gpu/drm/amd/display/dc/dce120/Makefile

index 1e3ef68a452a..8f508e662748 100644
--- a/drivers/gpu/drm/amd/display/dc/dce120/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dce120/Makefile
@@ -24,7 +24,7 @@
  # It provides the control and status of HW CRTC block.
  
  
-CFLAGS_$(AMDDALPATH)/dc/dce120/dce120_resource.o = $(call cc-disable-warning, override-init)

+CFLAGS_$(AMDDALPATH)/dc/dce120/dce120_resource.o = -Wno-override-init
  
  DCE120 = dce120_timing_generator.o
  
diff --git a/drivers/gpu/drm/amd/display/dc/dce60/Makefile b/drivers/gpu/drm/amd/display/dc/dce60/Makefile

index fee331accc0e..eede83ad91fa 100644
--- a/drivers/gpu/drm/amd/display/dc/dce60/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dce60/Makefile
@@ -23,7 +23,7 @@
  # Makefile for the 'controller' sub-component of DAL.
  # It provides the control and status of HW CRTC block.
  
-CFLAGS_$(AMDDALPATH)/dc/dce60/dce60_resource.o = $(call cc-disable-warning, override-init)

+CFLAGS_$(AMDDALPATH)/dc/dce60/dce60_resource.o = -Wno-override-init
  
  DCE60 = dce60_timing_generator.o dce60_hw_sequencer.o \

dce60_resource.o
diff --git a/drivers/gpu/drm/amd/display/dc/dce80/Makefile 
b/drivers/gpu/drm/amd/display/dc/dce80/Makefile
index 7eefffbdc925..fba189d26652 100644
--- a/drivers/gpu/drm/amd/display/dc/dce80/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dce80/Makefile
@@ -23,7 +23,7 @@
  # Makefile for the 'controller' sub-component of DAL.
  # It provides the control and status of HW CRTC block.
  
-CFLAGS_$(AMDDALPATH

Re: [PATCH] drm/amd/display: Add monitor patch for specific eDP

[Hamza Mahfooz]

On 2/27/24 13:18, Rodrigo Siqueira wrote:

From: Ivan Lipski 

[WHY]
Some eDP panels's ext caps don't write initial value cause the value of
dpcd_addr(0x317) is random.  It means that sometimes the eDP will
clarify it is OLED, miniLED...etc cause the backlight control interface
is incorrect.

[HOW]
Add a new panel patch to remove sink ext caps(HDR,OLED...etc)


I wonder if it would make sense to turn this into a DPCD qurik (see
drivers/gpu/drm/display/drm_dp_helper.c). Since, it is rather unsettling
that we have so many panel quirks in our driver.



Cc: sta...@vger.kernel.org # 6.5.x
Cc: Hamza Mahfooz 
Cc: Tsung-hua Lin 
Cc: Chris Chi 
Cc: Harry Wentland 
Tested-by: Daniel Wheeler 
Reviewed-by: Sun peng Li 
Acked-by: Rodrigo Siqueira 
Signed-off-by: Ivan Lipski 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 ++
  1 file changed, 6 insertions(+)

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 d9a482908380..764dc3ffd91b 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
@@ -63,6 +63,12 @@ static void apply_edid_quirks(struct edid *edid, struct 
dc_edid_caps *edid_caps)
DRM_DEBUG_DRIVER("Disabling FAMS on monitor with panel id 
%X\n", panel_id);
edid_caps->panel_patch.disable_fams = true;
break;
+   /* Workaround for some monitors that do not clear DPCD 0x317 if 
FreeSync is unsupported */
+   case drm_edid_encode_panel_id('A', 'U', 'O', 0xA7AB):
+   case drm_edid_encode_panel_id('A', 'U', 'O', 0xE69B):
+   DRM_DEBUG_DRIVER("Clearing DPCD 0x317 on monitor with panel id 
%X\n", panel_id);
+   edid_caps->panel_patch.remove_sink_ext_caps = true;
+   break;
default:
return;
}

--
Hamza

Re: [PATCH v2] drm/amd/display: add panel_power_savings sysfs entry to eDP connectors

[Hamza Mahfooz]

On 2/16/24 03:19, Pekka Paalanen wrote:

On Fri, 2 Feb 2024 10:28:35 -0500
Hamza Mahfooz  wrote:


We want programs besides the compositor to be able to enable or disable
panel power saving features.


Could you also explain why, in the commit message, please?

It is unexpected for arbitrary programs to be able to override the KMS
client, and certainly new ways to do so should not be added without an
excellent justification.


Also, to be completely honest with you, I'm not sure why it was
initially exposed as a DRM prop, since it's a power management feature.
Which is to say, that it doesn't really make sense to have the
compositor control it.



Maybe debugfs would be more appropriate if the purpose is only testing
rather than production environments?


However, since they are currently only
configurable through DRM properties, that isn't possible. So, to remedy
that issue introduce a new "panel_power_savings" sysfs attribute.


When the DRM property was added, what was used as the userspace to
prove its workings?


Thanks,
pq



Cc: Mario Limonciello 
Signed-off-by: Hamza Mahfooz 
---
v2: hide ABM_LEVEL_IMMEDIATE_DISABLE in the read case, force an atomic
 commit when setting the value, call sysfs_remove_group() in
 amdgpu_dm_connector_unregister() and add some documentation.
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 76 +++
  1 file changed, 76 insertions(+)

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 8590c9f1dda6..3c62489d03dc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6436,10 +6436,79 @@ int amdgpu_dm_connector_atomic_get_property(struct 
drm_connector *connector,
return ret;
  }
  
+/**

+ * DOC: panel power savings
+ *
+ * The display manager allows you to set your desired **panel power savings**
+ * level (between 0-4, with 0 representing off), e.g. using the following::
+ *
+ *   # echo 3 > /sys/class/drm/card0-eDP-1/amdgpu/panel_power_savings
+ *
+ * Modifying this value can have implications on color accuracy, so tread
+ * carefully.
+ */
+
+static ssize_t panel_power_savings_show(struct device *device,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_connector *connector = dev_get_drvdata(device);
+   struct drm_device *dev = connector->dev;
+   u8 val;
+
+   drm_modeset_lock(>mode_config.connection_mutex, NULL);
+   val = to_dm_connector_state(connector->state)->abm_level ==
+   ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
+   to_dm_connector_state(connector->state)->abm_level;
+   drm_modeset_unlock(>mode_config.connection_mutex);
+
+   return sysfs_emit(buf, "%u\n", val);
+}
+
+static ssize_t panel_power_savings_store(struct device *device,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct drm_connector *connector = dev_get_drvdata(device);
+   struct drm_device *dev = connector->dev;
+   long val;
+   int ret;
+
+   ret = kstrtol(buf, 0, );
+
+   if (ret)
+   return ret;
+
+   if (val < 0 || val > 4)
+   return -EINVAL;
+
+   drm_modeset_lock(>mode_config.connection_mutex, NULL);
+   to_dm_connector_state(connector->state)->abm_level = val ?:
+   ABM_LEVEL_IMMEDIATE_DISABLE;
+   drm_modeset_unlock(>mode_config.connection_mutex);
+
+   drm_kms_helper_hotplug_event(dev);
+
+   return count;
+}
+
+static DEVICE_ATTR_RW(panel_power_savings);
+
+static struct attribute *amdgpu_attrs[] = {
+   _attr_panel_power_savings.attr,
+   NULL
+};
+
+static const struct attribute_group amdgpu_group = {
+   .name = "amdgpu",
+   .attrs = amdgpu_attrs
+};
+
  static void amdgpu_dm_connector_unregister(struct drm_connector *connector)
  {
struct amdgpu_dm_connector *amdgpu_dm_connector = 
to_amdgpu_dm_connector(connector);
  
+	sysfs_remove_group(>kdev->kobj, _group);

drm_dp_aux_unregister(_dm_connector->dm_dp_aux.aux);
  }
  
@@ -6541,6 +6610,13 @@ amdgpu_dm_connector_late_register(struct drm_connector *connector)

to_amdgpu_dm_connector(connector);
int r;
  
+	if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {

+   r = sysfs_create_group(>kdev->kobj,
+  _group);
+   if (r)
+   return r;
+   }
+
amdgpu_dm_register_backlight_device(amdgpu_dm_connector);
  
  	if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) ||



--
Hamza

Re: [PATCH v2] drm/amd/display: add panel_power_savings sysfs entry to eDP connectors

[Hamza Mahfooz]

On 2/16/24 03:19, Pekka Paalanen wrote:

On Fri, 2 Feb 2024 10:28:35 -0500
Hamza Mahfooz  wrote:


We want programs besides the compositor to be able to enable or disable
panel power saving features.


Could you also explain why, in the commit message, please?

It is unexpected for arbitrary programs to be able to override the KMS
client, and certainly new ways to do so should not be added without an
excellent justification.

Maybe debugfs would be more appropriate if the purpose is only testing
rather than production environments?


However, since they are currently only
configurable through DRM properties, that isn't possible. So, to remedy
that issue introduce a new "panel_power_savings" sysfs attribute.


When the DRM property was added, what was used as the userspace to
prove its workings?


To my knowledge, it is only used by IGT. Also, it is worth noting that
it is a vendor specific property, so I doubt there are any compositors
out there that felt motivated enough to use it in any capacity.




Thanks,
pq



Cc: Mario Limonciello 
Signed-off-by: Hamza Mahfooz 
---
v2: hide ABM_LEVEL_IMMEDIATE_DISABLE in the read case, force an atomic
 commit when setting the value, call sysfs_remove_group() in
 amdgpu_dm_connector_unregister() and add some documentation.
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 76 +++
  1 file changed, 76 insertions(+)

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 8590c9f1dda6..3c62489d03dc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6436,10 +6436,79 @@ int amdgpu_dm_connector_atomic_get_property(struct 
drm_connector *connector,
return ret;
  }
  
+/**

+ * DOC: panel power savings
+ *
+ * The display manager allows you to set your desired **panel power savings**
+ * level (between 0-4, with 0 representing off), e.g. using the following::
+ *
+ *   # echo 3 > /sys/class/drm/card0-eDP-1/amdgpu/panel_power_savings
+ *
+ * Modifying this value can have implications on color accuracy, so tread
+ * carefully.
+ */
+
+static ssize_t panel_power_savings_show(struct device *device,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_connector *connector = dev_get_drvdata(device);
+   struct drm_device *dev = connector->dev;
+   u8 val;
+
+   drm_modeset_lock(>mode_config.connection_mutex, NULL);
+   val = to_dm_connector_state(connector->state)->abm_level ==
+   ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
+   to_dm_connector_state(connector->state)->abm_level;
+   drm_modeset_unlock(>mode_config.connection_mutex);
+
+   return sysfs_emit(buf, "%u\n", val);
+}
+
+static ssize_t panel_power_savings_store(struct device *device,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct drm_connector *connector = dev_get_drvdata(device);
+   struct drm_device *dev = connector->dev;
+   long val;
+   int ret;
+
+   ret = kstrtol(buf, 0, );
+
+   if (ret)
+   return ret;
+
+   if (val < 0 || val > 4)
+   return -EINVAL;
+
+   drm_modeset_lock(>mode_config.connection_mutex, NULL);
+   to_dm_connector_state(connector->state)->abm_level = val ?:
+   ABM_LEVEL_IMMEDIATE_DISABLE;
+   drm_modeset_unlock(>mode_config.connection_mutex);
+
+   drm_kms_helper_hotplug_event(dev);
+
+   return count;
+}
+
+static DEVICE_ATTR_RW(panel_power_savings);
+
+static struct attribute *amdgpu_attrs[] = {
+   _attr_panel_power_savings.attr,
+   NULL
+};
+
+static const struct attribute_group amdgpu_group = {
+   .name = "amdgpu",
+   .attrs = amdgpu_attrs
+};
+
  static void amdgpu_dm_connector_unregister(struct drm_connector *connector)
  {
struct amdgpu_dm_connector *amdgpu_dm_connector = 
to_amdgpu_dm_connector(connector);
  
+	sysfs_remove_group(>kdev->kobj, _group);

drm_dp_aux_unregister(_dm_connector->dm_dp_aux.aux);
  }
  
@@ -6541,6 +6610,13 @@ amdgpu_dm_connector_late_register(struct drm_connector *connector)

to_amdgpu_dm_connector(connector);
int r;
  
+	if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {

+   r = sysfs_create_group(>kdev->kobj,
+  _group);
+   if (r)
+   return r;
+   }
+
amdgpu_dm_register_backlight_device(amdgpu_dm_connector);
  
  	if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) ||



--
Hamza

Re: [PATCH] drm/amd/display: Fix && vs || typos

[Hamza Mahfooz]

On 2/9/24 08:02, Dan Carpenter wrote:

These ANDs should be ORs or it will lead to a NULL dereference.

Fixes: fb5a3d037082 ("drm/amd/display: Add NULL test for 'timing generator' in 
'dcn21_set_pipe()'")
Fixes: 886571d217d7 ("drm/amd/display: Fix 'panel_cntl' could be null in 
'dcn21_set_backlight_level()'")
Signed-off-by: Dan Carpenter 


Applied, thanks!


---
  drivers/gpu/drm/amd/display/dc/hwss/dcn21/dcn21_hwseq.c | 4 ++--
  1 file changed, 2 insertions(+), 2 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 5c7f380a84f9..7252f5f781f0 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
@@ -211,7 +211,7 @@ void dcn21_set_pipe(struct pipe_ctx *pipe_ctx)
struct dmcu *dmcu = pipe_ctx->stream->ctx->dc->res_pool->dmcu;
uint32_t otg_inst;
  
-	if (!abm && !tg && !panel_cntl)

+   if (!abm || !tg || !panel_cntl)
return;
  
  	otg_inst = tg->inst;

@@ -245,7 +245,7 @@ bool dcn21_set_backlight_level(struct pipe_ctx *pipe_ctx,
struct panel_cntl *panel_cntl = pipe_ctx->stream->link->panel_cntl;
uint32_t otg_inst;
  
-	if (!abm && !tg && !panel_cntl)

+   if (!abm || !tg || !panel_cntl)
return false;
  
  	otg_inst = tg->inst;

--
Hamza

[PATCH 3/3] drm/amdgpu: wire up the can_remove() callback

[Hamza Mahfooz]

Removing an amdgpu device that still has user space references allocated
to it causes undefined behaviour. So, implement amdgpu_pci_can_remove()
and disallow devices that still have files allocated to them from being
unbound.

Cc: sta...@vger.kernel.org
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index cc69005f5b46..cfa64f3c5be5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2323,6 +2323,22 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
return ret;
 }
 
+static bool amdgpu_pci_can_remove(struct pci_dev *pdev)
+{
+   struct drm_device *dev = pci_get_drvdata(pdev);
+
+   mutex_lock(>filelist_mutex);
+
+   if (!list_empty(>filelist)) {
+   mutex_unlock(>filelist_mutex);
+   return false;
+   }
+
+   mutex_unlock(>filelist_mutex);
+
+   return true;
+}
+
 static void
 amdgpu_pci_remove(struct pci_dev *pdev)
 {
@@ -2929,6 +2945,7 @@ static struct pci_driver amdgpu_kms_pci_driver = {
.name = DRIVER_NAME,
.id_table = pciidlist,
.probe = amdgpu_pci_probe,
+   .can_remove = amdgpu_pci_can_remove,
.remove = amdgpu_pci_remove,
.shutdown = amdgpu_pci_shutdown,
.driver.pm = _pm_ops,
-- 
2.43.0

[PATCH 2/3] PCI: introduce can_remove()

[Hamza Mahfooz]

Wire up the can_remove() callback, such that pci drivers can implement
their own version of it.

Cc: sta...@vger.kernel.org
Signed-off-by: Hamza Mahfooz 
---
 drivers/pci/pci-driver.c | 12 
 include/linux/pci.h  |  5 +
 2 files changed, 17 insertions(+)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 51ec9e7e784f..8aae484c5494 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -466,6 +466,17 @@ static int pci_device_probe(struct device *dev)
return error;
 }
 
+static bool pci_device_can_remove(struct device *dev)
+{
+   struct pci_dev *pci_dev = to_pci_dev(dev);
+   struct pci_driver *drv = pci_dev->driver;
+
+   if (drv->can_remove)
+   return drv->can_remove(pci_dev);
+
+   return true;
+}
+
 static void pci_device_remove(struct device *dev)
 {
struct pci_dev *pci_dev = to_pci_dev(dev);
@@ -1680,6 +1691,7 @@ struct bus_type pci_bus_type = {
.match  = pci_bus_match,
.uevent = pci_uevent,
.probe  = pci_device_probe,
+   .can_remove = pci_device_can_remove,
.remove = pci_device_remove,
.shutdown   = pci_device_shutdown,
.dev_groups = pci_dev_groups,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index add9368e6314..95276f44b23b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -902,6 +902,10 @@ struct module;
  * (negative number) otherwise.
  * The probe function always gets called from process
  * context, so it can sleep.
+ * @can_remove:The can_remove() function gets called during driver
+ * deregistration to determine if remove() can be called.
+ * The probe function always gets called from process
+ * context, so it can sleep.
  * @remove:The remove() function gets called whenever a device
  * being handled by this driver is removed (either during
  * deregistration of the driver or when it's manually
@@ -943,6 +947,7 @@ struct pci_driver {
const char  *name;
const struct pci_device_id *id_table;   /* Must be non-NULL for probe 
to be called */
int  (*probe)(struct pci_dev *dev, const struct pci_device_id *id); 
/* New device inserted */
+   bool (*can_remove)(struct pci_dev *dev);
void (*remove)(struct pci_dev *dev);/* Device removed (NULL if not 
a hot-plug capable driver) */
int  (*suspend)(struct pci_dev *dev, pm_message_t state);   /* 
Device suspended */
int  (*resume)(struct pci_dev *dev);/* Device woken up */
-- 
2.43.0

[PATCH 1/3] driver core: bus: introduce can_remove()

[Hamza Mahfooz]

Currently, drivers have no mechanism to block requests to unbind
devices. However, this can cause resource leaks and leave the device in
an inconsistent state, such that rebinding the device may cause a hang
or otherwise prevent the device from being rebound. So, introduce
the can_remove() callback to allow drivers to indicate if it isn't
appropriate to remove a device at the given time.

Cc: sta...@vger.kernel.org
Signed-off-by: Hamza Mahfooz 
---
 drivers/base/bus.c | 4 
 include/linux/device/bus.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index daee55c9b2d9..7c259b01ea99 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -239,6 +239,10 @@ static ssize_t unbind_store(struct device_driver *drv, 
const char *buf,
 
dev = bus_find_device_by_name(bus, NULL, buf);
if (dev && dev->driver == drv) {
+   if (dev->bus && dev->bus->can_remove &&
+   !dev->bus->can_remove(dev))
+   return -EBUSY;
+
device_driver_detach(dev);
err = count;
}
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index 5ef4ec1c36c3..c9d4af0ed3b8 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -46,6 +46,7 @@ struct fwnode_handle;
  * be called at late_initcall_sync level. If the device has
  * consumers that are never bound to a driver, this function
  * will never get called until they do.
+ * @can_remove: Called before attempting to remove a device from this bus.
  * @remove:Called when a device removed from this bus.
  * @shutdown:  Called at shut-down time to quiesce the device.
  *
@@ -85,6 +86,7 @@ struct bus_type {
int (*uevent)(const struct device *dev, struct kobj_uevent_env *env);
int (*probe)(struct device *dev);
void (*sync_state)(struct device *dev);
+   bool (*can_remove)(struct device *dev);
void (*remove)(struct device *dev);
void (*shutdown)(struct device *dev);
 
-- 
2.43.0

[PATCH v2] drm/amd/display: add panel_power_savings sysfs entry to eDP connectors

[Hamza Mahfooz]

We want programs besides the compositor to be able to enable or disable
panel power saving features. However, since they are currently only
configurable through DRM properties, that isn't possible. So, to remedy
that issue introduce a new "panel_power_savings" sysfs attribute.

Cc: Mario Limonciello 
Signed-off-by: Hamza Mahfooz 
---
v2: hide ABM_LEVEL_IMMEDIATE_DISABLE in the read case, force an atomic
commit when setting the value, call sysfs_remove_group() in
amdgpu_dm_connector_unregister() and add some documentation.
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 76 +++
 1 file changed, 76 insertions(+)

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 8590c9f1dda6..3c62489d03dc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6436,10 +6436,79 @@ int amdgpu_dm_connector_atomic_get_property(struct 
drm_connector *connector,
return ret;
 }
 
+/**
+ * DOC: panel power savings
+ *
+ * The display manager allows you to set your desired **panel power savings**
+ * level (between 0-4, with 0 representing off), e.g. using the following::
+ *
+ *   # echo 3 > /sys/class/drm/card0-eDP-1/amdgpu/panel_power_savings
+ *
+ * Modifying this value can have implications on color accuracy, so tread
+ * carefully.
+ */
+
+static ssize_t panel_power_savings_show(struct device *device,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_connector *connector = dev_get_drvdata(device);
+   struct drm_device *dev = connector->dev;
+   u8 val;
+
+   drm_modeset_lock(>mode_config.connection_mutex, NULL);
+   val = to_dm_connector_state(connector->state)->abm_level ==
+   ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
+   to_dm_connector_state(connector->state)->abm_level;
+   drm_modeset_unlock(>mode_config.connection_mutex);
+
+   return sysfs_emit(buf, "%u\n", val);
+}
+
+static ssize_t panel_power_savings_store(struct device *device,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct drm_connector *connector = dev_get_drvdata(device);
+   struct drm_device *dev = connector->dev;
+   long val;
+   int ret;
+
+   ret = kstrtol(buf, 0, );
+
+   if (ret)
+   return ret;
+
+   if (val < 0 || val > 4)
+   return -EINVAL;
+
+   drm_modeset_lock(>mode_config.connection_mutex, NULL);
+   to_dm_connector_state(connector->state)->abm_level = val ?:
+   ABM_LEVEL_IMMEDIATE_DISABLE;
+   drm_modeset_unlock(>mode_config.connection_mutex);
+
+   drm_kms_helper_hotplug_event(dev);
+
+   return count;
+}
+
+static DEVICE_ATTR_RW(panel_power_savings);
+
+static struct attribute *amdgpu_attrs[] = {
+   _attr_panel_power_savings.attr,
+   NULL
+};
+
+static const struct attribute_group amdgpu_group = {
+   .name = "amdgpu",
+   .attrs = amdgpu_attrs
+};
+
 static void amdgpu_dm_connector_unregister(struct drm_connector *connector)
 {
struct amdgpu_dm_connector *amdgpu_dm_connector = 
to_amdgpu_dm_connector(connector);
 
+   sysfs_remove_group(>kdev->kobj, _group);
drm_dp_aux_unregister(_dm_connector->dm_dp_aux.aux);
 }
 
@@ -6541,6 +6610,13 @@ amdgpu_dm_connector_late_register(struct drm_connector 
*connector)
to_amdgpu_dm_connector(connector);
int r;
 
+   if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
+   r = sysfs_create_group(>kdev->kobj,
+  _group);
+   if (r)
+   return r;
+   }
+
amdgpu_dm_register_backlight_device(amdgpu_dm_connector);
 
if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) ||
-- 
2.43.0

Re: [PATCH 2/6] drm: add drm_gem_object_is_shared_for_memory_stats() helper

[Hamza Mahfooz]

On 1/30/24 11:12, Alex Deucher wrote:

Add a helper so that drm drivers can consistently report
shared status via the fdinfo shared memory stats interface.

In addition to handle count, show buffers as shared if they
are shared via dma-buf as well (e.g., shared with v4l or some
other subsystem).

Link: 
https://lore.kernel.org/all/20231207180225.439482-1-alexander.deuc...@amd.com/
Signed-off-by: Alex Deucher 
---
  drivers/gpu/drm/drm_gem.c | 16 
  include/drm/drm_gem.h |  1 +
  2 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 44a948b80ee1..71b5f628d828 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1506,3 +1506,19 @@ int drm_gem_evict(struct drm_gem_object *obj)
return 0;
  }
  EXPORT_SYMBOL(drm_gem_evict);
+
+/**
+ * drm_gem_object_is_shared_for_memory_stats - helper for shared memory stats
+ *
+ * This helper should only be used for fdinfo shared memory stats to determine
+ * if a GEM object is shared.
+ *
+ * @obj: obj in question
+ */
+bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_object *obj)
+{
+   if ((obj->handle_count > 1) || obj->dma_buf)
+   return true;
+   return false;


nit: you can simplify this to:
return (obj->handle_count > 1) || obj->dma_buf;

(It maybe worth just inlining this to drm_gem.h).


+}
+EXPORT_SYMBOL(drm_gem_object_is_shared_for_memory_stats);
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 369505447acd..86a9c696f038 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -552,6 +552,7 @@ unsigned long drm_gem_lru_scan(struct drm_gem_lru *lru,
   bool (*shrink)(struct drm_gem_object *obj));
  
  int drm_gem_evict(struct drm_gem_object *obj);

+bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_object *obj);
  
  #ifdef CONFIG_LOCKDEP

  /**

--
Hamza

[PATCH] drm/amd/display: add panel_power_savings sysfs entry to eDP connectors

[Hamza Mahfooz]

We want programs besides the compositor to be able to enable or disable
panel power saving features. However, since they are currently only
configurable through DRM properties, that isn't possible. So, to remedy
that issue introduce a new "panel_power_savings" sysfs attribute.

Cc: Mario Limonciello 
Signed-off-by: Hamza Mahfooz 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 59 +++
 1 file changed, 59 insertions(+)

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 cd98b3565178..b3fcd833015d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6534,6 +6534,58 @@ amdgpu_dm_connector_atomic_duplicate_state(struct 
drm_connector *connector)
return _state->base;
 }
 
+static ssize_t panel_power_savings_show(struct device *device,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct drm_connector *connector = dev_get_drvdata(device);
+   struct drm_device *dev = connector->dev;
+   ssize_t val;
+
+   drm_modeset_lock(>mode_config.connection_mutex, NULL);
+   val = to_dm_connector_state(connector->state)->abm_level;
+   drm_modeset_unlock(>mode_config.connection_mutex);
+
+   return sysfs_emit(buf, "%lu\n", val);
+}
+
+static ssize_t panel_power_savings_store(struct device *device,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct drm_connector *connector = dev_get_drvdata(device);
+   struct drm_device *dev = connector->dev;
+   long val;
+   int ret;
+
+   ret = kstrtol(buf, 0, );
+
+   if (ret)
+   return ret;
+
+   if (val < 0 || val > 4)
+   return -EINVAL;
+
+   drm_modeset_lock(>mode_config.connection_mutex, NULL);
+   to_dm_connector_state(connector->state)->abm_level = val ?:
+   ABM_LEVEL_IMMEDIATE_DISABLE;
+   drm_modeset_unlock(>mode_config.connection_mutex);
+
+   return count;
+}
+
+static DEVICE_ATTR_RW(panel_power_savings);
+
+static struct attribute *amdgpu_attrs[] = {
+   _attr_panel_power_savings.attr,
+   NULL
+};
+
+static const struct attribute_group amdgpu_group = {
+   .name = "amdgpu",
+   .attrs = amdgpu_attrs
+};
+
 static int
 amdgpu_dm_connector_late_register(struct drm_connector *connector)
 {
@@ -6541,6 +6593,13 @@ amdgpu_dm_connector_late_register(struct drm_connector 
*connector)
to_amdgpu_dm_connector(connector);
int r;
 
+   if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
+   r = sysfs_create_group(>kdev->kobj,
+  _group);
+   if (r)
+   return r;
+   }
+
amdgpu_dm_register_backlight_device(amdgpu_dm_connector);
 
if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) ||
-- 
2.43.0

Re: [PATCH] drm/amd/display: Fix a switch statement in populate_dml_output_cfg_from_stream_state()

[Hamza Mahfooz]

On 1/13/24 09:58, Christophe JAILLET wrote:

It is likely that the statement related to 'dml_edp' is misplaced. So move
it in the correct "case SIGNAL_TYPE_EDP".

Fixes: 7966f319c66d ("drm/amd/display: Introduce DML2")
Signed-off-by: Christophe JAILLET 


Nice catch! Applied, thanks!


---
  drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c 
b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
index fa6a93dd9629..64d01a9cd68c 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
@@ -626,8 +626,8 @@ static void 
populate_dml_output_cfg_from_stream_state(struct dml_output_cfg_st *
if (is_dp2p0_output_encoder(pipe))
out->OutputEncoder[location] = dml_dp2p0;
break;
-   out->OutputEncoder[location] = dml_edp;
case SIGNAL_TYPE_EDP:
+   out->OutputEncoder[location] = dml_edp;
break;
case SIGNAL_TYPE_HDMI_TYPE_A:
case SIGNAL_TYPE_DVI_SINGLE_LINK:

--
Hamza

Re: [PATCH 6/6] drm: Add CONFIG_DRM_WERROR

[Hamza Mahfooz]

On 1/10/24 12:39, Jani Nikula wrote:

Add kconfig to enable -Werror subsystem wide. This is useful for
development and CI to keep the subsystem warning free, while avoiding
issues outside of the subsystem that kernel wide CONFIG_WERROR=y might
hit.

Signed-off-by: Jani Nikula 


Reviewed-by: Hamza Mahfooz 


---
  drivers/gpu/drm/Kconfig  | 18 ++
  drivers/gpu/drm/Makefile |  3 +++
  2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 6ec33d36f3a4..36a00cba2540 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -414,3 +414,21 @@ config DRM_LIB_RANDOM
  config DRM_PRIVACY_SCREEN
bool
default n
+
+config DRM_WERROR
+   bool "Compile the drm subsystem with warnings as errors"
+   # As this may inadvertently break the build, only allow the user
+   # to shoot oneself in the foot iff they aim really hard
+   depends on EXPERT
+   # We use the dependency on !COMPILE_TEST to not be enabled in
+   # allmodconfig or allyesconfig configurations
+   depends on !COMPILE_TEST
+   default n
+   help
+ A kernel build should not cause any compiler warnings, and this
+ enables the '-Werror' flag to enforce that rule in the drm subsystem.
+
+ The drm subsystem enables more warnings than the kernel default, so
+ this config option is disabled by default.
+
+ If in doubt, say N.
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 8b6be830f7c3..b7fd3e58b7af 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -32,6 +32,9 @@ subdir-ccflags-y += -Wno-sign-compare
  endif
  # --- end copy-paste
  
+# Enable -Werror in CI and development

+subdir-ccflags-$(CONFIG_DRM_WERROR) += -Werror
+
  drm-y := \
drm_aperture.o \
drm_atomic.o \

--
Hamza

Re: [PATCH 5/6] drm: enable (most) W=1 warnings by default across the subsystem

[Hamza Mahfooz]

On 1/10/24 12:39, Jani Nikula wrote:

At least the i915 and amd drivers enable a bunch more compiler warnings
than the kernel defaults.

Extend most of the W=1 warnings to the entire drm subsystem by
default. Use the copy-pasted warnings from scripts/Makefile.extrawarn
with s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and
keep up with them in the future.

This is similar to the approach currently used in i915.

Some of the -Wextra warnings do need to be disabled, just like in
Makefile.extrawarn, but take care to not disable them for W=2 or W=3
builds, depending on the warning.

There are too many -Wformat-truncation warnings to cleanly fix up front;
leave that warning disabled for now.

v2:
- Drop -Wformat-truncation (too many warnings)
- Drop -Wstringop-overflow (enabled by default upstream)

Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: Alex Deucher 
Cc: Christian König 
Cc: Pan, Xinhui 
Cc: Karol Herbst 
Cc: Lyude Paul 
Cc: Danilo Krummrich 
Cc: Rob Clark 
Cc: Abhinav Kumar 
Cc: Dmitry Baryshkov 
Cc: Sean Paul 
Cc: Marijn Suijten 
Cc: Hamza Mahfooz 
Acked-by: Javier Martinez Canillas 
Acked-by: Thomas Zimmermann 
Acked-by: Sui Jingfeng 
Signed-off-by: Jani Nikula 
---
  drivers/gpu/drm/Makefile | 27 +++
  1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 104b42df2e95..8b6be830f7c3 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -5,6 +5,33 @@
  
  CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)	+= -DDYNAMIC_DEBUG_MODULE
  
+# Unconditionally enable W=1 warnings locally

+# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
+subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter
+subdir-ccflags-y += -Wmissing-declarations
+subdir-ccflags-y += $(call cc-option, -Wrestrict)


It would be safer to do something along the lines of:

cond-flags := $(call cc-option, -Wrestrict) \
$(call cc-option, -Wunused-but-set-variable) \
$(call cc-option, -Wunused-const-variable) \
$(call cc-option, -Wunused-const-variable) \
$(call cc-option, -Wformat-overflow) \
$(call cc-option, -Wstringop-truncation)
subdir-ccflags-y += $(cond-flags)

Otherwise, you will end up breaking `$ make M=drivers/gpu/drm`
for a bunch of people.


+subdir-ccflags-y += -Wmissing-format-attribute
+subdir-ccflags-y += -Wmissing-prototypes
+subdir-ccflags-y += -Wold-style-definition
+subdir-ccflags-y += -Wmissing-include-dirs
+subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
+subdir-ccflags-y += $(call cc-option, -Wunused-const-variable)
+subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned)
+subdir-ccflags-y += $(call cc-option, -Wformat-overflow)
+# FIXME: fix -Wformat-truncation warnings and uncomment
+#subdir-ccflags-y += $(call cc-option, -Wformat-truncation)
+subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
+# The following turn off the warnings enabled by -Wextra
+ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),)
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-shift-negative-value
+endif
+ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),)
+subdir-ccflags-y += -Wno-sign-compare
+endif
+# --- end copy-paste
+
  drm-y := \
drm_aperture.o \
drm_atomic.o \

--
Hamza

Re: [RFC PATCH] drm/amd/display: fix bandwidth validation failure on DCN 2.1

[Hamza Mahfooz]

On 12/29/23 11:25, Melissa Wen wrote:

IGT `amdgpu/amd_color/crtc-lut-accuracy` fails right at the beginning of
the test execution, during atomic check, because DC rejects the
bandwidth state for a fb sizing 64x64. The test was previously working
with the deprecated dc_commit_state(). Now using
dc_validate_with_context() approach, the atomic check needs to perform a
full state validation. Therefore, set fast_validation to false in the
dc_validate_global_state call for atomic check.

Fixes: b8272241ff9d ("drm/amd/display: Drop dc_commit_state in favor of 
dc_commit_streams")
Signed-off-by: Melissa Wen 
---

Hi,

It's a long story. I was inspecting this bug report:
- https://gitlab.freedesktop.org/drm/amd/-/issues/2016
and noticed the IGT test `igt@amdgpu/amd_color@crtc-lut-accuracy`
mentioned there wasn't even being executed on a laptop with DCN 2.1
(HP HP ENVY x360 Convertible 13-ay1xxx/8929). The test fails right at
the beginning due to an atomic check rejection, as below:

Starting subtest: crtc-lut-accuracy
(amd_color:14772) igt_kms-CRITICAL: Test assertion failure function 
igt_display_commit_atomic, file ../lib/igt_kms.c:4530:
(amd_color:14772) igt_kms-CRITICAL: Failed assertion: ret == 0
(amd_color:14772) igt_kms-CRITICAL: Last errno: 22, Invalid argument
(amd_color:14772) igt_kms-CRITICAL: error: -22 != 0
Stack trace:
   #0 ../lib/igt_core.c:1989 __igt_fail_assert()
   #1 [igt_display_commit_atomic+0x44]
   #2 ../tests/amdgpu/amd_color.c:159 __igt_uniquereal_main395()
   #3 ../tests/amdgpu/amd_color.c:395 main()
   #4 ../sysdeps/nptl/libc_start_call_main.h:74 __libc_start_call_main()
   #5 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
   #6 [_start+0x21]
Subtest crtc-lut-accuracy failed.

Checking dmesg, we can see that a bandwidth validation failure causes
the atomic check rejection:

[  711.147663] amdgpu :04:00.0: [drm] Mode Validation Warning: Unknown 
Status failed validation.
[  711.147667] [drm:amdgpu_dm_atomic_check [amdgpu]] DC global validation 
failure: Bandwidth validation failure (BW and Watermark) (13)
[  711.147772] [drm:amdgpu_irq_dispatch [amdgpu]] Unregistered interrupt 
src_id: 243 of client_id:8
[  711.148033] [drm:amdgpu_dm_atomic_check [amdgpu]] Atomic check failed with 
err: -22

I also noticed that the atomic check doesn't fail if I change the fb
width and height used in the test from 64 to 66, and I can get the test
execution back (and with success). However, I recall that all test cases
of IGT `amd_color` were working in the past, so I bisected and found the
first bad commit:

b8272241ff9d drm/amd/display: Drop dc_commit_state in favor of dc_commit_streams

Bringing the `dc_commit_state` machinery back also prevents the
bandwidth validation failure, but the commit above says
dc_commit_streams validation is more complete than dc_commit_state, so I
discarded this approach.

After some debugging and code inspection, I found out that avoiding fast
validation on dc_validate_global_state during atomic check solves the
issue, but I'm not sure if this change may affect performance. I
compared exec time of some IGT tests and didn't see any differences, but
I recognize it doesn't provide enough evidence.

What do you think about this change? Should I examine other things? Do
you see any potential issue that I should investigate? Could you
recommend a better approach to assess any side-effect of not enabling
fast validation in the atomic check?

Please, let me know your thoughts.


We shouldn't be doing fast updates when lock_and_validation_needed is
true, so this seems to be correct.

Which is to say, applied, thanks!

Cc: sta...@vger.kernel.org



Happy New Year!

Melissa

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

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 2845c884398e..4f51a7ad7a3c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10745,7 +10745,7 @@ static int amdgpu_dm_atomic_check(struct drm_device 
*dev,
DRM_DEBUG_DRIVER("drm_dp_mst_atomic_check() failed\n");
goto fail;
}
-   status = dc_validate_global_state(dc, dm_state->context, true);
+   status = dc_validate_global_state(dc, dm_state->context, false);
if (status != DC_OK) {
DRM_DEBUG_DRIVER("DC global validation failure: %s 
(%d)",
   dc_status_to_str(status), status);

--
Hamza

Re: [PATCH v2 4/5] drm/atomic: Make the drm_atomic_state documentation less ambiguous

[Hamza Mahfooz]

On 12/14/23 05:09, Maxime Ripard wrote:

The current documentation of drm_atomic_state says that it's the "global
state object". This is confusing since, while it does contain all the
objects affected by an update and their respective states, if an object
isn't affected by this update it won't be part of it.

Thus, it's not truly a "global state", unlike object state structures
that do contain the entire state of a given object.

Signed-off-by: Maxime Ripard 


Reviewed-by: Hamza Mahfooz 


---
  include/drm/drm_atomic.h | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 914574b58ae7..5df67e587816 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -346,7 +346,13 @@ struct __drm_private_objs_state {
  };
  
  /**

- * struct drm_atomic_state - the global state object for atomic updates
+ * struct drm_atomic_state - Atomic commit structure
+ *
+ * This structure is the kernel counterpart of @drm_mode_atomic and represents
+ * an atomic commit that transitions from an old to a new display state. It
+ * contains all the objects affected by an atomic commits and both the new
+ * state structures and pointers to the old state structures for
+ * these.
   *
   * States are added to an atomic update by calling 
drm_atomic_get_crtc_state(),
   * drm_atomic_get_plane_state(), drm_atomic_get_connector_state(), or for

--
Hamza

Re: [RFC] drm: enable W=1 warnings by default across the subsystem

[Hamza Mahfooz]

Cc: Nathan Chancellor 

On 11/29/23 13:12, Jani Nikula wrote:

At least the i915 and amd drivers enable a bunch more compiler warnings
than the kernel defaults.

Extend the W=1 warnings to the entire drm subsystem by default. Use the
copy-pasted warnings from scripts/Makefile.extrawarn with
s/KBUILD_CFLAGS/subdir-ccflags-y/ to make it easier to compare and keep
up with them in the future.

This is similar to the approach currently used in i915.

Some of the -Wextra warnings do need to be disabled, just like in
Makefile.extrawarn, but take care to not disable them for W=2 or W=3
builds, depending on the warning.


I think this should go in after drm-misc-next has a clean build (for
COMPILE_TEST builds) with this patch applied. Otherwise, it will break a
lot of build configs.



Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: Alex Deucher 
Cc: Christian König 
Cc: Pan, Xinhui 
Cc: Karol Herbst 
Cc: Lyude Paul 
Cc: Danilo Krummrich 
Cc: Rob Clark 
Cc: Abhinav Kumar 
Cc: Dmitry Baryshkov 
Cc: Sean Paul 
Cc: Marijn Suijten 
Signed-off-by: Jani Nikula 

---

With my admittedly limited and very much x86 focused kernel config, I
get some -Wunused-but-set-variable and -Wformat-truncation= warnings,
but nothing we can't handle.

We could fix them up front, or disable the extra warnings on a per
driver basis with a FIXME comment in their respective Makefiles.

With the experience from i915, I think this would significantly reduce
the constant loop of warnings added by people not using W=1 and
subsequently fixed by people using W=1.

Note: I've Cc'd the maintainers of drm, drm misc and some of the biggest
drivers.
---
  drivers/gpu/drm/Makefile | 27 +++
  1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index b4cb0835620a..6939e4ea13d5 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -5,6 +5,33 @@
  
  CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)	+= -DDYNAMIC_DEBUG_MODULE
  
+# Unconditionally enable W=1 warnings locally

+# --- begin copy-paste W=1 warnings from scripts/Makefile.extrawarn
+subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter
+subdir-ccflags-y += -Wmissing-declarations
+subdir-ccflags-y += $(call cc-option, -Wrestrict)
+subdir-ccflags-y += -Wmissing-format-attribute
+subdir-ccflags-y += -Wmissing-prototypes
+subdir-ccflags-y += -Wold-style-definition
+subdir-ccflags-y += -Wmissing-include-dirs
+subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
+subdir-ccflags-y += $(call cc-option, -Wunused-const-variable)
+subdir-ccflags-y += $(call cc-option, -Wpacked-not-aligned)
+subdir-ccflags-y += $(call cc-option, -Wformat-overflow)
+subdir-ccflags-y += $(call cc-option, -Wformat-truncation)
+subdir-ccflags-y += $(call cc-option, -Wstringop-overflow)
+subdir-ccflags-y += $(call cc-option, -Wstringop-truncation)
+# The following turn off the warnings enabled by -Wextra
+ifeq ($(findstring 2, $(KBUILD_EXTRA_WARN)),)
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-shift-negative-value
+endif
+ifeq ($(findstring 3, $(KBUILD_EXTRA_WARN)),)
+subdir-ccflags-y += -Wno-sign-compare
+endif
+# --- end copy-paste
+
  drm-y := \
drm_aperture.o \
drm_atomic.o \

--
Hamza

Re: [PATCH 2/2] drm/amdgpu: use GTT only as fallback for VRAM|GTT

[Hamza Mahfooz]

On 11/27/23 09:54, Christian König wrote:

Try to fill up VRAM as well by setting the busy flag on GTT allocations.

This fixes the issue that when VRAM was evacuated for suspend it's never
filled up again unless the application is restarted.



Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2893


Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index aa0dd6dad068..ddc8fb4db678 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -173,6 +173,12 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo 
*abo, u32 domain)
abo->flags & AMDGPU_GEM_CREATE_PREEMPTIBLE ?
AMDGPU_PL_PREEMPT : TTM_PL_TT;
places[c].flags = 0;
+   /*
+* When GTT is just an alternative to VRAM make sure that we
+* only use it as fallback and still try to fill up VRAM first.
+*/
+   if (domain & AMDGPU_GEM_DOMAIN_VRAM)
+   places[c].flags |= TTM_PL_FLAG_BUSY;
c++;
}
  

--
Hamza

Re: [PATCH v9 0/4] drm: Add support for atomic async page-flip

[Hamza Mahfooz]

Hi André,
On 11/22/23 11:19, André Almeida wrote:

Hi,

This work from me and Simon adds support for DRM_MODE_PAGE_FLIP_ASYNC through
the atomic API. This feature is already available via the legacy API. The use
case is to be able to present a new frame immediately (or as soon as
possible), even if after missing a vblank. This might result in tearing, but
it's useful when a high framerate is desired, such as for gaming.

Differently from earlier versions, this one refuses to flip if any prop changes
for async flips. The idea is that the fast path of immediate page flips doesn't
play well with modeset changes, so only the fb_id can be changed.

Tested with:
  - Intel TigerLake-LP GT2
  - AMD VanGogh


Have you had a chance to test this with VRR enabled? Since, I suspect
this series might break that feature.



Thanks,
André

- User-space patch: https://github.com/Plagman/gamescope/pull/595
- IGT tests: 
https://lore.kernel.org/all/20231110163811.24158-1-andrealm...@igalia.com/

Changes from v8:
- Dropped atomic_async_page_flip_not_supported, giving that current design works
with any driver that support atomic and async at the same time.
- Dropped the patch that disabled atomic_async_page_flip_not_supported for AMD.
- Reordered commits
v8: https://lore.kernel.org/all/20231025005318.293690-1-andrealm...@igalia.com/

Changes from v7:
- Only accept flips to primary planes. If a driver support flips in different
planes, support will be added  later.
v7: 
https://lore.kernel.org/dri-devel/20231017092837.32428-1-andrealm...@igalia.com/

Changes from v6:
- Dropped the exception to allow MODE_ID changes (Simon)
- Clarify what happens when flipping with the same FB_ID (Pekka)

v6: 
https://lore.kernel.org/dri-devel/20230815185710.159779-1-andrealm...@igalia.com/

Changes from v5:
- Add note in the docs that not every redundant attribute will result in no-op,
   some might cause oversynchronization issues.

v5: 
https://lore.kernel.org/dri-devel/20230707224059.305474-1-andrealm...@igalia.com/

Changes from v4:
  - Documentation rewrote by Pekka Paalanen

v4: 
https://lore.kernel.org/dri-devel/20230701020917.143394-1-andrealm...@igalia.com/

Changes from v3:
  - Add new patch to reject prop changes
  - Add a documentation clarifying the KMS atomic state set

v3: 
https://lore.kernel.org/dri-devel/20220929184307.258331-1-cont...@emersion.fr/

André Almeida (1):
   drm: Refuse to async flip with atomic prop changes

Pekka Paalanen (1):
   drm/doc: Define KMS atomic state set

Simon Ser (2):
   drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits
   drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP

  Documentation/gpu/drm-uapi.rst  | 47 ++
  drivers/gpu/drm/drm_atomic_uapi.c   | 77 ++---
  drivers/gpu/drm/drm_crtc_internal.h |  2 +-
  drivers/gpu/drm/drm_ioctl.c |  4 ++
  drivers/gpu/drm/drm_mode_object.c   |  2 +-
  include/uapi/drm/drm.h  | 10 +++-
  include/uapi/drm/drm_mode.h |  9 
  7 files changed, 142 insertions(+), 9 deletions(-)


--
Hamza

Re: [Bug 218168] New: amdgpu: kfd_topology.c warning: the frame size of 1408 bytes is larger than 1024 bytes

[Hamza Mahfooz]

+ amd-gfx
+ Felix

On 11/20/23 10:16, bugzilla-dae...@kernel.org wrote:

https://bugzilla.kernel.org/show_bug.cgi?id=218168

 Bug ID: 218168
Summary: amdgpu: kfd_topology.c warning: the frame size of 1408
 bytes is larger than 1024 bytes
Product: Drivers
Version: 2.5
   Hardware: All
 OS: Linux
 Status: NEW
   Severity: normal
   Priority: P3
  Component: Video(DRI - non Intel)
   Assignee: drivers_video-...@kernel-bugs.osdl.org
   Reporter: bluesun...@gmail.com
 Regression: No

Trying to compile Linux 6.6.2 with GCC 13.2.1 and CONFIG_WERROR=y:

[...]
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: In function
'kfd_topology_add_device':
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:2082:1: error: the frame
size of 1408 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
  2082 | }
   | ^
cc1: all warnings being treated as errors
[...]


--
Hamza

Re: [PATCH v2] drm/amd/display: fix NULL dereference

[Hamza Mahfooz]

On 11/14/23 10:27, José Pekkarinen wrote:

The following patch will fix a minor issue where a debug message is
referencing an struct that has just being checked whether is null or
not. This has been noticed by using coccinelle, in the following output:

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c:540:25-29: ERROR: 
aconnector is NULL but dereferenced.

Fixes: 5d72e247e58c9 ("drm/amd/display: switch DC over to the new DRM logging 
macros")


You only need the first 12 characters of the hash here. I have fixed it
for you and applied the patch in this case. But, in the future please
test your patches against `./scripts/checkpatch.pl` before submitting
them.


Signed-off-by: José Pekkarinen 
---
[v1 -> v2]: Remove the debugging message, requested by Hamza

  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 5 +
  1 file changed, 1 insertion(+), 4 deletions(-)

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 ed784cf27d39..c7a29bb737e2 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
@@ -536,11 +536,8 @@ bool dm_helpers_dp_read_dpcd(
  
  	struct amdgpu_dm_connector *aconnector = link->priv;
  
-	if (!aconnector) {

-   drm_dbg_dp(aconnector->base.dev,
-  "Failed to find connector for link!\n");
+   if (!aconnector)
return false;
-   }
  
  	return drm_dp_dpcd_read(>dm_dp_aux.aux, address, data,

size) == size;

--
Hamza

Re: [PATCH] drm/amd/display: fix NULL dereference

[Hamza Mahfooz]

On 11/14/23 01:36, José Pekkarinen wrote:

The following patch will fix a minor issue where a debug message is
referencing an struct that has just being checked whether is null or
not. This has been noticed by using coccinelle, in the following output:

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c:540:25-29: ERROR: 
aconnector is NULL but dereferenced.

Fixes: 5d72e247e58c9 ("drm/amd/display: switch DC over to the new DRM logging 
macros")
Signed-off-by: José Pekkarinen 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

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 ed784cf27d39..7048dab5e356 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
@@ -537,8 +537,7 @@ bool dm_helpers_dp_read_dpcd(
struct amdgpu_dm_connector *aconnector = link->priv;
  
  	if (!aconnector) {

-   drm_dbg_dp(aconnector->base.dev,
-  "Failed to find connector for link!\n");
+   DRM_ERROR("Failed to find connector for link!");


I would prefer a patch that drops this error message entirely since
it's not particularly useful. As, it's only possible before hw init
(and at that point it's expected).


return false;
}
  

--
Hamza

[PATCH v2] drm/amd/display: add a debugfs interface for the DMUB trace mask

[Hamza Mahfooz]

For features that are implemented primarily in DMUB (e.g. PSR), it is
useful to be able to trace them at a DMUB level from the kernel,
especially when debugging issues. So, introduce a debugfs interface that
is able to read and set the DMUB trace mask dynamically at runtime and
document how to use it.

Cc: Alex Deucher 
Cc: Mario Limonciello 
Signed-off-by: Hamza Mahfooz 
---
v2: only return -ETIMEDOUT for DMUB_STATUS_TIMEOUT
---
 Documentation/gpu/amdgpu/display/dc-debug.rst | 41 
 .../gpu/amdgpu/display/trace-groups-table.csv | 29 ++
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 97 +++
 .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h   | 40 +++-
 4 files changed, 205 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/gpu/amdgpu/display/trace-groups-table.csv

diff --git a/Documentation/gpu/amdgpu/display/dc-debug.rst 
b/Documentation/gpu/amdgpu/display/dc-debug.rst
index 40c55a618918..817631b1dbf3 100644
--- a/Documentation/gpu/amdgpu/display/dc-debug.rst
+++ b/Documentation/gpu/amdgpu/display/dc-debug.rst
@@ -75,3 +75,44 @@ change in real-time by using something like::
 
 When reporting a bug related to DC, consider attaching this log before and
 after you reproduce the bug.
+
+DMUB Firmware Debug
+===
+
+Sometimes, dmesg logs aren't enough. This is especially true if a feature is
+implemented primarily in DMUB firmware. In such cases, all we see in dmesg when
+an issue arises is some generic timeout error. So, to get more relevant
+information, we can trace DMUB commands by enabling the relevant bits in
+`amdgpu_dm_dmub_trace_mask`.
+
+Currently, we support the tracing of the following groups:
+
+Trace Groups
+
+
+.. csv-table::
+   :header-rows: 1
+   :widths: 1, 1
+   :file: ./trace-groups-table.csv
+
+**Note: Not all ASICs support all of the listed trace groups**
+
+So, to enable just PSR tracing you can use the following command::
+
+  # echo 0x8020 > /sys/kernel/debug/dri/0/amdgpu_dm_dmub_trace_mask
+
+Then, you need to enable logging trace events to the buffer, which you can do
+using the following::
+
+  # echo 1 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+
+Lastly, after you are able to reproduce the issue you are trying to debug,
+you can disable tracing and read the trace log by using the following::
+
+  # echo 0 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+  # cat /sys/kernel/debug/dri/0/amdgpu_dm_dmub_tracebuffer
+
+So, when reporting bugs related to features such as PSR and ABM, consider
+enabling the relevant bits in the mask before reproducing the issue and
+attach the log that you obtain from the trace buffer in any bug reports that 
you
+create.
diff --git a/Documentation/gpu/amdgpu/display/trace-groups-table.csv 
b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
new file mode 100644
index ..3f6a50d1d883
--- /dev/null
+++ b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
@@ -0,0 +1,29 @@
+Name, Mask Value
+INFO, 0x1
+IRQ SVC, 0x2
+VBIOS, 0x4
+REGISTER, 0x8
+PHY DBG, 0x10
+PSR, 0x20
+AUX, 0x40
+SMU, 0x80
+MALL, 0x100
+ABM, 0x200
+ALPM, 0x400
+TIMER, 0x800
+HW LOCK MGR, 0x1000
+INBOX1, 0x2000
+PHY SEQ, 0x4000
+PSR STATE, 0x8000
+ZSTATE, 0x1
+TRANSMITTER CTL, 0x2
+PANEL CNTL, 0x4
+FAMS, 0x8
+DPIA, 0x10
+SUBVP, 0x20
+INBOX0, 0x40
+SDP, 0x400
+REPLAY, 0x800
+REPLAY RESIDENCY, 0x2000
+CURSOR INFO, 0x8000
+IPS, 0x1
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 45c972f2630d..67dea56cf583 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -2971,6 +2971,100 @@ static int allow_edp_hotplug_detection_set(void *data, 
u64 val)
return 0;
 }
 
+static int dmub_trace_mask_set(void *data, u64 val)
+{
+   struct amdgpu_device *adev = data;
+   struct dmub_srv *srv = adev->dm.dc->ctx->dmub_srv->dmub;
+   enum dmub_gpint_command cmd;
+   enum dmub_status status;
+   u64 mask = 0x;
+   u8 shift = 0;
+   u32 res;
+   int i;
+
+   if (!srv->fw_version)
+   return -EINVAL;
+
+   for (i = 0;  i < 4; i++) {
+   res = (val & mask) >> shift;
+
+   switch (i) {
+   case 0:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD0;
+   break;
+   case 1:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD1;
+   break;
+   case 2:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD2;
+   break;
+   case 3:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD3;
+   break;
+   }
+
+   status = dmub_srv_send_gpint_command(srv, cmd, r

[PATCH] drm/amd/display: add a debugfs interface for the DMUB trace mask

[Hamza Mahfooz]

For features that are implemented primarily in DMUB (e.g. PSR), it is
useful to be able to trace them at a DMUB level from the kernel,
especially when debugging issues. So, introduce a debugfs interface that
is able to read and set the DMUB trace mask dynamically at runtime and
document how to use it.

Cc: Alex Deucher 
Cc: Mario Limonciello 
Signed-off-by: Hamza Mahfooz 
---
 Documentation/gpu/amdgpu/display/dc-debug.rst | 41 +
 .../gpu/amdgpu/display/trace-groups-table.csv | 29 ++
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 91 +++
 .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h   | 40 +++-
 4 files changed, 199 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/gpu/amdgpu/display/trace-groups-table.csv

diff --git a/Documentation/gpu/amdgpu/display/dc-debug.rst 
b/Documentation/gpu/amdgpu/display/dc-debug.rst
index 40c55a618918..817631b1dbf3 100644
--- a/Documentation/gpu/amdgpu/display/dc-debug.rst
+++ b/Documentation/gpu/amdgpu/display/dc-debug.rst
@@ -75,3 +75,44 @@ change in real-time by using something like::
 
 When reporting a bug related to DC, consider attaching this log before and
 after you reproduce the bug.
+
+DMUB Firmware Debug
+===
+
+Sometimes, dmesg logs aren't enough. This is especially true if a feature is
+implemented primarily in DMUB firmware. In such cases, all we see in dmesg when
+an issue arises is some generic timeout error. So, to get more relevant
+information, we can trace DMUB commands by enabling the relevant bits in
+`amdgpu_dm_dmub_trace_mask`.
+
+Currently, we support the tracing of the following groups:
+
+Trace Groups
+
+
+.. csv-table::
+   :header-rows: 1
+   :widths: 1, 1
+   :file: ./trace-groups-table.csv
+
+**Note: Not all ASICs support all of the listed trace groups**
+
+So, to enable just PSR tracing you can use the following command::
+
+  # echo 0x8020 > /sys/kernel/debug/dri/0/amdgpu_dm_dmub_trace_mask
+
+Then, you need to enable logging trace events to the buffer, which you can do
+using the following::
+
+  # echo 1 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+
+Lastly, after you are able to reproduce the issue you are trying to debug,
+you can disable tracing and read the trace log by using the following::
+
+  # echo 0 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+  # cat /sys/kernel/debug/dri/0/amdgpu_dm_dmub_tracebuffer
+
+So, when reporting bugs related to features such as PSR and ABM, consider
+enabling the relevant bits in the mask before reproducing the issue and
+attach the log that you obtain from the trace buffer in any bug reports that 
you
+create.
diff --git a/Documentation/gpu/amdgpu/display/trace-groups-table.csv 
b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
new file mode 100644
index ..3f6a50d1d883
--- /dev/null
+++ b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
@@ -0,0 +1,29 @@
+Name, Mask Value
+INFO, 0x1
+IRQ SVC, 0x2
+VBIOS, 0x4
+REGISTER, 0x8
+PHY DBG, 0x10
+PSR, 0x20
+AUX, 0x40
+SMU, 0x80
+MALL, 0x100
+ABM, 0x200
+ALPM, 0x400
+TIMER, 0x800
+HW LOCK MGR, 0x1000
+INBOX1, 0x2000
+PHY SEQ, 0x4000
+PSR STATE, 0x8000
+ZSTATE, 0x1
+TRANSMITTER CTL, 0x2
+PANEL CNTL, 0x4
+FAMS, 0x8
+DPIA, 0x10
+SUBVP, 0x20
+INBOX0, 0x40
+SDP, 0x400
+REPLAY, 0x800
+REPLAY RESIDENCY, 0x2000
+CURSOR INFO, 0x8000
+IPS, 0x1
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 13a177d34376..06a73f283e9d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -2971,6 +2971,94 @@ static int allow_edp_hotplug_detection_set(void *data, 
u64 val)
return 0;
 }
 
+static int dmub_trace_mask_set(void *data, u64 val)
+{
+   struct amdgpu_device *adev = data;
+   struct dmub_srv *srv = adev->dm.dc->ctx->dmub_srv->dmub;
+   enum dmub_gpint_command cmd;
+   enum dmub_status status;
+   u64 mask = 0x;
+   u8 shift = 0;
+   u32 res;
+   int i;
+
+   if (!srv->fw_version)
+   return -EINVAL;
+
+   for (i = 0;  i < 4; i++) {
+   res = (val & mask) >> shift;
+
+   switch (i) {
+   case 0:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD0;
+   break;
+   case 1:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD1;
+   break;
+   case 2:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD2;
+   break;
+   case 3:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD3;
+   break;
+   }
+
+   status = dmub_srv_send_gpint_command(srv, cmd, res, 30);
+
+   if (status != DMUB_STAT

Re: [PATCH] drm/edid: add a quirk for two 240Hz Samsung monitors

[Hamza Mahfooz]

On 11/1/23 17:36, Alex Deucher wrote:

On Wed, Nov 1, 2023 at 5:01 PM Hamza Mahfooz  wrote:


Without this fix the 5120x1440@240 timing of these monitors
leads to screen flickering.

Cc: sta...@vger.kernel.org # 6.1+
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1442
Co-developed-by: Harry Wentland 
Signed-off-by: Harry Wentland 
Signed-off-by: Hamza Mahfooz 
---
  drivers/gpu/drm/drm_edid.c | 47 +++---
  1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bca2af4fe1fc..3fdb8907f66b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -89,6 +89,8 @@ static int oui(u8 first, u8 second, u8 third)
  #define EDID_QUIRK_NON_DESKTOP (1 << 12)
  /* Cap the DSC target bitrate to 15bpp */
  #define EDID_QUIRK_CAP_DSC_15BPP   (1 << 13)
+/* Fix up a particular 5120x1440@240Hz timing */
+#define EDID_QUIRK_FIXUP_5120_1440_240 (1 << 14)


What is wrong with the original timing that needs to be fixed?


Apparently, all of timing values for the 5120x1440@240 mode of these
monitors aren't set correctly (they are all lower than they should be)
in their EDIDs. For what it's worth, the windows driver has had a quirk
similar the one proposed in this patch for ~2 years.



Alex




  #define MICROSOFT_IEEE_OUI 0xca125c

@@ -170,6 +172,12 @@ static const struct edid_quirk {
 EDID_QUIRK('S', 'A', 'M', 596, EDID_QUIRK_PREFER_LARGE_60),
 EDID_QUIRK('S', 'A', 'M', 638, EDID_QUIRK_PREFER_LARGE_60),

+   /* Samsung C49G95T */
+   EDID_QUIRK('S', 'A', 'M', 0x7053, EDID_QUIRK_FIXUP_5120_1440_240),
+
+   /* Samsung S49AG95 */
+   EDID_QUIRK('S', 'A', 'M', 0x71ac, EDID_QUIRK_FIXUP_5120_1440_240),
+
 /* Sony PVM-2541A does up to 12 bpc, but only reports max 8 bpc */
 EDID_QUIRK('S', 'N', 'Y', 0x2541, EDID_QUIRK_FORCE_12BPC),

@@ -6586,7 +6594,37 @@ static void update_display_info(struct drm_connector 
*connector,
 drm_edid_to_eld(connector, drm_edid);
  }

-static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device 
*dev,
+static void drm_mode_displayid_detailed_edid_quirks(struct drm_connector 
*connector,
+   struct drm_display_mode 
*mode)
+{
+   unsigned int hsync_width;
+   unsigned int vsync_width;
+
+   if (connector->display_info.quirks & EDID_QUIRK_FIXUP_5120_1440_240) {
+   if (mode->hdisplay == 5120 && mode->vdisplay == 1440 &&
+   mode->clock == 1939490) {
+   hsync_width = mode->hsync_end - mode->hsync_start;
+   vsync_width = mode->vsync_end - mode->vsync_start;
+
+   mode->clock = 2018490;
+   mode->hdisplay = 5120;
+   mode->hsync_start = 5120 + 8;
+   mode->hsync_end = 5120 + 8 + hsync_width;
+   mode->htotal = 5200;
+
+   mode->vdisplay = 1440;
+   mode->vsync_start = 1440 + 165;
+   mode->vsync_end = 1440 + 165 + vsync_width;
+   mode->vtotal = 1619;
+
+   drm_dbg_kms(connector->dev,
+   "[CONNECTOR:%d:%s] Samsung 240Hz mode quirk 
applied\n",
+   connector->base.id, connector->name);
+   }
+   }
+}
+
+static struct drm_display_mode *drm_mode_displayid_detailed(struct 
drm_connector *connector,
 struct 
displayid_detailed_timings_1 *timings,
 bool type_7)
  {
@@ -6605,7 +6643,7 @@ static struct drm_display_mode 
*drm_mode_displayid_detailed(struct drm_device *d
 bool hsync_positive = (timings->hsync[1] >> 7) & 0x1;
 bool vsync_positive = (timings->vsync[1] >> 7) & 0x1;

-   mode = drm_mode_create(dev);
+   mode = drm_mode_create(connector->dev);
 if (!mode)
 return NULL;

@@ -6628,6 +,9 @@ static struct drm_display_mode 
*drm_mode_displayid_detailed(struct drm_device *d

 if (timings->flags & 0x80)
 mode->type |= DRM_MODE_TYPE_PREFERRED;
+
+   drm_mode_displayid_detailed_edid_quirks(connector, mode);
+
 drm_mode_set_name(mode);

 return mode;
@@ -6650,7 +6691,7 @@ static int add_displayid_detailed_1_modes(struct 
drm_connector *connector,
 for (i = 0; i < num_timings; i++) {
 struct displayid_detailed_timings_1 *timings = 
>timings[i];

-   newmode = drm_mode_displayid_detailed(connector->dev, timings, 
type_7);
+   newmode = drm_mode_displayid_detailed(connector, timings, 
type_7);
 if (!newmode)
 continue;

--
2.42.0


--
Hamza

Re: [PATCH] drm/amd/display: Increase frame warning limit for clang in dml2

[Hamza Mahfooz]

On 11/2/23 13:12, Nathan Chancellor wrote:

On Thu, Nov 02, 2023 at 12:59:00PM -0400, Hamza Mahfooz wrote:

On 11/2/23 12:24, Nathan Chancellor wrote:

When building ARCH=x86_64 allmodconfig with clang, which have sanitizers
enabled, there is a warning about a large stack frame.

drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6265:13: 
error: stack frame size (2520) exceeds limit (2048) in 'dml_prefetch_check' 
[-Werror,-Wframe-larger-than]
 6265 | static void dml_prefetch_check(struct display_mode_lib_st *mode_lib)
  | ^
1 error generated.

Notably, GCC 13.2.0 does not do too much of a better job, as it is right
at the current limit of 2048:

drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c: In 
function 'dml_prefetch_check':
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6705:1: 
error: the frame size of 2048 bytes is larger than 1800 bytes 
[-Werror=frame-larger-than=]
 6705 | }
  | ^

In the past, these warnings have been avoided by reducing the number of
parameters to various functions so that not as many arguments need to be
passed on the stack. However, these patches take a good amount of effort
to write despite being mechanical due to code structure and complexity
and they are never carried forward to new generations of the code so
that effort has to be expended every new hardware generation, which
becomes harder to justify as time goes on.

There is some effort to improve clang's code generation but that may
take some time between code review, shifting priorities, and release
cycles. To avoid having a noticeable or lengthy breakage in
all{mod,yes}config, which are easy testing targets that have -Werror
enabled, increase the limit for clang by 50% so that cases of extremely
poor code generation can still be caught while not breaking the majority
of builds. When clang's code generation improves, the limit increase can
be restricted to older clang versions.

Signed-off-by: Nathan Chancellor 
---
If there is another DRM pull before 6.7-rc1, it would be much
appreciated if this could make that so that other trees are not
potentially broken by this. If not, no worries, as it was my fault for
not sending this sooner.
---
   drivers/gpu/drm/amd/display/dc/dml2/Makefile | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile 
b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
index 70ae5eba624e..dff8237c0999 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
@@ -60,7 +60,7 @@ endif
   endif
   ifneq ($(CONFIG_FRAME_WARN),0)
-frame_warn_flag := -Wframe-larger-than=2048
+frame_warn_flag := -Wframe-larger-than=$(if $(CONFIG_CC_IS_CLANG),3072,2048)


I would prefer checking for `CONFIG_KASAN || CONFIG_KCSAN` instead
since the stack usage shouldn't change much if both of those are disabled.


So something like this? Or were you talking about replacing the clang
check entirely with the KASAN/KCSAN check?


I think for the time being replacing the clang check with a KASAN/KCSAN
check would make more sense. Considering that, the allmodconfig for older
versions of gcc is also broken (see [1]).

1. 
https://lore.kernel.org/amd-gfx/CADVatmO9NCs=ryng72hnzmdpqg862gpgnnfhq4uwtpekjok...@mail.gmail.com/




diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile 
b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
index 70ae5eba624e..0fc1b13295eb 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
@@ -60,8 +60,12 @@ endif
  endif
  
  ifneq ($(CONFIG_FRAME_WARN),0)

+ifeq ($(CONFIG_CC_IS_CLANG)$(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),yy)
+frame_warn_flag := -Wframe-larger-than=3072
+else
  frame_warn_flag := -Wframe-larger-than=2048
  endif
+endif
  
  CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) $(frame_warn_flag)

  CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_util.o := $(dml2_ccflags)


   endif
   CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) 
$(frame_warn_flag)

---
base-commit: 21e80f3841c01aeaf32d7aee7bbc87b3db1aa0c6
change-id: 
20231102-amdgpu-dml2-increase-frame-size-warning-for-clang-c93bd2d6a871

Best regards,

--
Hamza


--
Hamza

Re: [PATCH] drm/amd/display: Increase frame warning limit for clang in dml2

[Hamza Mahfooz]

On 11/2/23 12:24, Nathan Chancellor wrote:

When building ARCH=x86_64 allmodconfig with clang, which have sanitizers
enabled, there is a warning about a large stack frame.

   drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6265:13: 
error: stack frame size (2520) exceeds limit (2048) in 'dml_prefetch_check' 
[-Werror,-Wframe-larger-than]
6265 | static void dml_prefetch_check(struct display_mode_lib_st *mode_lib)
 | ^
   1 error generated.

Notably, GCC 13.2.0 does not do too much of a better job, as it is right
at the current limit of 2048:

   drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c: In 
function 'dml_prefetch_check':
   drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/display_mode_core.c:6705:1: 
error: the frame size of 2048 bytes is larger than 1800 bytes 
[-Werror=frame-larger-than=]
6705 | }
 | ^

In the past, these warnings have been avoided by reducing the number of
parameters to various functions so that not as many arguments need to be
passed on the stack. However, these patches take a good amount of effort
to write despite being mechanical due to code structure and complexity
and they are never carried forward to new generations of the code so
that effort has to be expended every new hardware generation, which
becomes harder to justify as time goes on.

There is some effort to improve clang's code generation but that may
take some time between code review, shifting priorities, and release
cycles. To avoid having a noticeable or lengthy breakage in
all{mod,yes}config, which are easy testing targets that have -Werror
enabled, increase the limit for clang by 50% so that cases of extremely
poor code generation can still be caught while not breaking the majority
of builds. When clang's code generation improves, the limit increase can
be restricted to older clang versions.

Signed-off-by: Nathan Chancellor 
---
If there is another DRM pull before 6.7-rc1, it would be much
appreciated if this could make that so that other trees are not
potentially broken by this. If not, no worries, as it was my fault for
not sending this sooner.
---
  drivers/gpu/drm/amd/display/dc/dml2/Makefile | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile 
b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
index 70ae5eba624e..dff8237c0999 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
@@ -60,7 +60,7 @@ endif
  endif
  
  ifneq ($(CONFIG_FRAME_WARN),0)

-frame_warn_flag := -Wframe-larger-than=2048
+frame_warn_flag := -Wframe-larger-than=$(if $(CONFIG_CC_IS_CLANG),3072,2048)


I would prefer checking for `CONFIG_KASAN || CONFIG_KCSAN` instead
since the stack usage shouldn't change much if both of those are disabled.


  endif
  
  CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) $(frame_warn_flag)


---
base-commit: 21e80f3841c01aeaf32d7aee7bbc87b3db1aa0c6
change-id: 
20231102-amdgpu-dml2-increase-frame-size-warning-for-clang-c93bd2d6a871

Best regards,

--
Hamza

[PATCH] drm/edid: add a quirk for two 240Hz Samsung monitors

[Hamza Mahfooz]

Without this fix the 5120x1440@240 timing of these monitors
leads to screen flickering.

Cc: sta...@vger.kernel.org # 6.1+
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1442
Co-developed-by: Harry Wentland 
Signed-off-by: Harry Wentland 
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/drm_edid.c | 47 +++---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bca2af4fe1fc..3fdb8907f66b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -89,6 +89,8 @@ static int oui(u8 first, u8 second, u8 third)
 #define EDID_QUIRK_NON_DESKTOP (1 << 12)
 /* Cap the DSC target bitrate to 15bpp */
 #define EDID_QUIRK_CAP_DSC_15BPP   (1 << 13)
+/* Fix up a particular 5120x1440@240Hz timing */
+#define EDID_QUIRK_FIXUP_5120_1440_240 (1 << 14)
 
 #define MICROSOFT_IEEE_OUI 0xca125c
 
@@ -170,6 +172,12 @@ static const struct edid_quirk {
EDID_QUIRK('S', 'A', 'M', 596, EDID_QUIRK_PREFER_LARGE_60),
EDID_QUIRK('S', 'A', 'M', 638, EDID_QUIRK_PREFER_LARGE_60),
 
+   /* Samsung C49G95T */
+   EDID_QUIRK('S', 'A', 'M', 0x7053, EDID_QUIRK_FIXUP_5120_1440_240),
+
+   /* Samsung S49AG95 */
+   EDID_QUIRK('S', 'A', 'M', 0x71ac, EDID_QUIRK_FIXUP_5120_1440_240),
+
/* Sony PVM-2541A does up to 12 bpc, but only reports max 8 bpc */
EDID_QUIRK('S', 'N', 'Y', 0x2541, EDID_QUIRK_FORCE_12BPC),
 
@@ -6586,7 +6594,37 @@ static void update_display_info(struct drm_connector 
*connector,
drm_edid_to_eld(connector, drm_edid);
 }
 
-static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device 
*dev,
+static void drm_mode_displayid_detailed_edid_quirks(struct drm_connector 
*connector,
+   struct drm_display_mode 
*mode)
+{
+   unsigned int hsync_width;
+   unsigned int vsync_width;
+
+   if (connector->display_info.quirks & EDID_QUIRK_FIXUP_5120_1440_240) {
+   if (mode->hdisplay == 5120 && mode->vdisplay == 1440 &&
+   mode->clock == 1939490) {
+   hsync_width = mode->hsync_end - mode->hsync_start;
+   vsync_width = mode->vsync_end - mode->vsync_start;
+
+   mode->clock = 2018490;
+   mode->hdisplay = 5120;
+   mode->hsync_start = 5120 + 8;
+   mode->hsync_end = 5120 + 8 + hsync_width;
+   mode->htotal = 5200;
+
+   mode->vdisplay = 1440;
+   mode->vsync_start = 1440 + 165;
+   mode->vsync_end = 1440 + 165 + vsync_width;
+   mode->vtotal = 1619;
+
+   drm_dbg_kms(connector->dev,
+   "[CONNECTOR:%d:%s] Samsung 240Hz mode quirk 
applied\n",
+   connector->base.id, connector->name);
+   }
+   }
+}
+
+static struct drm_display_mode *drm_mode_displayid_detailed(struct 
drm_connector *connector,
struct 
displayid_detailed_timings_1 *timings,
bool type_7)
 {
@@ -6605,7 +6643,7 @@ static struct drm_display_mode 
*drm_mode_displayid_detailed(struct drm_device *d
bool hsync_positive = (timings->hsync[1] >> 7) & 0x1;
bool vsync_positive = (timings->vsync[1] >> 7) & 0x1;
 
-   mode = drm_mode_create(dev);
+   mode = drm_mode_create(connector->dev);
if (!mode)
return NULL;
 
@@ -6628,6 +,9 @@ static struct drm_display_mode 
*drm_mode_displayid_detailed(struct drm_device *d
 
if (timings->flags & 0x80)
mode->type |= DRM_MODE_TYPE_PREFERRED;
+
+   drm_mode_displayid_detailed_edid_quirks(connector, mode);
+
drm_mode_set_name(mode);
 
return mode;
@@ -6650,7 +6691,7 @@ static int add_displayid_detailed_1_modes(struct 
drm_connector *connector,
for (i = 0; i < num_timings; i++) {
struct displayid_detailed_timings_1 *timings = >timings[i];
 
-   newmode = drm_mode_displayid_detailed(connector->dev, timings, 
type_7);
+   newmode = drm_mode_displayid_detailed(connector, timings, 
type_7);
if (!newmode)
continue;
 
-- 
2.42.0

Re: [PATCH] drm/amdgpu: Fixes uninitialized variable usage in amdgpu_dm_setup_replay

[Hamza Mahfooz]

On 10/27/23 11:55, Lakha, Bhawanpreet wrote:

[AMD Official Use Only - General]



There was a consensus to use memset instead of {0}. I remember making 
changes related to that previously.


Hm, seems like it's used rather consistently in the DM and in DC
though.



Bhawan


*From:* Mahfooz, Hamza 
*Sent:* October 27, 2023 11:53 AM
*To:* Yuran Pereira ; airl...@gmail.com 

*Cc:* Li, Sun peng (Leo) ; Lakha, Bhawanpreet 
; Pan, Xinhui ; Siqueira, 
Rodrigo ; linux-ker...@vger.kernel.org 
; amd-...@lists.freedesktop.org 
; dri-devel@lists.freedesktop.org 
; Deucher, Alexander 
; Koenig, Christian 
; 
linux-kernel-ment...@lists.linuxfoundation.org 

*Subject:* Re: [PATCH] drm/amdgpu: Fixes uninitialized variable usage in 
amdgpu_dm_setup_replay

On 10/26/23 17:25, Yuran Pereira wrote:

Since `pr_config` is not initialized after its declaration, the
following operations with `replay_enable_option` may be performed
when `replay_enable_option` is holding junk values which could
possibly lead to undefined behaviour

```
  ...
  pr_config.replay_enable_option |= pr_enable_option_static_screen;
  ...

  if (!pr_config.replay_timing_sync_supported)
  pr_config.replay_enable_option &= ~pr_enable_option_general_ui;
  ...
```

This patch initializes `pr_config` after its declaration to ensure that
it doesn't contain junk data, and prevent any undefined behaviour

Addresses-Coverity-ID: 1544428 ("Uninitialized scalar variable")
Fixes: dede1fea4460 ("drm/amd/display: Add Freesync Panel DM code")
Signed-off-by: Yuran Pereira 
---
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c | 3 +++
   1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
index 32d3086c4cb7..40526507f50b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
@@ -23,6 +23,7 @@
    *
    */
  
+#include 

   #include "amdgpu_dm_replay.h"
   #include "dc.h"
   #include "dm_helpers.h"
@@ -74,6 +75,8 @@ bool amdgpu_dm_setup_replay(struct dc_link *link, struct 
amdgpu_dm_connector *ac
    struct replay_config pr_config;


I would prefer setting pr_config = {0};


    union replay_debug_flags *debug_flags = NULL;
  
+ memset(_config, 0, sizeof(pr_config));

+
    // For eDP, if Replay is supported, return true to skip checks
    if (link->replay_settings.config.replay_supported)
    return true;

--
Hamza


--
Hamza

Re: [PATCH] drm/amdgpu: Fixes uninitialized variable usage in amdgpu_dm_setup_replay

[Hamza Mahfooz]

Also, please write the tagline in present tense.
On 10/27/23 11:53, Hamza Mahfooz wrote:

On 10/26/23 17:25, Yuran Pereira wrote:

Since `pr_config` is not initialized after its declaration, the
following operations with `replay_enable_option` may be performed
when `replay_enable_option` is holding junk values which could
possibly lead to undefined behaviour

```
 ...
 pr_config.replay_enable_option |= pr_enable_option_static_screen;
 ...

 if (!pr_config.replay_timing_sync_supported)
 pr_config.replay_enable_option &= ~pr_enable_option_general_ui;
 ...
```

This patch initializes `pr_config` after its declaration to ensure that
it doesn't contain junk data, and prevent any undefined behaviour

Addresses-Coverity-ID: 1544428 ("Uninitialized scalar variable")
Fixes: dede1fea4460 ("drm/amd/display: Add Freesync Panel DM code")
Signed-off-by: Yuran Pereira 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c

index 32d3086c4cb7..40526507f50b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
@@ -23,6 +23,7 @@
   *
   */
+#include 
  #include "amdgpu_dm_replay.h"
  #include "dc.h"
  #include "dm_helpers.h"
@@ -74,6 +75,8 @@ bool amdgpu_dm_setup_replay(struct dc_link *link, 
struct amdgpu_dm_connector *ac

  struct replay_config pr_config;


I would prefer setting pr_config = {0};


  union replay_debug_flags *debug_flags = NULL;
+    memset(_config, 0, sizeof(pr_config));
+
  // For eDP, if Replay is supported, return true to skip checks
  if (link->replay_settings.config.replay_supported)
  return true;

--
Hamza

Re: [PATCH] drm/amdgpu: Fixes uninitialized variable usage in amdgpu_dm_setup_replay

[Hamza Mahfooz]

On 10/26/23 17:25, Yuran Pereira wrote:

Since `pr_config` is not initialized after its declaration, the
following operations with `replay_enable_option` may be performed
when `replay_enable_option` is holding junk values which could
possibly lead to undefined behaviour

```
 ...
 pr_config.replay_enable_option |= pr_enable_option_static_screen;
 ...

 if (!pr_config.replay_timing_sync_supported)
 pr_config.replay_enable_option &= ~pr_enable_option_general_ui;
 ...
```

This patch initializes `pr_config` after its declaration to ensure that
it doesn't contain junk data, and prevent any undefined behaviour

Addresses-Coverity-ID: 1544428 ("Uninitialized scalar variable")
Fixes: dede1fea4460 ("drm/amd/display: Add Freesync Panel DM code")
Signed-off-by: Yuran Pereira 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
index 32d3086c4cb7..40526507f50b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_replay.c
@@ -23,6 +23,7 @@
   *
   */
  
+#include 

  #include "amdgpu_dm_replay.h"
  #include "dc.h"
  #include "dm_helpers.h"
@@ -74,6 +75,8 @@ bool amdgpu_dm_setup_replay(struct dc_link *link, struct 
amdgpu_dm_connector *ac
struct replay_config pr_config;


I would prefer setting pr_config = {0};


union replay_debug_flags *debug_flags = NULL;
  
+	memset(_config, 0, sizeof(pr_config));

+
// For eDP, if Replay is supported, return true to skip checks
if (link->replay_settings.config.replay_supported)
return true;

--
Hamza

Re: [PATCH v2] drm/atomic-helper: Fix spelling mistake "preceeding" -> "preceding"

[Hamza Mahfooz]

On 10/26/23 22:44, chentao wrote:

From: Kunwu Chan 

There is a typo in the kernel documentation for function
drm_atomic_helper_wait_for_dependencies. Fix it.

Signed-off-by: Kunwu Chan 


Applied, thanks!


---
  drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 2444fc33dd7c..c3f677130def 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2382,10 +2382,10 @@ int drm_atomic_helper_setup_commit(struct 
drm_atomic_state *state,
  EXPORT_SYMBOL(drm_atomic_helper_setup_commit);
  
  /**

- * drm_atomic_helper_wait_for_dependencies - wait for required preceeding 
commits
+ * drm_atomic_helper_wait_for_dependencies - wait for required preceding 
commits
   * @old_state: atomic state object with old state structures
   *
- * This function waits for all preceeding commits that touch the same CRTC as
+ * This function waits for all preceding commits that touch the same CRTC as
   * @old_state to both be committed to the hardware (as signalled by
   * drm_atomic_helper_commit_hw_done()) and executed by the hardware (as 
signalled
   * by calling drm_crtc_send_vblank_event() on the _crtc_state.event).

--
Hamza

Re: [PATCH] drm/atomic: Spelling fix in comments

[Hamza Mahfooz]

Hi Kunwu,

Can you make the tagline something along the lines of `drm/atomic
helper: fix spelling mistake "preceeding" -> "preceding"`, in general to
determine an appropriate prefix, you can see what previous commits used
when making changes to files in your particular patch, e.g. using the
following:

$ git log drivers/gpu/drm/drm_atomic_helper.c
On 10/25/23 04:26, Kunwu Chan wrote:

fix a typo in a comments.


For patch descriptions you should try to more descriptive.

So, something like "There is a spelling mistake in 


drm_atomic_helper_wait_for_dependencies()'s kernel doc. Fix it." would
be better.



Signed-off-by: Kunwu Chan 
---
  drivers/gpu/drm/drm_atomic_helper.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 2444fc33dd7c..c3f677130def 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2382,10 +2382,10 @@ int drm_atomic_helper_setup_commit(struct 
drm_atomic_state *state,
  EXPORT_SYMBOL(drm_atomic_helper_setup_commit);
  
  /**

- * drm_atomic_helper_wait_for_dependencies - wait for required preceeding 
commits
+ * drm_atomic_helper_wait_for_dependencies - wait for required preceding 
commits
   * @old_state: atomic state object with old state structures
   *
- * This function waits for all preceeding commits that touch the same CRTC as
+ * This function waits for all preceding commits that touch the same CRTC as
   * @old_state to both be committed to the hardware (as signalled by
   * drm_atomic_helper_commit_hw_done()) and executed by the hardware (as 
signalled
   * by calling drm_crtc_send_vblank_event() on the _crtc_state.event).

--
Hamza

Re: [PATCH v3] drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()

[Hamza Mahfooz]

On 10/15/23 10:27, Geert Uytterhoeven wrote:

Currently drm_client_buffer_addfb() uses the legacy drm_mode_addfb(),
which uses bpp and depth to guess the wanted buffer format.
However, drm_client_buffer_addfb() already knows the exact buffer
format, so there is no need to convert back and forth between buffer
format and bpp/depth, and the function can just call drm_mode_addfb2()
directly instead.

Signed-off-by: Geert Uytterhoeven 
Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 


Applied and pushed to drm-misc-next, thanks!


---
v3:
   - Extract from series "[PATCH v2 0/8] drm: fb-helper/ssd130x: Add
 support for DRM_FORMAT_R1"
 (https://lore.kernel.org/all/cover.1692888745.git.ge...@linux-m68k.org),
 as this patch has merits on its own,
v2:
   - Add Reviewed-by, Tested-by,
   - s/drm_mode_create_dumb/drm_client_buffer_addfb/ in one-line summary.
---
  drivers/gpu/drm/drm_client.c | 13 +
  1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index d4296440f297fc5a..a780832a0963fe38 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -395,19 +395,16 @@ static int drm_client_buffer_addfb(struct 
drm_client_buffer *buffer,
   u32 handle)
  {
struct drm_client_dev *client = buffer->client;
-   struct drm_mode_fb_cmd fb_req = { };
-   const struct drm_format_info *info;
+   struct drm_mode_fb_cmd2 fb_req = { };
int ret;
  
-	info = drm_format_info(format);

-   fb_req.bpp = drm_format_info_bpp(info, 0);
-   fb_req.depth = info->depth;
fb_req.width = width;
fb_req.height = height;
-   fb_req.handle = handle;
-   fb_req.pitch = buffer->pitch;
+   fb_req.pixel_format = format;
+   fb_req.handles[0] = handle;
+   fb_req.pitches[0] = buffer->pitch;
  
-	ret = drm_mode_addfb(client->dev, _req, client->file);

+   ret = drm_mode_addfb2(client->dev, _req, client->file);
if (ret)
return ret;
  

--
Hamza

Re: [PATCH] drm/amd/display: Respect CONFIG_FRAME_WARN=0 in DML2

[Hamza Mahfooz]

On 10/18/23 14:45, Nathan Chancellor wrote:

display_mode_code.c is unconditionally built with
-Wframe-larger-than=2048, which causes warnings even when
CONFIG_FRAME_WARN has been set to 0, which should show no warnings.

Use the existing $(frame_warn_flag) variable, which handles this
situation. This is basically commit 25f178bbd078 ("drm/amd/display:
Respect CONFIG_FRAME_WARN=0 in dml Makefile") but for DML2.

Fixes: 7966f319c66d ("drm/amd/display: Introduce DML2")
Signed-off-by: Nathan Chancellor 


Applied, thanks!


---
  drivers/gpu/drm/amd/display/dc/dml2/Makefile | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile 
b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
index f35ed8de260d..66431525f2a0 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
@@ -61,7 +61,7 @@ ifneq ($(CONFIG_FRAME_WARN),0)
  frame_warn_flag := -Wframe-larger-than=2048
  endif
  
-CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) -Wframe-larger-than=2048

+CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) 
$(frame_warn_flag)
  CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_util.o := $(dml2_ccflags)
  CFLAGS_$(AMDDALPATH)/dc/dml2/dml2_wrapper.o := $(dml2_ccflags)
  CFLAGS_$(AMDDALPATH)/dc/dml2/dml2_utils.o := $(dml2_ccflags)

---
base-commit: cd90511557fdfb394bb4ac4c3b539b007383914c
change-id: 20231018-amdgpu-dml2-respect-frame_warn-536e19b69ce0

Best regards,

--
Hamza

Re: [PATCH] drm/edid: add 8 bpc quirk to the BenQ GW2765

[Hamza Mahfooz]

On 10/13/23 06:30, Ville Syrjälä wrote:

On Thu, Oct 12, 2023 at 02:49:27PM -0400, Hamza Mahfooz wrote:

The BenQ GW2765 reports that it supports higher (> 8) bpc modes, but
when trying to set them we end up with a black screen. So, limit it to 8
bpc modes.


Bad cable/etc was ruled out as the cause?


Yup, the issue was also reproduced by two different people with same
aforementioned monitor.





Cc: sta...@vger.kernel.org # 6.5+
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2610
Signed-off-by: Hamza Mahfooz 
---
  drivers/gpu/drm/drm_edid.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0454da505687..bca2af4fe1fc 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -123,6 +123,9 @@ static const struct edid_quirk {
/* AEO model 0 reports 8 bpc, but is a 6 bpc panel */
EDID_QUIRK('A', 'E', 'O', 0, EDID_QUIRK_FORCE_6BPC),
  
+	/* BenQ GW2765 */

+   EDID_QUIRK('B', 'N', 'Q', 0x78d6, EDID_QUIRK_FORCE_8BPC),
+
/* BOE model on HP Pavilion 15-n233sl reports 8 bpc, but is a 6 bpc 
panel */
EDID_QUIRK('B', 'O', 'E', 0x78b, EDID_QUIRK_FORCE_6BPC),
  
--

2.42.0



--
Hamza

[PATCH] drm/edid: add 8 bpc quirk to the BenQ GW2765

[Hamza Mahfooz]

The BenQ GW2765 reports that it supports higher (> 8) bpc modes, but
when trying to set them we end up with a black screen. So, limit it to 8
bpc modes.

Cc: sta...@vger.kernel.org # 6.5+
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2610
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/drm_edid.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0454da505687..bca2af4fe1fc 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -123,6 +123,9 @@ static const struct edid_quirk {
/* AEO model 0 reports 8 bpc, but is a 6 bpc panel */
EDID_QUIRK('A', 'E', 'O', 0, EDID_QUIRK_FORCE_6BPC),
 
+   /* BenQ GW2765 */
+   EDID_QUIRK('B', 'N', 'Q', 0x78d6, EDID_QUIRK_FORCE_8BPC),
+
/* BOE model on HP Pavilion 15-n233sl reports 8 bpc, but is a 6 bpc 
panel */
EDID_QUIRK('B', 'O', 'E', 0x78b, EDID_QUIRK_FORCE_6BPC),
 
-- 
2.42.0

Re: [PATCH RESEND v2 1/2] drm/print: Add drm_dbg_ratelimited

[Hamza Mahfooz]

On 10/10/23 08:15, Andi Shyti wrote:

From: Nirmoy Das 

Add a function for ratelimitted debug print.

Signed-off-by: Nirmoy Das 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Reviewed-by: Matthew Auld 
Reviewed-by: Andrzej Hajda 
Reviewed-by: Andi Shyti 
Reviewed-by: Sam Ravnborg 
Signed-off-by: Andi Shyti 
---
  include/drm/drm_print.h | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index a93a387f8a1a..ad77ac4b6808 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -602,6 +602,9 @@ void __drm_err(const char *format, ...);
drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ## 
__VA_ARGS__);\
  })
  
+#define drm_dbg_ratelimited(drm, fmt, ...) \

+   __DRM_DEFINE_DBG_RATELIMITED(DRIVER, drm, fmt, ## __VA_ARGS__)
+


I guess since this was last sent drm_dbg_driver() was introduced,
with drm_dbg() only being grandfathered in since it's already
widely used, so it would probably be better to call this
drm_dbg_driver_ratelimited() instead.


  #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
__DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
  

--
Hamza

Re: [PATCH 0/5] drm/amd/display: Remove migrate-disable and move memory allocation.

[Hamza Mahfooz]

On 9/21/23 10:15, Sebastian Andrzej Siewior wrote:

Hi,

I stumbled uppon the amdgpu driver via a bugzilla report. The actual fix
is #4 + #5 and the rest was made while looking at the code.

Sebastian


I have applied the series, thanks!





--
Hamza

Re: [PATCH 1/5] drm/amd/display: Remove migrate_en/dis from dc_fpu_begin().

[Hamza Mahfooz]

On 10/3/23 15:53, Harry Wentland wrote:

On 2023-09-21 10:15, Sebastian Andrzej Siewior wrote:

This is a revert of the commit mentioned below while it is not wrong, as
in the kernel will explode, having migrate_disable() here it is
complete waste of resources.

Additionally commit message is plain wrong the review tag does not make


Not sure I follow what's unhelpful about the review tag with
0c316556d1249 ("drm/amd/display: Disable migration to ensure consistency of per-CPU 
variable")

I do wish the original patch showed the splat it's attempting
to fix. It apparently made a difference for something, whether
inadvertently or not. I wish I knew what that "something" was.


I did some digging, and it seems like the intention of that patch was to
fix the following splat:

WARNING: CPU: 5 PID: 1062 at 
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/dc_fpu.c:71 
dc_assert_fp_enabled+0x1a/0x30 [amdgpu]

[...]
CPU: 5 PID: 1062 Comm: Xorg Tainted: G   OE 
5.15.0-56-generic #62-Ubuntu
Hardware name: ASUS System Product Name/ROG STRIX Z590-F GAMING WIFI, 
BIOS 1202 10/27/2021

RIP: 0010:dc_assert_fp_enabled+0x1a/0x30 [amdgpu]
Code: ff 45 31 f6 0f 0b e9 ca fe ff ff e8 90 1c 1f f7 48 c7 c0 00 30 03 
00 65 48 03 05 b1 aa 86 3f 8b 00 85 c0 7e 05 c3 cc cc cc cc <0f> 0b c3 
cc cc cc cc 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00

RSP: :b89b82a8f118 EFLAGS: 00010246
RAX:  RBX: 8c271cd0 RCX: 
RDX: 8c2708025000 RSI: 8c270e8c RDI: 8c271cd0
RBP: b89b82a8f1d0 R08:  R09: 7f6a
R10: b89b82a8f240 R11:  R12: 0002
R13: 8c271cd0 R14: 8c270e8c R15: 8c2708025000
FS:  7f0570019a80() GS:8c2e3fb4() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 5594858a0058 CR3: 00010e204001 CR4: 00770ee0
PKRU: 5554
Call Trace:
 
 ? dcn20_populate_dml_pipes_from_context+0x47/0x1730 [amdgpu]
 ? __kmalloc_node+0x2cb/0x3a0
 dcn32_populate_dml_pipes_from_context+0x2b/0x450 [amdgpu]
 dcn32_internal_validate_bw+0x15f9/0x2670 [amdgpu]
 dcn32_find_dummy_latency_index_for_fw_based_mclk_switch+0xd0/0x310 
[amdgpu]

 dcn32_calculate_wm_and_dlg_fpu+0xe6/0x1e50 [amdgpu]
 dcn32_calculate_wm_and_dlg+0x46/0x70 [amdgpu]
 dcn32_validate_bandwidth+0x1b7/0x3e0 [amdgpu]
 dc_validate_global_state+0x32c/0x560 [amdgpu]
 dc_validate_with_context+0x6e6/0xd80 [amdgpu]
 dc_commit_streams+0x21b/0x500 [amdgpu]
 dc_commit_state+0xf3/0x150 [amdgpu]
 amdgpu_dm_atomic_commit_tail+0x60d/0x3120 [amdgpu]
 ? dcn32_internal_validate_bw+0xcf8/0x2670 [amdgpu]
 ? fill_plane_buffer_attributes+0x1e5/0x560 [amdgpu]
 ? dcn32_validate_bandwidth+0x1e0/0x3e0 [amdgpu]
 ? kfree+0x1f7/0x250
 ? dcn32_validate_bandwidth+0x1e0/0x3e0 [amdgpu]
 ? dc_validate_global_state+0x32c/0x560 [amdgpu]
 ? __cond_resched+0x1a/0x50
 ? __wait_for_common+0x3e/0x150
 ? fill_plane_buffer_attributes+0x1e5/0x560 [amdgpu]
 ? usleep_range_state+0x90/0x90
 ? wait_for_completion_timeout+0x1d/0x30
 commit_tail+0xc2/0x170 [drm_kms_helper]
 ? drm_atomic_helper_swap_state+0x20f/0x370 [drm_kms_helper]
 drm_atomic_helper_commit+0x12b/0x150 [drm_kms_helper]
 amdgpu_dm_atomic_commit+0x11/0x20 [amdgpu]
 drm_atomic_commit+0x47/0x60 [drm]
 drm_mode_obj_set_property_ioctl+0x16b/0x420 [drm]
 ? mutex_lock+0x13/0x50
 ? drm_mode_createblob_ioctl+0xf6/0x130 [drm]
 ? drm_mode_obj_find_prop_id+0x90/0x90 [drm]
 drm_ioctl_kernel+0xb0/0x100 [drm]
 drm_ioctl+0x268/0x4b0 [drm]
 ? drm_mode_obj_find_prop_id+0x90/0x90 [drm]
 ? ktime_get_mono_fast_ns+0x4a/0xa0
 amdgpu_drm_ioctl+0x4e/0x90 [amdgpu]
 __x64_sys_ioctl+0x92/0xd0
 do_syscall_64+0x59/0xc0
 ? do_user_addr_fault+0x1e7/0x670
 ? do_syscall_64+0x69/0xc0
 ? exit_to_user_mode_prepare+0x37/0xb0
 ? irqentry_exit_to_user_mode+0x9/0x20
 ? irqentry_exit+0x1d/0x30
 ? exc_page_fault+0x89/0x170
 entry_SYSCALL_64_after_hwframe+0x61/0xcb
RIP: 0033:0x7f05704a2aff
Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 
44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <41> 89 c0 
3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00

RSP: 002b:7ffc8c45a3f0 EFLAGS: 0246 ORIG_RAX: 0010
RAX: ffda RBX: 7ffc8c45a480 RCX: 7f05704a2aff
RDX: 7ffc8c45a480 RSI: c01864ba RDI: 000e
RBP: c01864ba R08: 0077 R09: 
R10: 7f05705a22f0 R11: 0246 R12: 0004
R13: 000e R14: 000f R15: 7ffc8c45a8a8
 
---[ end trace 4deab30bb69df00f ]---



Harry


it any better. The migrate_disable() interface has a fat comment
describing it and it includes the word "undesired" in the headline which
should tickle people to read it before using it.
Initially I assumed it is worded too harsh but now I beg to differ.

The reviewer of the original commit, even not understanding what
migrate_disable() does should ask the following:

- 

[PATCH] Revert "drm/amd/display: Check all enabled planes in dm_check_crtc_cursor"

[Hamza Mahfooz]

From: Ivan Lipski 

This reverts commit 45e1ade04b4d60fe5df859076005779f27c4c9be.

Since, it causes the following IGT tests to fail:
kms_cursor_legacy@cursor-vs-flip.*
kms_cursor_legacy@flip-vs-cursor.*

Signed-off-by: Ivan Lipski 
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

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 32156609fbcf..49ffb4d6e9cc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10290,24 +10290,14 @@ static int dm_check_crtc_cursor(struct 
drm_atomic_state *state,
 * blending properties match the underlying planes'.
 */
 
-   new_cursor_state = drm_atomic_get_plane_state(state, cursor);
-   if (IS_ERR(new_cursor_state))
-   return PTR_ERR(new_cursor_state);
-
-   if (!new_cursor_state->fb)
+   new_cursor_state = drm_atomic_get_new_plane_state(state, cursor);
+   if (!new_cursor_state || !new_cursor_state->fb)
return 0;
 
dm_get_oriented_plane_size(new_cursor_state, _src_w, 
_src_h);
cursor_scale_w = new_cursor_state->crtc_w * 1000 / cursor_src_w;
cursor_scale_h = new_cursor_state->crtc_h * 1000 / cursor_src_h;
 
-   /* Need to check all enabled planes, even if this commit doesn't change
-* their state
-*/
-   i = drm_atomic_add_affected_planes(state, crtc);
-   if (i)
-   return i;
-
for_each_new_plane_in_state_reverse(state, underlying, 
new_underlying_state, i) {
/* Narrow down to non-cursor planes on the same CRTC as the 
cursor */
if (new_underlying_state->crtc != crtc || underlying == 
crtc->cursor)
-- 
2.42.0

Re: [PATCH] drm/amd/display: fix the ability to use lower resolution modes on eDP

[Hamza Mahfooz]

On 9/14/23 17:04, Hamza Mahfooz wrote:


On 9/14/23 16:40, Harry Wentland wrote:

On 2023-09-14 13:53, Hamza Mahfooz wrote:

On eDP we can receive invalid modes from dm_update_crtc_state() for
entirely new streams for which drm_mode_set_crtcinfo() shouldn't be
called on. So, instead of calling drm_mode_set_crtcinfo() from within
create_stream_for_sink() we can instead call it from
amdgpu_dm_connector_mode_valid(). Since, we are guaranteed to only call
drm_mode_set_crtcinfo() for valid modes from that function (invalid
modes are rejected by that callback) and that is the only user
of create_validate_stream_for_sink() that we need to call
drm_mode_set_crtcinfo() for (as before commit cb841d27b876
("drm/amd/display: Always pass connector_state to stream validation"),
that is the only place where create_validate_stream_for_sink()'s
dm_state was NULL).



I don't seem to see how a NULL dm_state in
create_validate_stream_for_sink() (or create_stream_for_sink() for that
matter) has an impact on the drm_mode_set_crtcinfo() call. That one 
depends

on !old_stream and 


If we look back to commit 4a2df0d1f28e ("drm/amd/display: Fixed
non-native modes not lighting up") it seems like the intent was to only
have drm_mode_set_crtcinfo() called for
amdgpu_dm_connector_mode_valid(). Since, even if we go that far back
create_stream_for_sink()'s dm_state was only NULL when it was called
from amdgpu_dm_connector_mode_valid().



It does look like  is an empty mode if we can't find a 
preferred_mode,

though. Not sure if that can cause an issue.


I don't think it should be an issue, since before commit 4a2df0d1f28e
("drm/amd/display: Fixed non-native modes not lighting up") we always


I meant to refer to commit bd49f19039c1 ("drm/amd/display: Always set
crtcinfo from create_stream_for_sink") here.

called drm_mode_set_crtcinfo() in the aforementioned case (and only for 
that case).




Harry


Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2693
Fixes: cb841d27b876 ("drm/amd/display: Always pass connector_state to 
stream validation")

Signed-off-by: Hamza Mahfooz 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

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 933c9b5d5252..beef4fef7338 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6128,8 +6128,6 @@ create_stream_for_sink(struct 
amdgpu_dm_connector *aconnector,

  if (recalculate_timing)
  drm_mode_set_crtcinfo(_mode, 0);
-    else if (!old_stream)
-    drm_mode_set_crtcinfo(, 0);
  /*
   * If scaling is enabled and refresh rate didn't change
@@ -6691,6 +6689,8 @@ enum drm_mode_status 
amdgpu_dm_connector_mode_valid(struct drm_connector *connec

  goto fail;
  }
+    drm_mode_set_crtcinfo(mode, 0);
+
  stream = create_validate_stream_for_sink(aconnector, mode,
   to_dm_connector_state(connector->state),
   NULL);



--
Hamza

Re: [PATCH] drm/amd/display: fix the ability to use lower resolution modes on eDP

[Hamza Mahfooz]

On 9/14/23 16:40, Harry Wentland wrote:

On 2023-09-14 13:53, Hamza Mahfooz wrote:

On eDP we can receive invalid modes from dm_update_crtc_state() for
entirely new streams for which drm_mode_set_crtcinfo() shouldn't be
called on. So, instead of calling drm_mode_set_crtcinfo() from within
create_stream_for_sink() we can instead call it from
amdgpu_dm_connector_mode_valid(). Since, we are guaranteed to only call
drm_mode_set_crtcinfo() for valid modes from that function (invalid
modes are rejected by that callback) and that is the only user
of create_validate_stream_for_sink() that we need to call
drm_mode_set_crtcinfo() for (as before commit cb841d27b876
("drm/amd/display: Always pass connector_state to stream validation"),
that is the only place where create_validate_stream_for_sink()'s
dm_state was NULL).



I don't seem to see how a NULL dm_state in
create_validate_stream_for_sink() (or create_stream_for_sink() for that
matter) has an impact on the drm_mode_set_crtcinfo() call. That one depends
on !old_stream and 


If we look back to commit 4a2df0d1f28e ("drm/amd/display: Fixed
non-native modes not lighting up") it seems like the intent was to only
have drm_mode_set_crtcinfo() called for
amdgpu_dm_connector_mode_valid(). Since, even if we go that far back
create_stream_for_sink()'s dm_state was only NULL when it was called
from amdgpu_dm_connector_mode_valid().



It does look like  is an empty mode if we can't find a preferred_mode,
though. Not sure if that can cause an issue.


I don't think it should be an issue, since before commit 4a2df0d1f28e
("drm/amd/display: Fixed non-native modes not lighting up") we always
called drm_mode_set_crtcinfo() in the aforementioned case (and only for 
that case).




Harry


Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2693
Fixes: cb841d27b876 ("drm/amd/display: Always pass connector_state to stream 
validation")
Signed-off-by: Hamza Mahfooz 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

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 933c9b5d5252..beef4fef7338 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6128,8 +6128,6 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,
  
  	if (recalculate_timing)

drm_mode_set_crtcinfo(_mode, 0);
-   else if (!old_stream)
-   drm_mode_set_crtcinfo(, 0);
  
  	/*

 * If scaling is enabled and refresh rate didn't change
@@ -6691,6 +6689,8 @@ enum drm_mode_status 
amdgpu_dm_connector_mode_valid(struct drm_connector *connec
goto fail;
}
  
+	drm_mode_set_crtcinfo(mode, 0);

+
stream = create_validate_stream_for_sink(aconnector, mode,
 
to_dm_connector_state(connector->state),
 NULL);



--
Hamza

[PATCH] drm/amd/display: fix the ability to use lower resolution modes on eDP

[Hamza Mahfooz]

On eDP we can receive invalid modes from dm_update_crtc_state() for
entirely new streams for which drm_mode_set_crtcinfo() shouldn't be
called on. So, instead of calling drm_mode_set_crtcinfo() from within
create_stream_for_sink() we can instead call it from
amdgpu_dm_connector_mode_valid(). Since, we are guaranteed to only call
drm_mode_set_crtcinfo() for valid modes from that function (invalid
modes are rejected by that callback) and that is the only user
of create_validate_stream_for_sink() that we need to call
drm_mode_set_crtcinfo() for (as before commit cb841d27b876
("drm/amd/display: Always pass connector_state to stream validation"),
that is the only place where create_validate_stream_for_sink()'s
dm_state was NULL).

Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2693
Fixes: cb841d27b876 ("drm/amd/display: Always pass connector_state to stream 
validation")
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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 933c9b5d5252..beef4fef7338 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6128,8 +6128,6 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,
 
if (recalculate_timing)
drm_mode_set_crtcinfo(_mode, 0);
-   else if (!old_stream)
-   drm_mode_set_crtcinfo(, 0);
 
/*
 * If scaling is enabled and refresh rate didn't change
@@ -6691,6 +6689,8 @@ enum drm_mode_status 
amdgpu_dm_connector_mode_valid(struct drm_connector *connec
goto fail;
}
 
+   drm_mode_set_crtcinfo(mode, 0);
+
stream = create_validate_stream_for_sink(aconnector, mode,
 
to_dm_connector_state(connector->state),
 NULL);
-- 
2.42.0

Re: [PATCH] drm/amd/display: Fix -Wuninitialized in dm_helpers_dp_mst_send_payload_allocation()

[Hamza Mahfooz]

On 9/13/23 16:03, Alex Deucher wrote:

On Wed, Sep 13, 2023 at 3:57 PM Hamza Mahfooz  wrote:



On 9/13/23 15:54, Alex Deucher wrote:

On Wed, Sep 13, 2023 at 12:17 PM Hamza Mahfooz  wrote:



On 9/13/23 12:10, Nathan Chancellor wrote:

When building with clang, there is a warning (or error when
CONFIG_WERROR is set):

 
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:368:21: 
error: variable 'old_payload' is uninitialized when used here 
[-Werror,-Wuninitialized]
   368 |  new_payload, 
old_payload);
   |   
^~~
 
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:344:61: 
note: initialize the variable 'old_payload' to silence this warning
   344 | struct drm_dp_mst_atomic_payload *new_payload, 
*old_payload;
   |
^
   |
 = NULL
 1 error generated.

This variable is not required outside of this function so allocate
old_payload on the stack and pass it by reference to
dm_helpers_construct_old_payload(), resolving the warning.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1931
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload 
allocation/removement")
Signed-off-by: Nathan Chancellor 


Reviewed-by: Hamza Mahfooz 

Hm, seems like this was pushed through drm-misc-next and as such our CI
didn't get a chance to test it.


Can you push this to drm-misc?  That is where Wayne's patches landed.
If not, I can push it.


No need, I cherry-picked Wayne's patches to amd-staging-drm-next and
applied Nathan's fix.


Yes, but we can only have patches go upstream via one tree.  Wayne's
patches are in drm-misc, so that is where the fix should be.  It's
fine to have them in amd-staging-drm-next for testing purposes, but I
won't be upstreaming them via the amdgpu -next tree since they are
already in drm-misc.


In that case can you push it to drm-misc?



Alex





Alex






---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

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 9ad509279b0a..c4c35f6844f4 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
@@ -341,7 +341,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_state *mst_state;
struct drm_dp_mst_topology_mgr *mst_mgr;
- struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
+ struct drm_dp_mst_atomic_payload *new_payload, old_payload;
enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD;
enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD;
int ret = 0;
@@ -365,8 +365,8 @@ bool dm_helpers_dp_mst_send_payload_allocation(
ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, 
new_payload);
} else {
dm_helpers_construct_old_payload(stream->link, 
mst_state->pbn_div,
-  new_payload, old_payload);
- drm_dp_remove_payload_part2(mst_mgr, mst_state, old_payload, 
new_payload);
+  new_payload, _payload);
+ drm_dp_remove_payload_part2(mst_mgr, mst_state, _payload, 
new_payload);
}

if (ret) {

---
base-commit: 8569c31545385195bdb0c021124e68336e91c693
change-id: 
20230913-fix-wuninitialized-dm_helpers_dp_mst_send_payload_allocation-c37b33aaad18

Best regards,

--
Hamza


--
Hamza


--
Hamza

Re: [PATCH] drm/amd/display: Fix -Wuninitialized in dm_helpers_dp_mst_send_payload_allocation()

[Hamza Mahfooz]

On 9/13/23 15:54, Alex Deucher wrote:

On Wed, Sep 13, 2023 at 12:17 PM Hamza Mahfooz  wrote:



On 9/13/23 12:10, Nathan Chancellor wrote:

When building with clang, there is a warning (or error when
CONFIG_WERROR is set):

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:368:21: 
error: variable 'old_payload' is uninitialized when used here 
[-Werror,-Wuninitialized]
  368 |  new_payload, 
old_payload);
  |   
^~~
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:344:61: 
note: initialize the variable 'old_payload' to silence this warning
  344 | struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
  |^
  | 
= NULL
1 error generated.

This variable is not required outside of this function so allocate
old_payload on the stack and pass it by reference to
dm_helpers_construct_old_payload(), resolving the warning.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1931
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload 
allocation/removement")
Signed-off-by: Nathan Chancellor 


Reviewed-by: Hamza Mahfooz 

Hm, seems like this was pushed through drm-misc-next and as such our CI
didn't get a chance to test it.


Can you push this to drm-misc?  That is where Wayne's patches landed.
If not, I can push it.


No need, I cherry-picked Wayne's patches to amd-staging-drm-next and
applied Nathan's fix.



Alex






---
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)

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 9ad509279b0a..c4c35f6844f4 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
@@ -341,7 +341,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
   struct amdgpu_dm_connector *aconnector;
   struct drm_dp_mst_topology_state *mst_state;
   struct drm_dp_mst_topology_mgr *mst_mgr;
- struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
+ struct drm_dp_mst_atomic_payload *new_payload, old_payload;
   enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD;
   enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD;
   int ret = 0;
@@ -365,8 +365,8 @@ bool dm_helpers_dp_mst_send_payload_allocation(
   ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, 
new_payload);
   } else {
   dm_helpers_construct_old_payload(stream->link, 
mst_state->pbn_div,
-  new_payload, old_payload);
- drm_dp_remove_payload_part2(mst_mgr, mst_state, old_payload, 
new_payload);
+  new_payload, _payload);
+ drm_dp_remove_payload_part2(mst_mgr, mst_state, _payload, 
new_payload);
   }

   if (ret) {

---
base-commit: 8569c31545385195bdb0c021124e68336e91c693
change-id: 
20230913-fix-wuninitialized-dm_helpers_dp_mst_send_payload_allocation-c37b33aaad18

Best regards,

--
Hamza


--
Hamza

Re: [PATCH] drm/amd/display: Fix -Wuninitialized in dm_helpers_dp_mst_send_payload_allocation()

[Hamza Mahfooz]

On 9/13/23 12:10, Nathan Chancellor wrote:

When building with clang, there is a warning (or error when
CONFIG_WERROR is set):

   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:368:21: 
error: variable 'old_payload' is uninitialized when used here 
[-Werror,-Wuninitialized]
 368 |  new_payload, 
old_payload);
 |   
^~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:344:61: 
note: initialize the variable 'old_payload' to silence this warning
 344 | struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
 |^
 | 
= NULL
   1 error generated.

This variable is not required outside of this function so allocate
old_payload on the stack and pass it by reference to
dm_helpers_construct_old_payload(), resolving the warning.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1931
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload 
allocation/removement")
Signed-off-by: Nathan Chancellor 


Applied, thanks!


---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

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 9ad509279b0a..c4c35f6844f4 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
@@ -341,7 +341,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_state *mst_state;
struct drm_dp_mst_topology_mgr *mst_mgr;
-   struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
+   struct drm_dp_mst_atomic_payload *new_payload, old_payload;
enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD;
enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD;
int ret = 0;
@@ -365,8 +365,8 @@ bool dm_helpers_dp_mst_send_payload_allocation(
ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, 
new_payload);
} else {
dm_helpers_construct_old_payload(stream->link, 
mst_state->pbn_div,
-new_payload, old_payload);
-   drm_dp_remove_payload_part2(mst_mgr, mst_state, old_payload, 
new_payload);
+new_payload, _payload);
+   drm_dp_remove_payload_part2(mst_mgr, mst_state, _payload, 
new_payload);
}
  
  	if (ret) {


---
base-commit: 8569c31545385195bdb0c021124e68336e91c693
change-id: 
20230913-fix-wuninitialized-dm_helpers_dp_mst_send_payload_allocation-c37b33aaad18

Best regards,

--
Hamza

Re: [PATCH] drm/amd/display: Fix -Wuninitialized in dm_helpers_dp_mst_send_payload_allocation()

[Hamza Mahfooz]

On 9/13/23 12:10, Nathan Chancellor wrote:

When building with clang, there is a warning (or error when
CONFIG_WERROR is set):

   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:368:21: 
error: variable 'old_payload' is uninitialized when used here 
[-Werror,-Wuninitialized]
 368 |  new_payload, 
old_payload);
 |   
^~~
   drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:344:61: 
note: initialize the variable 'old_payload' to silence this warning
 344 | struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
 |^
 | 
= NULL
   1 error generated.

This variable is not required outside of this function so allocate
old_payload on the stack and pass it by reference to
dm_helpers_construct_old_payload(), resolving the warning.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1931
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload 
allocation/removement")
Signed-off-by: Nathan Chancellor 


Reviewed-by: Hamza Mahfooz 

Hm, seems like this was pushed through drm-misc-next and as such our CI
didn't get a chance to test it.



---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

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 9ad509279b0a..c4c35f6844f4 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
@@ -341,7 +341,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_state *mst_state;
struct drm_dp_mst_topology_mgr *mst_mgr;
-   struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
+   struct drm_dp_mst_atomic_payload *new_payload, old_payload;
enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD;
enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD;
int ret = 0;
@@ -365,8 +365,8 @@ bool dm_helpers_dp_mst_send_payload_allocation(
ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, 
new_payload);
} else {
dm_helpers_construct_old_payload(stream->link, 
mst_state->pbn_div,
-new_payload, old_payload);
-   drm_dp_remove_payload_part2(mst_mgr, mst_state, old_payload, 
new_payload);
+new_payload, _payload);
+   drm_dp_remove_payload_part2(mst_mgr, mst_state, _payload, 
new_payload);
}
  
  	if (ret) {


---
base-commit: 8569c31545385195bdb0c021124e68336e91c693
change-id: 
20230913-fix-wuninitialized-dm_helpers_dp_mst_send_payload_allocation-c37b33aaad18

Best regards,

--
Hamza

Re: [PATCH v4 0/2] Merge all debug module parameters

[Hamza Mahfooz]

On 9/11/23 13:12, André Almeida wrote:

As suggested by Christian at [0], this patchset merges all debug modules
parameters and creates a new one for disabling soft recovery:


Maybe we can overload the amdgpu_gpu_recovery module option with this.
Or even better merge all the developer module parameter into a
amdgpu_debug option. This way it should be pretty obvious that this
isn't meant to be used by someone who doesn't know how to use it.


[0] 
https://lore.kernel.org/dri-devel/55f69184-1aa2-55d6-4a10-1560d75c7...@amd.com/


I have applied the series, thanks!



Changelog:
- rebased on top of current amd-staging-drm-next
v3: https://lore.kernel.org/lkml/20230831152903.521404-1-andrealm...@igalia.com

- move enum from include/amd_shared.h to amdgpu/amdgpu_drv.c
v2: https://lore.kernel.org/lkml/20230830220808.421935-1-andrealm...@igalia.com/

- drop old module params
- use BIT() macros
- replace global var with adev-> vars
v1: https://lore.kernel.org/lkml/20230824162505.173399-1-andrealm...@igalia.com/

André Almeida (2):
   drm/amdgpu: Merge debug module parameters
   drm/amdgpu: Create an option to disable soft recovery

  drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  5 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 63 
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c  |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c |  7 ++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   |  2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c |  2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_crat.c|  2 +-
  8 files changed, 59 insertions(+), 26 deletions(-)


--
Hamza

Re: [PATCH v3 0/2] Merge all debug module parameters

[Hamza Mahfooz]

On 9/11/23 09:54, André Almeida wrote:

Christian, Alex, I think this series is ready to be picked as well.


Can you rebase this onto amd-staging-drm-next
(https://gitlab.freedesktop.org/agd5f/linux)? Since it currently doesn't
apply there.



Em 31/08/2023 12:29, André Almeida escreveu:

As suggested by Christian at [0], this patchset merges all debug modules
parameters and creates a new one for disabling soft recovery:


Maybe we can overload the amdgpu_gpu_recovery module option with this.
Or even better merge all the developer module parameter into a
amdgpu_debug option. This way it should be pretty obvious that this
isn't meant to be used by someone who doesn't know how to use it.


[0] 
https://lore.kernel.org/dri-devel/55f69184-1aa2-55d6-4a10-1560d75c7...@amd.com/


Changelog:
- move enum from include/amd_shared.h to amdgpu/amdgpu_drv.c
v2: 
https://lore.kernel.org/lkml/20230830220808.421935-1-andrealm...@igalia.com/


- drop old module params
- use BIT() macros
- replace global var with adev-> vars
v1: 
https://lore.kernel.org/lkml/20230824162505.173399-1-andrealm...@igalia.com/


André Almeida (2):
   drm/amdgpu: Merge debug module parameters
   drm/amdgpu: Create an option to disable soft recovery

  drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  5 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 63 
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c  |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c |  6 ++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   |  2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c |  2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_crat.c    |  2 +-
  8 files changed, 58 insertions(+), 26 deletions(-)


--
Hamza

Re: [PATCH] drm/amd/display: fix replay_mode kernel-doc warning

[Hamza Mahfooz]

On 9/10/23 19:44, Randy Dunlap wrote:

Fix the typo in the kernel-doc for @replay_mode to prevent
kernel-doc warnings:

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:623: warning: Incorrect use 
of kernel-doc format:  * @replay mode: Replay supported
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:626: warning: Function 
parameter or member 'replay_mode' not described in 'amdgpu_hdmi_vsdb_info'

Fixes: ec8e59cb4e0c ("drm/amd/display: Get replay info from VSDB")
Signed-off-by: Randy Dunlap 
Reported-by: kernel test robot 
Cc: Bhawanpreet Lakha 
Cc: Harry Wentland 
Cc: Alex Deucher 
Cc: Leo Li 
Cc: Rodrigo Siqueira 
Cc: amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org


Applied, thanks!


---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff -- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -620,7 +620,7 @@ struct amdgpu_hdmi_vsdb_info {
unsigned int max_refresh_rate_hz;
  
  	/**

-* @replay mode: Replay supported
+* @replay_mode: Replay supported
 */
bool replay_mode;
  };

--
Hamza

Re: [PATCH] drm/amd/display: clean up some inconsistent indenting

[Hamza Mahfooz]

On 9/8/23 03:54, Jiapeng Chong wrote:

No functional modification involved.

drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_dpms.c:2476 
link_set_dpms_on() warn: if statement not indented.

Reported-by: Abaci Robot 
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=6502
Signed-off-by: Jiapeng Chong 


Applied, thanks!


---
  drivers/gpu/drm/amd/display/dc/link/link_dpms.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c 
b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
index cd9dd270b05f..e7e528c68cb6 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
@@ -2474,9 +2474,8 @@ void link_set_dpms_on(
 */
if (pipe_ctx->stream->timing.flags.DSC) {
if (dc_is_dp_signal(pipe_ctx->stream->signal) ||
-   dc_is_virtual_signal(pipe_ctx->stream->signal))
-   link_set_dsc_enable(pipe_ctx, true);
-
+   dc_is_virtual_signal(pipe_ctx->stream->signal))
+   link_set_dsc_enable(pipe_ctx, true);
}
  
  	status = enable_link(state, pipe_ctx);

--
Hamza

[PATCH v2 2/2] Revert "drm/amd: Disable S/G for APUs when 64GB or more host memory"

[Hamza Mahfooz]

This reverts commit 5b7a256c982636ebc4f16b708b40ff56d33c8a86.

Since, we now have an actual fix for this issue, we can get rid of this
workaround as it can cause pin failures if enough VRAM isn't carved out
by the BIOS.

Cc: sta...@vger.kernel.org # 6.1+
Signed-off-by: Hamza Mahfooz 
---
v2: new to the series
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 26 ---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  5 ++--
 3 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 83a9607a87b8..3a86d11d1605 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1316,7 +1316,6 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 void amdgpu_device_pci_config_reset(struct amdgpu_device *adev);
 int amdgpu_device_pci_reset(struct amdgpu_device *adev);
 bool amdgpu_device_need_post(struct amdgpu_device *adev);
-bool amdgpu_sg_display_supported(struct amdgpu_device *adev);
 bool amdgpu_device_pcie_dynamic_switching_supported(void);
 bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev);
 bool amdgpu_device_aspm_support_quirk(void);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5f32e8d4f3d3..3d540b0cf0e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1358,32 +1358,6 @@ bool amdgpu_device_need_post(struct amdgpu_device *adev)
return true;
 }
 
-/*
- * On APUs with >= 64GB white flickering has been observed w/ SG enabled.
- * Disable S/G on such systems until we have a proper fix.
- * https://gitlab.freedesktop.org/drm/amd/-/issues/2354
- * https://gitlab.freedesktop.org/drm/amd/-/issues/2735
- */
-bool amdgpu_sg_display_supported(struct amdgpu_device *adev)
-{
-   switch (amdgpu_sg_display) {
-   case -1:
-   break;
-   case 0:
-   return false;
-   case 1:
-   return true;
-   default:
-   return false;
-   }
-   if ((totalram_pages() << (PAGE_SHIFT - 10)) +
-   (adev->gmc.real_vram_size / 1024) >= 6400) {
-   DRM_WARN("Disabling S/G due to >=64GB RAM\n");
-   return false;
-   }
-   return true;
-}
-
 /*
  * Intel hosts such as Raptor Lake and Sapphire Rapids don't support dynamic
  * speed switching. Until we have confirmation from Intel that a specific host
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 5f14cd9391ca..740a6fcafe4c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1654,8 +1654,9 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
}
break;
}
-   if (init_data.flags.gpu_vm_support)
-   init_data.flags.gpu_vm_support = 
amdgpu_sg_display_supported(adev);
+   if (init_data.flags.gpu_vm_support &&
+   (amdgpu_sg_display == 0))
+   init_data.flags.gpu_vm_support = false;
 
if (init_data.flags.gpu_vm_support)
adev->mode_info.gpu_vm_support = true;
-- 
2.41.0

[PATCH v2 1/2] drm/amd/display: fix the white screen issue when >= 64GB DRAM

[Hamza Mahfooz]

From: Yifan Zhang 

Dropping bit 31:4 of page table base is wrong, it makes page table
base points to wrong address if phys addr is beyond 64GB; dropping
page_table_start/end bit 31:4 is unnecessary since dcn20_vmid_setup
will do that. Also, while we are at it, cleanup the assignments using
upper_32_bits()/lower_32_bits() and AMDGPU_GPU_PAGE_SHIFT.

Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2354
Fixes: 81d0bcf99009 ("drm/amdgpu: make display pinning more flexible (v2)")
Signed-off-by: Yifan Zhang 
Signed-off-by: Hamza Mahfooz 
---
v2: use upper_32_bits()/lower_32_bits() and AMDGPU_GPU_PAGE_SHIFT
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

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 1bb1a394f55f..5f14cd9391ca 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1283,11 +1283,15 @@ static void mmhub_read_system_context(struct 
amdgpu_device *adev, struct dc_phy_
 
pt_base = amdgpu_gmc_pd_addr(adev->gart.bo);
 
-   page_table_start.high_part = (u32)(adev->gmc.gart_start >> 44) & 0xF;
-   page_table_start.low_part = (u32)(adev->gmc.gart_start >> 12);
-   page_table_end.high_part = (u32)(adev->gmc.gart_end >> 44) & 0xF;
-   page_table_end.low_part = (u32)(adev->gmc.gart_end >> 12);
-   page_table_base.high_part = upper_32_bits(pt_base) & 0xF;
+   page_table_start.high_part = upper_32_bits(adev->gmc.gart_start >>
+  AMDGPU_GPU_PAGE_SHIFT);
+   page_table_start.low_part = lower_32_bits(adev->gmc.gart_start >>
+ AMDGPU_GPU_PAGE_SHIFT);
+   page_table_end.high_part = upper_32_bits(adev->gmc.gart_end >>
+AMDGPU_GPU_PAGE_SHIFT);
+   page_table_end.low_part = lower_32_bits(adev->gmc.gart_end >>
+   AMDGPU_GPU_PAGE_SHIFT);
+   page_table_base.high_part = upper_32_bits(pt_base);
page_table_base.low_part = lower_32_bits(pt_base);
 
pa_config->system_aperture.start_addr = (uint64_t)logical_addr_low << 
18;
-- 
2.41.0

[PATCH] drm/amd/display: prevent potential division by zero errors

[Hamza Mahfooz]

There are two places in apply_below_the_range() where it's possible for
a divide by zero error to occur. So, to fix this make sure the divisor
is non-zero before attempting the computation in both cases.

Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2637
Fixes: a463b263032f ("drm/amd/display: Fix frames_to_insert math")
Fixes: ded6119e825a ("drm/amd/display: Reinstate LFC optimization")
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c 
b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
index dbd60811f95d..ef3a67409021 100644
--- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
+++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
@@ -338,7 +338,9 @@ static void apply_below_the_range(struct core_freesync 
*core_freesync,
 *  - Delta for CEIL: delta_from_mid_point_in_us_1
 *  - Delta for FLOOR: delta_from_mid_point_in_us_2
 */
-   if ((last_render_time_in_us / mid_point_frames_ceil) < 
in_out_vrr->min_duration_in_us) {
+   if (mid_point_frames_ceil &&
+   (last_render_time_in_us / mid_point_frames_ceil) <
+   in_out_vrr->min_duration_in_us) {
/* Check for out of range.
 * If using CEIL produces a value that is out of range,
 * then we are forced to use FLOOR.
@@ -385,8 +387,9 @@ static void apply_below_the_range(struct core_freesync 
*core_freesync,
/* Either we've calculated the number of frames to insert,
 * or we need to insert min duration frames
 */
-   if (last_render_time_in_us / frames_to_insert <
-   in_out_vrr->min_duration_in_us){
+   if (frames_to_insert &&
+   (last_render_time_in_us / frames_to_insert) <
+   in_out_vrr->min_duration_in_us){
frames_to_insert -= (frames_to_insert > 1) ?
1 : 0;
}
-- 
2.41.0

[PATCH v2 2/2] drm/amd/display: limit the v_startup workaround for ASICs older than DCN3.1

[Hamza Mahfooz]

Since, calling dcn20_adjust_freesync_v_startup() on DCN3.1+ ASICs
can cause the display to flicker and underflow to occur we shouldn't
call it for them. So, ensure that the DCN version is less than
DCN_VERSION_3_1 before calling dcn20_adjust_freesync_v_startup().

Cc: sta...@vger.kernel.org
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
index 1bfdf0271fdf..a68fb45ed487 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
@@ -1099,7 +1099,8 @@ void dcn20_calculate_dlg_params(struct dc *dc,
context->res_ctx.pipe_ctx[i].plane_res.bw.dppclk_khz =

pipes[pipe_idx].clks_cfg.dppclk_mhz * 1000;
context->res_ctx.pipe_ctx[i].pipe_dlg_param = 
pipes[pipe_idx].pipe.dest;
-   if 
(context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
+   if (dc->ctx->dce_version < DCN_VERSION_3_1 &&
+   
context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
dcn20_adjust_freesync_v_startup(
>res_ctx.pipe_ctx[i].stream->timing,

>res_ctx.pipe_ctx[i].pipe_dlg_param.vstartup_start);
-- 
2.41.0

[PATCH v2 1/2] Revert "drm/amd/display: Remove v_startup workaround for dcn3+"

[Hamza Mahfooz]

This reverts commit 3a31e8b89b7240d9a17ace8a1ed050bdcb560f9e.

We still need to call dcn20_adjust_freesync_v_startup() for older DCN3+
ASICs otherwise it can cause DP to HDMI 2.1 PCONs to fail to light up.

Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2809
Signed-off-by: Hamza Mahfooz 
---
 .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c  | 24 ---
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
index 0989a0152ae8..1bfdf0271fdf 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
@@ -1099,6 +1099,10 @@ void dcn20_calculate_dlg_params(struct dc *dc,
context->res_ctx.pipe_ctx[i].plane_res.bw.dppclk_khz =

pipes[pipe_idx].clks_cfg.dppclk_mhz * 1000;
context->res_ctx.pipe_ctx[i].pipe_dlg_param = 
pipes[pipe_idx].pipe.dest;
+   if 
(context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
+   dcn20_adjust_freesync_v_startup(
+   >res_ctx.pipe_ctx[i].stream->timing,
+   
>res_ctx.pipe_ctx[i].pipe_dlg_param.vstartup_start);
 
pipe_idx++;
}
@@ -1927,7 +1931,6 @@ static bool dcn20_validate_bandwidth_internal(struct dc 
*dc, struct dc_state *co
int vlevel = 0;
int pipe_split_from[MAX_PIPES];
int pipe_cnt = 0;
-   int i = 0;
display_e2e_pipe_params_st *pipes = kzalloc(dc->res_pool->pipe_count * 
sizeof(display_e2e_pipe_params_st), GFP_ATOMIC);
DC_LOGGER_INIT(dc->ctx->logger);
 
@@ -1951,15 +1954,6 @@ static bool dcn20_validate_bandwidth_internal(struct dc 
*dc, struct dc_state *co
dcn20_calculate_wm(dc, context, pipes, _cnt, pipe_split_from, 
vlevel, fast_validate);
dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
 
-   for (i = 0; i < dc->res_pool->pipe_count; i++) {
-   if (!context->res_ctx.pipe_ctx[i].stream)
-   continue;
-   if 
(context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
-   dcn20_adjust_freesync_v_startup(
-   >res_ctx.pipe_ctx[i].stream->timing,
-   
>res_ctx.pipe_ctx[i].pipe_dlg_param.vstartup_start);
-   }
-
BW_VAL_TRACE_END_WATERMARKS();
 
goto validate_out;
@@ -2232,7 +2226,6 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc,
int vlevel = 0;
int pipe_split_from[MAX_PIPES];
int pipe_cnt = 0;
-   int i = 0;
display_e2e_pipe_params_st *pipes = kzalloc(dc->res_pool->pipe_count * 
sizeof(display_e2e_pipe_params_st), GFP_ATOMIC);
DC_LOGGER_INIT(dc->ctx->logger);
 
@@ -2261,15 +2254,6 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc,
dcn21_calculate_wm(dc, context, pipes, _cnt, pipe_split_from, 
vlevel, fast_validate);
dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
 
-   for (i = 0; i < dc->res_pool->pipe_count; i++) {
-   if (!context->res_ctx.pipe_ctx[i].stream)
-   continue;
-   if 
(context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
-   dcn20_adjust_freesync_v_startup(
-   >res_ctx.pipe_ctx[i].stream->timing,
-   
>res_ctx.pipe_ctx[i].pipe_dlg_param.vstartup_start);
-   }
-
BW_VAL_TRACE_END_WATERMARKS();
 
goto validate_out;
-- 
2.41.0

[PATCH] Revert "drm/amd/display: Remove v_startup workaround for dcn3+"

[Hamza Mahfooz]

This reverts commit 3a31e8b89b7240d9a17ace8a1ed050bdcb560f9e.

We still need to call dcn20_adjust_freesync_v_startup() for older DCN3+
ASICs otherwise it can cause DP to HDMI 2.1 PCONs to fail to light up.
So, reintroduce the reverted code and limit it to ASICs older than
DCN31.

Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2809
Signed-off-by: Hamza Mahfooz 
---
 .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c  | 24 ---
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
index 0989a0152ae8..0841176e8d6c 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c
@@ -1099,6 +1099,10 @@ void dcn20_calculate_dlg_params(struct dc *dc,
context->res_ctx.pipe_ctx[i].plane_res.bw.dppclk_khz =

pipes[pipe_idx].clks_cfg.dppclk_mhz * 1000;
context->res_ctx.pipe_ctx[i].pipe_dlg_param = 
pipes[pipe_idx].pipe.dest;
+   if (dc->ctx->dce_version < DCN_VERSION_3_1 &&
+   
context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
+   
dcn20_adjust_freesync_v_startup(>res_ctx.pipe_ctx[i].stream->timing,
+   
>res_ctx.pipe_ctx[i].pipe_dlg_param.vstartup_start);
 
pipe_idx++;
}
@@ -1927,7 +1931,6 @@ static bool dcn20_validate_bandwidth_internal(struct dc 
*dc, struct dc_state *co
int vlevel = 0;
int pipe_split_from[MAX_PIPES];
int pipe_cnt = 0;
-   int i = 0;
display_e2e_pipe_params_st *pipes = kzalloc(dc->res_pool->pipe_count * 
sizeof(display_e2e_pipe_params_st), GFP_ATOMIC);
DC_LOGGER_INIT(dc->ctx->logger);
 
@@ -1951,15 +1954,6 @@ static bool dcn20_validate_bandwidth_internal(struct dc 
*dc, struct dc_state *co
dcn20_calculate_wm(dc, context, pipes, _cnt, pipe_split_from, 
vlevel, fast_validate);
dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
 
-   for (i = 0; i < dc->res_pool->pipe_count; i++) {
-   if (!context->res_ctx.pipe_ctx[i].stream)
-   continue;
-   if 
(context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
-   dcn20_adjust_freesync_v_startup(
-   >res_ctx.pipe_ctx[i].stream->timing,
-   
>res_ctx.pipe_ctx[i].pipe_dlg_param.vstartup_start);
-   }
-
BW_VAL_TRACE_END_WATERMARKS();
 
goto validate_out;
@@ -2232,7 +2226,6 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc,
int vlevel = 0;
int pipe_split_from[MAX_PIPES];
int pipe_cnt = 0;
-   int i = 0;
display_e2e_pipe_params_st *pipes = kzalloc(dc->res_pool->pipe_count * 
sizeof(display_e2e_pipe_params_st), GFP_ATOMIC);
DC_LOGGER_INIT(dc->ctx->logger);
 
@@ -2261,15 +2254,6 @@ bool dcn21_validate_bandwidth_fp(struct dc *dc,
dcn21_calculate_wm(dc, context, pipes, _cnt, pipe_split_from, 
vlevel, fast_validate);
dcn20_calculate_dlg_params(dc, context, pipes, pipe_cnt, vlevel);
 
-   for (i = 0; i < dc->res_pool->pipe_count; i++) {
-   if (!context->res_ctx.pipe_ctx[i].stream)
-   continue;
-   if 
(context->res_ctx.pipe_ctx[i].stream->adaptive_sync_infopacket.valid)
-   dcn20_adjust_freesync_v_startup(
-   >res_ctx.pipe_ctx[i].stream->timing,
-   
>res_ctx.pipe_ctx[i].pipe_dlg_param.vstartup_start);
-   }
-
BW_VAL_TRACE_END_WATERMARKS();
 
goto validate_out;
-- 
2.41.0

[PATCH] Revert "Revert "drm/amd/display: Implement zpos property""

[Hamza Mahfooz]

This reverts commit 984612bd4649c91f12e9c7c7f9e914fdc8ba7d3f.

The problematic IGT test case (i.e. kms_atomic@plane-immutable-zpos) has
been fixed as of commit cb77add45011 ("tests/kms_atomic: remove zpos <
N-planes assert") to the IGT repo. So, reintroduce the reverted code.

Link: 
https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/commit/cb77add45011b129e21f3cb2a4089a73dde56179
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 9 +
 1 file changed, 9 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 894bc7e4fdaa..df568a7cd005 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
@@ -1469,6 +1469,15 @@ int amdgpu_dm_plane_init(struct amdgpu_display_manager 
*dm,
drm_plane_create_blend_mode_property(plane, blend_caps);
}
 
+   if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
+   drm_plane_create_zpos_immutable_property(plane, 0);
+   } else if (plane->type == DRM_PLANE_TYPE_OVERLAY) {
+   unsigned int zpos = 1 + drm_plane_index(plane);
+   drm_plane_create_zpos_property(plane, zpos, 1, 254);
+   } else if (plane->type == DRM_PLANE_TYPE_CURSOR) {
+   drm_plane_create_zpos_immutable_property(plane, 255);
+   }
+
if (plane->type == DRM_PLANE_TYPE_PRIMARY &&
plane_cap &&
(plane_cap->pixel_format_support.nv12 ||
-- 
2.41.0

Re: [PATCH (set 1) 00/20] Rid W=1 warnings from GPU

[Hamza Mahfooz]

On 8/24/23 08:07, Lee Jones wrote:

On Thu, 24 Aug 2023, Jani Nikula wrote:


On Thu, 24 Aug 2023, Lee Jones  wrote:

This set is part of a larger effort attempting to clean-up W=1
kernel builds, which are currently overwhelmingly riddled with
niggly little warnings.


The next question is, how do we keep it W=1 clean going forward?


My plan was to fix them all, then move each warning to W=0.

Arnd recently submitted a set doing just that for a bunch of them.

https://lore.kernel.org/all/20230811140327.3754597-1-a...@kernel.org/

I like to think a bunch of this is built on top of my previous efforts.

GPU is a particularly tricky though - the warnings seem to come in faster
than I can squash them.  Maybe the maintainers can find a way to test
new patches on merge?


I guess on that note, do you know if there is a way to run
`scripts/kernel-doc` on patches instead of whole files? That would make
much easier to block new kernel-doc issues from appearing.




Most people don't use W=1 because it's too noisy, so it's a bit of a
catch-22.

In i915, we enable a lot of W=1 warnings using subdir-ccflags-y in our
Makefile. For CI/developer use we also enable kernel-doc warnings by
default.

Should we start enabling some of those warning flags in drm/Makefile to
to keep the entire subsystem warning free?


That would we awesome!  We'd just need buy-in.


--
Hamza

Re: [PATCH v2] drm/amdgpu: register a dirty framebuffer callback for fbcon

[Hamza Mahfooz]

On 8/23/23 16:51, Alex Deucher wrote:

@Mahfooz, Hamza
  can you respin with the NULL check?


sure.



Alex

On Wed, Aug 16, 2023 at 10:25 AM Christian König
 wrote:


Am 16.08.23 um 15:41 schrieb Hamza Mahfooz:


On 8/16/23 01:55, Christian König wrote:



Am 15.08.23 um 19:26 schrieb Hamza Mahfooz:

fbcon requires that we implement _framebuffer_funcs.dirty.
Otherwise, the framebuffer might take a while to flush (which would
manifest as noticeable lag). However, we can't enable this callback for
non-fbcon cases since it might cause too many atomic commits to be made
at once. So, implement amdgpu_dirtyfb() and only enable it for fbcon
framebuffers on devices that support atomic KMS.

Cc: Aurabindo Pillai 
Cc: Mario Limonciello 
Cc: sta...@vger.kernel.org # 6.1+
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2519
Signed-off-by: Hamza Mahfooz 
---
v2: update variable names
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 26
-
   1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index d20dd3f852fc..d3b59f99cb7c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -38,6 +38,8 @@
   #include 
   #include 
   #include 
+#include 
+#include 
   #include 
   #include 
   #include 
@@ -532,11 +534,29 @@ bool amdgpu_display_ddc_probe(struct
amdgpu_connector *amdgpu_connector,
   return true;
   }
+static int amdgpu_dirtyfb(struct drm_framebuffer *fb, struct
drm_file *file,
+  unsigned int flags, unsigned int color,
+  struct drm_clip_rect *clips, unsigned int num_clips)
+{
+
+if (strcmp(fb->comm, "[fbcon]"))
+return -ENOSYS;


Once more to the v2 of this patch: Tests like those are a pretty big
NO-GO for upstreaming.


On closer inspection it is actually sufficient to check if `file` is
NULL here (since it means that the request isn't from userspace). So, do
you think that would be palatable for upstream?


That's certainly better than doing a string compare, but I'm not sure if
that's sufficient.

In general drivers shouldn't have any special handling for fdcon.

You should probably have Thomas Zimmermann  take a
look at this.

Regards,
Christian.





Regards,
Christian.


+
+return drm_atomic_helper_dirtyfb(fb, file, flags, color, clips,
+ num_clips);
+}
+
   static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
   .destroy = drm_gem_fb_destroy,
   .create_handle = drm_gem_fb_create_handle,
   };
+static const struct drm_framebuffer_funcs amdgpu_fb_funcs_atomic = {
+.destroy = drm_gem_fb_destroy,
+.create_handle = drm_gem_fb_create_handle,
+.dirty = amdgpu_dirtyfb
+};
+
   uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
 uint64_t bo_flags)
   {
@@ -1139,7 +1159,11 @@ static int
amdgpu_display_gem_fb_verify_and_init(struct drm_device *dev,
   if (ret)
   goto err;
-ret = drm_framebuffer_init(dev, >base, _fb_funcs);
+if (drm_drv_uses_atomic_modeset(dev))
+ret = drm_framebuffer_init(dev, >base,
+   _fb_funcs_atomic);
+else
+ret = drm_framebuffer_init(dev, >base, _fb_funcs);
   if (ret)
   goto err;





--
Hamza

[PATCH] drm/amd/display: register edp_backlight_control() for DCN301

[Hamza Mahfooz]

As made mention of in commit 099303e9a9bd ("drm/amd/display: eDP
intermittent black screen during PnP"), we need to turn off the
display's backlight before powering off an eDP display. Not doing so
will result in undefined behaviour according to the eDP spec. So, set
DCN301's edp_backlight_control() function pointer to
dce110_edp_backlight_control().

Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2765
Fixes: 9c75891feef0 ("drm/amd/display: rework recent update PHY state commit")
Suggested-by: Swapnil Patel 
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c 
b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
index 257df8660b4c..61205cdbe2d5 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
@@ -75,6 +75,7 @@ static const struct hw_sequencer_funcs dcn301_funcs = {
.get_hw_state = dcn10_get_hw_state,
.clear_status_bits = dcn10_clear_status_bits,
.wait_for_mpcc_disconnect = dcn10_wait_for_mpcc_disconnect,
+   .edp_backlight_control = dce110_edp_backlight_control,
.edp_power_control = dce110_edp_power_control,
.edp_wait_for_hpd_ready = dce110_edp_wait_for_hpd_ready,
.set_cursor_position = dcn10_set_cursor_position,
-- 
2.41.0

Re: [PATCH] drm/amd/display: fix mode scaling (RMX_.*)

[Hamza Mahfooz]

On 8/18/23 09:28, Alex Deucher wrote:

On Fri, Aug 18, 2023 at 9:25 AM Hamza Mahfooz  wrote:


As made mention of in commit 4a2df0d1f28e ("drm/amd/display: Fixed
non-native modes not lighting up"), we shouldn't call
drm_mode_set_crtcinfo() once the crtc timings have been decided. Since,
it can cause settings to be unintentionally overwritten. So, since
dm_state is never NULL now, we can use old_stream to determine if we
should call drm_mode_set_crtcinfo() because we only need to set the crtc
timing parameters for entirely new streams.

Cc: Harry Wentland 
Cc: Rodrigo Siqueira 
Fixes: 712237a4a1b4 ("drm/amd/display: Always set crtcinfo from 
create_stream_for_sink")
Signed-off-by: Hamza Mahfooz 


Does this fix:
https://gitlab.freedesktop.org/drm/amd/-/issues/2783
If so, add a link tag for that.


The issue I'm addressing is specific to the colorspace patches (which
weren't ported to 6.4.y to my knowledge). So, that's probably unrelated.



Alex


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

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 3b27b7742854..e9aff5014e39 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6035,7 +6035,7 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,

 if (recalculate_timing)
 drm_mode_set_crtcinfo(_mode, 0);
-   else
+   else if (!old_stream)
 drm_mode_set_crtcinfo(, 0);

 /*
--
2.41.0


--
Hamza

[PATCH] drm/amd/display: fix mode scaling (RMX_.*)

[Hamza Mahfooz]

As made mention of in commit 4a2df0d1f28e ("drm/amd/display: Fixed
non-native modes not lighting up"), we shouldn't call
drm_mode_set_crtcinfo() once the crtc timings have been decided. Since,
it can cause settings to be unintentionally overwritten. So, since
dm_state is never NULL now, we can use old_stream to determine if we
should call drm_mode_set_crtcinfo() because we only need to set the crtc
timing parameters for entirely new streams.

Cc: Harry Wentland 
Cc: Rodrigo Siqueira 
Fixes: 712237a4a1b4 ("drm/amd/display: Always set crtcinfo from 
create_stream_for_sink")
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 3b27b7742854..e9aff5014e39 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6035,7 +6035,7 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,
 
if (recalculate_timing)
drm_mode_set_crtcinfo(_mode, 0);
-   else
+   else if (!old_stream)
drm_mode_set_crtcinfo(, 0);
 
/*
-- 
2.41.0

Re: [PATCH v2] drm/amdgpu: register a dirty framebuffer callback for fbcon

[Hamza Mahfooz]

On 8/16/23 01:55, Christian König wrote:



Am 15.08.23 um 19:26 schrieb Hamza Mahfooz:

fbcon requires that we implement _framebuffer_funcs.dirty.
Otherwise, the framebuffer might take a while to flush (which would
manifest as noticeable lag). However, we can't enable this callback for
non-fbcon cases since it might cause too many atomic commits to be made
at once. So, implement amdgpu_dirtyfb() and only enable it for fbcon
framebuffers on devices that support atomic KMS.

Cc: Aurabindo Pillai 
Cc: Mario Limonciello 
Cc: sta...@vger.kernel.org # 6.1+
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2519
Signed-off-by: Hamza Mahfooz 
---
v2: update variable names
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 26 -
  1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c

index d20dd3f852fc..d3b59f99cb7c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -38,6 +38,8 @@
  #include 
  #include 
  #include 
+#include 
+#include 
  #include 
  #include 
  #include 
@@ -532,11 +534,29 @@ bool amdgpu_display_ddc_probe(struct 
amdgpu_connector *amdgpu_connector,

  return true;
  }
+static int amdgpu_dirtyfb(struct drm_framebuffer *fb, struct drm_file 
*file,

+  unsigned int flags, unsigned int color,
+  struct drm_clip_rect *clips, unsigned int num_clips)
+{
+
+    if (strcmp(fb->comm, "[fbcon]"))
+    return -ENOSYS;


Once more to the v2 of this patch: Tests like those are a pretty big 
NO-GO for upstreaming.


On closer inspection it is actually sufficient to check if `file` is
NULL here (since it means that the request isn't from userspace). So, do
you think that would be palatable for upstream?



Regards,
Christian.


+
+    return drm_atomic_helper_dirtyfb(fb, file, flags, color, clips,
+ num_clips);
+}
+
  static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
  .destroy = drm_gem_fb_destroy,
  .create_handle = drm_gem_fb_create_handle,
  };
+static const struct drm_framebuffer_funcs amdgpu_fb_funcs_atomic = {
+    .destroy = drm_gem_fb_destroy,
+    .create_handle = drm_gem_fb_create_handle,
+    .dirty = amdgpu_dirtyfb
+};
+
  uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
    uint64_t bo_flags)
  {
@@ -1139,7 +1159,11 @@ static int 
amdgpu_display_gem_fb_verify_and_init(struct drm_device *dev,

  if (ret)
  goto err;
-    ret = drm_framebuffer_init(dev, >base, _fb_funcs);
+    if (drm_drv_uses_atomic_modeset(dev))
+    ret = drm_framebuffer_init(dev, >base,
+   _fb_funcs_atomic);
+    else
+    ret = drm_framebuffer_init(dev, >base, _fb_funcs);
  if (ret)
  goto err;



--
Hamza

[PATCH v2] drm/amdgpu: register a dirty framebuffer callback for fbcon

[Hamza Mahfooz]

fbcon requires that we implement _framebuffer_funcs.dirty.
Otherwise, the framebuffer might take a while to flush (which would
manifest as noticeable lag). However, we can't enable this callback for
non-fbcon cases since it might cause too many atomic commits to be made
at once. So, implement amdgpu_dirtyfb() and only enable it for fbcon
framebuffers on devices that support atomic KMS.

Cc: Aurabindo Pillai 
Cc: Mario Limonciello 
Cc: sta...@vger.kernel.org # 6.1+
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2519
Signed-off-by: Hamza Mahfooz 
---
v2: update variable names
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 26 -
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index d20dd3f852fc..d3b59f99cb7c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -38,6 +38,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -532,11 +534,29 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector 
*amdgpu_connector,
return true;
 }
 
+static int amdgpu_dirtyfb(struct drm_framebuffer *fb, struct drm_file *file,
+ unsigned int flags, unsigned int color,
+ struct drm_clip_rect *clips, unsigned int num_clips)
+{
+
+   if (strcmp(fb->comm, "[fbcon]"))
+   return -ENOSYS;
+
+   return drm_atomic_helper_dirtyfb(fb, file, flags, color, clips,
+num_clips);
+}
+
 static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
.destroy = drm_gem_fb_destroy,
.create_handle = drm_gem_fb_create_handle,
 };
 
+static const struct drm_framebuffer_funcs amdgpu_fb_funcs_atomic = {
+   .destroy = drm_gem_fb_destroy,
+   .create_handle = drm_gem_fb_create_handle,
+   .dirty = amdgpu_dirtyfb
+};
+
 uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
  uint64_t bo_flags)
 {
@@ -1139,7 +1159,11 @@ static int amdgpu_display_gem_fb_verify_and_init(struct 
drm_device *dev,
if (ret)
goto err;
 
-   ret = drm_framebuffer_init(dev, >base, _fb_funcs);
+   if (drm_drv_uses_atomic_modeset(dev))
+   ret = drm_framebuffer_init(dev, >base,
+  _fb_funcs_atomic);
+   else
+   ret = drm_framebuffer_init(dev, >base, _fb_funcs);
 
if (ret)
goto err;
-- 
2.41.0

[PATCH] drm/amdgpu: register a dirty framebuffer callback for fbcon

[Hamza Mahfooz]

fbcon requires that we implement _framebuffer_funcs.dirty.
Otherwise, the framebuffer might take awhile to flush (which would
manifest as noticeable lag). However, we can't enable this callback for
non-fbcon cases since it might cause too many atomic commits to be made
at once. So, implement amdgpu_dirtyfb() and only enable it for fbcon
framebuffers on devices that support atomic KMS.

Cc: Aurabindo Pillai 
Cc: Mario Limonciello 
Cc: sta...@vger.kernel.org # 6.1+
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2519
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 26 -
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index d20dd3f852fc..743db9aee68c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -38,6 +38,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -532,11 +534,29 @@ bool amdgpu_display_ddc_probe(struct amdgpu_connector 
*amdgpu_connector,
return true;
 }
 
+static int amdgpu_dirtyfb(struct drm_framebuffer *fb, struct drm_file *file,
+ unsigned int flags, unsigned int color,
+ struct drm_clip_rect *clips, unsigned int num_clips)
+{
+
+   if (strcmp(framebuffer->comm, "[fbcon]"))
+   return -ENOSYS;
+
+   return drm_atomic_helper_dirtyfb(framebuffer, file_priv, flags, color,
+clips, num_clips);
+}
+
 static const struct drm_framebuffer_funcs amdgpu_fb_funcs = {
.destroy = drm_gem_fb_destroy,
.create_handle = drm_gem_fb_create_handle,
 };
 
+static const struct drm_framebuffer_funcs amdgpu_fb_funcs_atomic = {
+   .destroy = drm_gem_fb_destroy,
+   .create_handle = drm_gem_fb_create_handle,
+   .dirty = amdgpu_dirtyfb
+};
+
 uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
  uint64_t bo_flags)
 {
@@ -1139,7 +1159,11 @@ static int amdgpu_display_gem_fb_verify_and_init(struct 
drm_device *dev,
if (ret)
goto err;
 
-   ret = drm_framebuffer_init(dev, >base, _fb_funcs);
+   if (drm_drv_uses_atomic_modeset(dev))
+   ret = drm_framebuffer_init(dev, >base,
+  _fb_funcs_atomic);
+   else
+   ret = drm_framebuffer_init(dev, >base, _fb_funcs);
 
if (ret)
goto err;
-- 
2.41.0

[PATCH v2] drm/amd/display: ensure async flips are only accepted for fast updates

[Hamza Mahfooz]

We should be checking to see if async flips are supported in
amdgpu_dm_atomic_check() (i.e. not dm_crtc_helper_atomic_check()). Also,
async flipping isn't supported if a plane's framebuffer changes memory
domains during an atomic commit. So, move the check from
dm_crtc_helper_atomic_check() to amdgpu_dm_atomic_check() and check if
the memory domain has changed in amdgpu_dm_atomic_check().

Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2733
Fixes: 3f86b60691e6 ("drm/amd/display: only accept async flips for fast 
updates")
Signed-off-by: Hamza Mahfooz 
---
v2: link issue and revert back to the old way of setting update_type.
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 ---
 .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 12 --
 2 files changed, 21 insertions(+), 15 deletions(-)

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 32fb551862b0..1d3afab5bc85 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8086,10 +8086,12 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
 * fast updates.
 */
if (crtc->state->async_flip &&
-   acrtc_state->update_type != UPDATE_TYPE_FAST)
+   (acrtc_state->update_type != UPDATE_TYPE_FAST ||
+get_mem_type(old_plane_state->fb) != get_mem_type(fb)))
drm_warn_once(state->dev,
  "[PLANE:%d:%s] async flip with non-fast 
update\n",
  plane->base.id, plane->name);
+
bundle->flip_addrs[planes_count].flip_immediate =
crtc->state->async_flip &&
acrtc_state->update_type == UPDATE_TYPE_FAST &&
@@ -10050,6 +10052,11 @@ static int amdgpu_dm_atomic_check(struct drm_device 
*dev,
 
/* Remove exiting planes if they are modified */
for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, 
new_plane_state, i) {
+   if (old_plane_state->fb && new_plane_state->fb &&
+   get_mem_type(old_plane_state->fb) !=
+   get_mem_type(new_plane_state->fb))
+   lock_and_validation_needed = true;
+
ret = dm_update_plane_state(dc, state, plane,
old_plane_state,
new_plane_state,
@@ -10297,9 +10304,20 @@ static int amdgpu_dm_atomic_check(struct drm_device 
*dev,
struct dm_crtc_state *dm_new_crtc_state =
to_dm_crtc_state(new_crtc_state);
 
+   /*
+* Only allow async flips for fast updates that don't change
+* the FB pitch, the DCC state, rotation, etc.
+*/
+   if (new_crtc_state->async_flip && lock_and_validation_needed) {
+   drm_dbg_atomic(crtc->dev,
+  "[CRTC:%d:%s] async flips are only 
supported for fast updates\n",
+  crtc->base.id, crtc->name);
+   ret = -EINVAL;
+   goto fail;
+   }
+
dm_new_crtc_state->update_type = lock_and_validation_needed ?
-UPDATE_TYPE_FULL :
-UPDATE_TYPE_FAST;
+   UPDATE_TYPE_FULL : UPDATE_TYPE_FAST;
}
 
/* Must be success */
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 30d4c6fd95f5..440fc0869a34 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -398,18 +398,6 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc 
*crtc,
return -EINVAL;
}
 
-   /*
-* Only allow async flips for fast updates that don't change the FB
-* pitch, the DCC state, rotation, etc.
-*/
-   if (crtc_state->async_flip &&
-   dm_crtc_state->update_type != UPDATE_TYPE_FAST) {
-   drm_dbg_atomic(crtc->dev,
-  "[CRTC:%d:%s] async flips are only supported for 
fast updates\n",
-  crtc->base.id, crtc->name);
-   return -EINVAL;
-   }
-
/* In some use cases, like reset, no stream is attached */
if (!dm_crtc_state->stream)
return 0;
-- 
2.41.0

[PATCH] drm/amd/display: ensure async flips are only accepted for fast updates

[Hamza Mahfooz]

We should be checking to see if async flips are supported in
amdgpu_dm_atomic_check() (i.e. not dm_crtc_helper_atomic_check()). Also,
async flipping isn't supported if a plane's framebuffer changes memory
domains during an atomic commit. So, move the check from
dm_crtc_helper_atomic_check() to amdgpu_dm_atomic_check() and check if
the memory domain has changed in amdgpu_dm_atomic_check().

Cc: sta...@vger.kernel.org
Fixes: 3f86b60691e6 ("drm/amd/display: only accept async flips for fast 
updates")
Tested-by: Marcus Seyfarth 
Signed-off-by: Hamza Mahfooz 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 25 ---
 .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 12 -
 2 files changed, 21 insertions(+), 16 deletions(-)

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 32fb551862b0..e561d99b3f40 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8086,7 +8086,8 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
 * fast updates.
 */
if (crtc->state->async_flip &&
-   acrtc_state->update_type != UPDATE_TYPE_FAST)
+   (acrtc_state->update_type != UPDATE_TYPE_FAST ||
+get_mem_type(old_plane_state->fb) != get_mem_type(fb)))
drm_warn_once(state->dev,
  "[PLANE:%d:%s] async flip with non-fast 
update\n",
  plane->base.id, plane->name);
@@ -10050,12 +10051,18 @@ static int amdgpu_dm_atomic_check(struct drm_device 
*dev,
 
/* Remove exiting planes if they are modified */
for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, 
new_plane_state, i) {
+   if (old_plane_state->fb && new_plane_state->fb &&
+   get_mem_type(old_plane_state->fb) !=
+   get_mem_type(new_plane_state->fb))
+   lock_and_validation_needed = true;
+
ret = dm_update_plane_state(dc, state, plane,
old_plane_state,
new_plane_state,
false,
_and_validation_needed,
_top_most_overlay);
+
if (ret) {
DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
goto fail;
@@ -10069,6 +10076,7 @@ static int amdgpu_dm_atomic_check(struct drm_device 
*dev,
   new_crtc_state,
   false,
   _and_validation_needed);
+
if (ret) {
DRM_DEBUG_DRIVER("DISABLE: dm_update_crtc_state() 
failed\n");
goto fail;
@@ -10297,9 +10305,18 @@ static int amdgpu_dm_atomic_check(struct drm_device 
*dev,
struct dm_crtc_state *dm_new_crtc_state =
to_dm_crtc_state(new_crtc_state);
 
-   dm_new_crtc_state->update_type = lock_and_validation_needed ?
-UPDATE_TYPE_FULL :
-UPDATE_TYPE_FAST;
+   /*
+* Only allow async flips for fast updates that don't change
+* the FB pitch, the DCC state, rotation, etc.
+*/
+   if (new_crtc_state->async_flip && lock_and_validation_needed) {
+   drm_dbg_atomic(crtc->dev,
+  "[CRTC:%d:%s] async flips are only 
supported for fast updates\n",
+  crtc->base.id, crtc->name);
+   ret = -EINVAL;
+   goto fail;
+   } else
+   dm_new_crtc_state->update_type = UPDATE_TYPE_FAST;
}
 
/* Must be success */
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 30d4c6fd95f5..440fc0869a34 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -398,18 +398,6 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc 
*crtc,
return -EINVAL;
}
 
-   /*
-* Only allow async flips for fast updates that don't change the FB
-* pitch, the DCC state, rotation, etc.
-*/
-   if (crtc_state->async_flip &&
-   dm_crtc_state->update_type != UPDATE_

Re: [PATCH] amd/display: only accept async flips for fast updates

[Hamza Mahfooz]

On 6/21/23 16:24, André Almeida wrote:

From: Simon Ser 

Up until now, amdgpu was silently degrading to vsync when
user-space requested an async flip but the hardware didn't support
it.

The hardware doesn't support immediate flips when the update changes
the FB pitch, the DCC state, the rotation, enables or disables CRTCs
or planes, etc. This is reflected in the dm_crtc_state.update_type
field: UPDATE_TYPE_FAST means that immediate flip is supported.

Silently degrading async flips to vsync is not the expected behavior
from a uAPI point-of-view. Xorg expects async flips to fail if
unsupported, to be able to fall back to a blit. i915 already behaves
this way.

This patch aligns amdgpu with uAPI expectations and returns a failure
when an async flip is not possible.

Signed-off-by: Simon Ser 
Reviewed-by: André Almeida 
Reviewed-by: Alex Deucher 
Signed-off-by: André Almeida 


Applied, thanks!


---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  8 
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c   | 12 
  2 files changed, 20 insertions(+)

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 514f6785a020..1d9b84e5835f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8136,7 +8136,15 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
 * Only allow immediate flips for fast updates that don't
 * change memory domain, FB pitch, DCC state, rotation or
 * mirroring.
+*
+* dm_crtc_helper_atomic_check() only accepts async flips with
+* fast updates.
 */
+   if (crtc->state->async_flip &&
+   acrtc_state->update_type != UPDATE_TYPE_FAST)
+   drm_warn_once(state->dev,
+ "[PLANE:%d:%s] async flip with non-fast 
update\n",
+ plane->base.id, plane->name);
bundle->flip_addrs[planes_count].flip_immediate =
crtc->state->async_flip &&
acrtc_state->update_type == UPDATE_TYPE_FAST &&
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 440fc0869a34..30d4c6fd95f5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -398,6 +398,18 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc 
*crtc,
return -EINVAL;
}
  
+	/*

+* Only allow async flips for fast updates that don't change the FB
+* pitch, the DCC state, rotation, etc.
+*/
+   if (crtc_state->async_flip &&
+   dm_crtc_state->update_type != UPDATE_TYPE_FAST) {
+   drm_dbg_atomic(crtc->dev,
+  "[CRTC:%d:%s] async flips are only supported for fast 
updates\n",
+  crtc->base.id, crtc->name);
+   return -EINVAL;
+   }
+
/* In some use cases, like reset, no stream is attached */
if (!dm_crtc_state->stream)
return 0;

--
Hamza

[PATCH] drm/amd/amdgpu: enable W=1 for amdgpu

[Hamza Mahfooz]

We have a clean build with W=1 as of
commit 12a15dd589ac ("drm/amd/display/amdgpu_dm/amdgpu_dm_helpers: Move
SYNAPTICS_DEVICE_ID into CONFIG_DRM_AMD_DC_DCN ifdef"). So, let's enable
these checks unconditionally for the entire module to catch these errors
during development.

Cc: Alex Deucher 
Cc: Nathan Chancellor 
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/amdgpu/Makefile | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 86b833085f19..8d16f280b695 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -40,7 +40,18 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
-I$(FULL_AMD_PATH)/amdkfd
 
 subdir-ccflags-y := -Wextra
-subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
+subdir-ccflags-y += -Wunused
+subdir-ccflags-y += -Wmissing-prototypes
+subdir-ccflags-y += -Wmissing-declarations
+subdir-ccflags-y += -Wmissing-include-dirs
+subdir-ccflags-y += -Wold-style-definition
+subdir-ccflags-y += -Wmissing-format-attribute
+# Need this to avoid recursive variable evaluation issues
+cond-flags := $(call cc-option, -Wunused-but-set-variable) \
+   $(call cc-option, -Wunused-const-variable) \
+   $(call cc-option, -Wstringop-truncation) \
+   $(call cc-option, -Wpacked-not-aligned)
+subdir-ccflags-y += $(cond-flags)
 subdir-ccflags-y += -Wno-unused-parameter
 subdir-ccflags-y += -Wno-type-limits
 subdir-ccflags-y += -Wno-sign-compare
-- 
2.40.1

Re: [RESEND 11/15] drm/amd/display/amdgpu_dm/amdgpu_dm_helpers: Move SYNAPTICS_DEVICE_ID into CONFIG_DRM_AMD_DC_DCN ifdef

[Hamza Mahfooz]

On 6/9/23 04:17, Lee Jones wrote:

Fixes the following W=1 kernel build warning(s):

  drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_helpers.c:48:22: 
warning: ‘SYNAPTICS_DEVICE_ID’ defined but not used [-Wunused-const-variable=]

Cc: Harry Wentland 
Cc: Leo Li 
Cc: Rodrigo Siqueira 
Cc: Alex Deucher 
Cc: "Christian König" 
Cc: "Pan, Xinhui" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Lee Jones 


Applied, thanks!


---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

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 09e056a647087..cd20cfc049969 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
@@ -44,9 +44,6 @@
  #include "dm_helpers.h"
  #include "ddc_service_types.h"
  
-/* MST Dock */

-static const uint8_t SYNAPTICS_DEVICE_ID[] = "SYNA";
-
  /* dm_helpers_parse_edid_caps
   *
   * Parse edid caps
@@ -702,6 +699,9 @@ static void apply_synaptics_fifo_reset_wa(struct drm_dp_aux 
*aux)
DC_LOG_DC("Done apply_synaptics_fifo_reset_wa\n");
  }
  
+/* MST Dock */

+static const uint8_t SYNAPTICS_DEVICE_ID[] = "SYNA";
+
  static uint8_t write_dsc_enable_synaptics_non_virtual_dpcd_mst(
struct drm_dp_aux *aux,
const struct dc_stream_state *stream,

--
Hamza

Re: [PATCH] drm/amdgpu: Wrap -Wunused-but-set-variable in cc-option

[Hamza Mahfooz]

On 6/8/23 13:01, Nathan Chancellor wrote:

-Wunused-but-set-variable was only supported in clang starting with
13.0.0, so earlier versions will emit a warning, which is turned into a
hard error for the kernel to mirror GCC:

   error: unknown warning option '-Wunused-but-set-variable'; did you mean 
'-Wunused-const-variable'? [-Werror,-Wunknown-warning-option]

The minimum supported version of clang for building the kernel is
11.0.0, so match the rest of the kernel and wrap
-Wunused-but-set-variable in a cc-option call, so that it is only used
when supported by the compiler.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1869
Fixes: a0fd5a5f676c ("drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR")
Signed-off-by: Nathan Chancellor 


Applied, thanks!


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

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 7ee68b1bbfed..86b833085f19 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -40,7 +40,7 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
-I$(FULL_AMD_PATH)/amdkfd
  
  subdir-ccflags-y := -Wextra

-subdir-ccflags-y += -Wunused-but-set-variable
+subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
  subdir-ccflags-y += -Wno-unused-parameter
  subdir-ccflags-y += -Wno-type-limits
  subdir-ccflags-y += -Wno-sign-compare

---
base-commit: 6bd4b01e8938779b0d959bdf33949a9aa258a363
change-id: 
20230608-amdgpu-wrap-wunused-but-set-variable-in-cc-option-0be9528ac5c8

Best regards,

--
Hamza

[PATCH 2/2] drm/amd/display: mark dml314's UseMinimumDCFCLK() as noinline_for_stack

[Hamza Mahfooz]

clang reports:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn314/display_mode_vba_314.c:3892:6:
 error: stack frame size (2632) exceeds limit (2048) in 
'dml314_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
 3892 | void dml314_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_lib)
  |  ^
1 error generated.

So, since UseMinimumDCFCLK() consumes a lot of stack space, mark it as
noinline_for_stack to prevent it from blowing up
dml314_ModeSupportAndSystemConfigurationFull()'s stack size.

Signed-off-by: Hamza Mahfooz 
---
 .../gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c| 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c
index 27b83162ae45..1532a7e0ed6c 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn314/display_mode_vba_314.c
@@ -7061,7 +7061,7 @@ static double CalculateUrgentLatency(
return ret;
 }
 
-static void UseMinimumDCFCLK(
+static noinline_for_stack void UseMinimumDCFCLK(
struct display_mode_lib *mode_lib,
int MaxPrefetchMode,
int ReorderingBytes)
-- 
2.40.1

[PATCH 1/2] drm/amd/display: mark dml31's UseMinimumDCFCLK() as noinline_for_stack

[Hamza Mahfooz]

clang reports:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:3797:6:
 error: stack frame size (2632) exceeds limit (2048) in 
'dml31_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
 3797 | void dml31_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_lib)
  |  ^
1 error generated.

So, since UseMinimumDCFCLK() consumes a lot of stack space, mark it as
noinline_for_stack to prevent it from blowing up
dml31_ModeSupportAndSystemConfigurationFull()'s stack size.

Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c
index 01603abd75bb..43016c462251 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c
@@ -7032,7 +7032,7 @@ static double CalculateUrgentLatency(
return ret;
 }
 
-static void UseMinimumDCFCLK(
+static noinline_for_stack void UseMinimumDCFCLK(
struct display_mode_lib *mode_lib,
int MaxPrefetchMode,
int ReorderingBytes)
-- 
2.40.1

Re: PROBLEM: AMD Ryzen 9 7950X iGPU - Blinking Issue

[Hamza Mahfooz]

On 6/3/23 10:52, Felix Richter wrote:

Hi Guys,

sorry for the silence from my side. I had a lot of things to take care 
of after returning from vacation. Also I had to wait on the zfs modules 
to be updated to support kernel 6.3 for further testing.


The bad news is that I am still experiencing issues. I have been able to 
get a reproducible trigger for the buggy behavior. The moment I take a 
screenshot or any other program like `wdisplays` accesses the screen 
buffer the screen starts flickering. The only way to reset it is to 
reboot the machine or log out of the desktop.


With this I did a bisection to figure out which commit is responsible 
for this. I attached the logs to the mail. The short version is that I 
identified commit 81d0bcf9900932633d270d5bc4a54ff599c6ebdb as the 
culprit. Seems that there are side effects of having more flexible 
buffer placement for the case of the internal GPU. To verify that this 
actually is the cause of the issue I built the current archlinux kernel 
with an extra patch to revert the commit: 
https://github.com/ju6ge/linux/tree/v6.3.5-ju6ge. The result is that be 
bug is fixed!


Now if this is the desired long term fix I do not know …


Can you provide a dmidecode of your RAM (i.e. # dmidecode --type=memory)?

The current trend seems to suggest that if you have 64 or more gigs of
RAM, you will probably still experience issues with S/G mode enabled
even with my fix applied.



Kind regards,
Felix Richter

On 02.05.23 16:12, Linux regression tracking (Thorsten Leemhuis) wrote:

On 02.05.23 15:48, Felix Richter wrote:

On 5/2/23 15:34, Linux regression tracking (Thorsten Leemhuis) wrote:

On 02.05.23 15:13, Alex Deucher wrote:

On Tue, May 2, 2023 at 7:45 AM Linux regression tracking (Thorsten
Leemhuis)  wrote:


On 30.04.23 13:44, Felix Richter wrote:

Hi,

I am running into an issue with the integrated GPU of the Ryzen 9
7950X. It seems to be a regression from kernel version 6.1 to 6.2.
The bug materializes in from of my monitor blinking, meaning it
turns full white shortly. This happens very often so that the
system becomes unpleasant to use.

I am running the Archlinux Kernel:
The Issue happens on the bleeding edge kernel: 6.2.13
Switching back to the LTS kernel resolves the issue: 6.1.26

I have two monitors attached to the system. One 42 inch 4k Display
and a 24 inch 1080p Display and am running sway as my desktop.

Let me know if there is more information I could provide to help
narrow down the issue.

Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel 
regression

tracking bot:

#regzbot ^introduced v6.1..v6.2
#regzbot title drm: amdgpu: system becomes unpleasant to use after
monitor starts blinking and turns full white
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify
when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page 
listed in

the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags
pointing
to the report (the parent of this mail). See page linked in footer 
for

details.

This sounds exactly like the issue that was fixed in this patch which
is already on it's way to Linus:
https://gitlab.freedesktop.org/agd5f/linux/-/commit/08da182175db4c7f80850354849d95f2670e8cd9

FWIW, you in the flood of emails likely missed that this is the same
thread where you yesterday replied "If the module parameter didn't help
then perhaps you are seeing some other issue.  Can you bisect?". That's
why I decided to add this to the tracking. Or am I missing something
obvious here?

/me looks around again and can't see anything, but that doesn't have to
mean anything...

Felix, btw, this guide might help you with the bisection, even if it's
just for kernel compilation:

https://docs.kernel.org/next/admin-guide/quickly-build-trimmed-linux.html

And to indirectly reply to your mail from yesterday[1]. You might want
to ignore the arch linux kernel git repo and just do a bisection 
between

6.1 and the latest 6.2.y kernel using upstream repos; and if I were you
I'd also try 6.3 or even mainline before that, in case the issue was
fixed already.

[1]
https://lore.kernel.org/all/04749ee4-0728-92fe-bcb0-a7320279e...@felixrichter.tech/


Thanks for the pointers, I'll do a bisection on my desktop from 6.1 to
the newest commit.

FWIW, I wonder what you actually mean with "newest commit" here: a
bisection between 6.1 and mainline HEAD might be a waste of time, *if*
this is something that only happens in 6.2.y (say due to a broken or
incomplete backport)


That was the part I was mostly unsure about … where
to start from.

I was planning to use PKGBUILD scripts from arch to achieve the same
configuration as I would when 

Re: [PATCH] drm/amd/display: fix compilation error due to shifting negative value

[Hamza Mahfooz]

On 6/2/23 06:12, GONG, Ruiqi wrote:

Currently compiling linux-next with allmodconfig triggers the following
error:

./drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h: In function 
‘dc_fixpt_truncate’:
./drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:528:22: error: 
left shift of negative value [-Werror=shift-negative-value]
   528 |  arg.value &= (~0LL) << (FIXED31_32_BITS_PER_FRACTIONAL_PART - 
frac_bits);
   |  ^~

Use `unsigned long long` instead.

Signed-off-by: GONG, Ruiqi 


Applied, thanks!


---
  drivers/gpu/drm/amd/display/include/fixed31_32.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/include/fixed31_32.h 
b/drivers/gpu/drm/amd/display/include/fixed31_32.h
index ece97ae0e826..d4cf7ead1d87 100644
--- a/drivers/gpu/drm/amd/display/include/fixed31_32.h
+++ b/drivers/gpu/drm/amd/display/include/fixed31_32.h
@@ -525,7 +525,7 @@ static inline struct fixed31_32 dc_fixpt_truncate(struct 
fixed31_32 arg, unsigne
  
  	if (negative)

arg.value = -arg.value;
-   arg.value &= (~0LL) << (FIXED31_32_BITS_PER_FRACTIONAL_PART - 
frac_bits);
+   arg.value &= (~0ULL) << (FIXED31_32_BITS_PER_FRACTIONAL_PART - 
frac_bits);
if (negative)
arg.value = -arg.value;
return arg;

--
Hamza

Re: [PATCH] drm/amd/display: clean up some inconsistent indenting

[Hamza Mahfooz]

On 6/2/23 02:17, Jiapeng Chong wrote:

No functional modification involved.

drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_dpms.c:2377 
link_set_dpms_on() warn: inconsistent indenting.

Reported-by: Abaci Robot 
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=5376
Signed-off-by: Jiapeng Chong 


Applied, thanks!


---
  drivers/gpu/drm/amd/display/dc/link/link_dpms.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c 
b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
index 2963267fe74a..f7f1a1586f3b 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
@@ -2374,8 +2374,8 @@ void link_set_dpms_on(
}
}
  
-		if (dc_is_virtual_signal(pipe_ctx->stream->signal))

-   return;
+   if (dc_is_virtual_signal(pipe_ctx->stream->signal))
+   return;
  
  	link_enc = link_enc_cfg_get_link_enc(link);

ASSERT(link_enc);

--
Hamza

Re: [PATCH] drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR

[Hamza Mahfooz]

On 5/30/23 11:50, Ho, Kenny wrote:

[Public]

On 5/30/23 11:24, Hamza Mahfooz wrote:

I am able to get clean builds with this enabled on GCC 11-13 and Clang
15, at least as of commit e786aef0869c ("drm/amd/display: remove unused
definition") on amd-staging-drm-next.


Did you try intentionally introducing a warning to see if the build indeed fail?


Yes, I tried a couple of different ones.

--
Hamza

Re: [PATCH] drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR

[Hamza Mahfooz]

On 5/25/23 12:38, Hamza Mahfooz wrote:

We want to do -Werror builds on our CI. However, non-amdgpu breakages
have prevented us from doing so thus far. Also, there are a number of
additional checks that we should enable, that the community cares about
and are hidden behind -Wextra. So, define DRM_AMDGPU_WERROR to only
enable -Werror for the amdgpu kernel module and enable -Wextra while
disabling all of the checks that are too noisy.

Cc: Alex Deucher 
Cc: Kenny Ho 
Suggested-by: Jani Nikula 
Signed-off-by: Hamza Mahfooz 


I am able to get clean builds with this enabled on GCC 11-13 and Clang
15, at least as of commit e786aef0869c ("drm/amd/display: remove unused
definition") on amd-staging-drm-next.


---
  drivers/gpu/drm/amd/amdgpu/Kconfig  | 10 ++
  drivers/gpu/drm/amd/amdgpu/Makefile |  9 +
  2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
b/drivers/gpu/drm/amd/amdgpu/Kconfig
index 07135ffa6d24..334511f331e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -66,6 +66,16 @@ config DRM_AMDGPU_USERPTR
  This option selects CONFIG_HMM and CONFIG_HMM_MIRROR if it
  isn't already selected to enabled full userptr support.
  
+config DRM_AMDGPU_WERROR

+   bool "Force the compiler to throw an error instead of a warning when 
compiling"
+   depends on DRM_AMDGPU
+   depends on EXPERT
+   depends on !COMPILE_TEST
+   default n
+   help
+ Add -Werror to the build flags for amdgpu.ko.
+ Only enable this if you are warning code for amdgpu.ko.
+
  source "drivers/gpu/drm/amd/acp/Kconfig"
  source "drivers/gpu/drm/amd/display/Kconfig"
  source "drivers/gpu/drm/amd/amdkfd/Kconfig"
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 74a9aa6fe18c..7ee68b1bbfed 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -39,6 +39,15 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
-I$(FULL_AMD_DISPLAY_PATH)/amdgpu_dm \
-I$(FULL_AMD_PATH)/amdkfd
  
+subdir-ccflags-y := -Wextra

+subdir-ccflags-y += -Wunused-but-set-variable
+subdir-ccflags-y += -Wno-unused-parameter
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-sign-compare
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-override-init
+subdir-ccflags-$(CONFIG_DRM_AMDGPU_WERROR) += -Werror
+
  amdgpu-y := amdgpu_drv.o
  
  # add KMS driver

--
Hamza

[PATCH] drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR

[Hamza Mahfooz]

We want to do -Werror builds on our CI. However, non-amdgpu breakages
have prevented us from doing so thus far. Also, there are a number of
additional checks that we should enable, that the community cares about
and are hidden behind -Wextra. So, define DRM_AMDGPU_WERROR to only
enable -Werror for the amdgpu kernel module and enable -Wextra while
disabling all of the checks that are too noisy.

Cc: Alex Deucher 
Cc: Kenny Ho 
Suggested-by: Jani Nikula 
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/amdgpu/Kconfig  | 10 ++
 drivers/gpu/drm/amd/amdgpu/Makefile |  9 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
b/drivers/gpu/drm/amd/amdgpu/Kconfig
index 07135ffa6d24..334511f331e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -66,6 +66,16 @@ config DRM_AMDGPU_USERPTR
  This option selects CONFIG_HMM and CONFIG_HMM_MIRROR if it
  isn't already selected to enabled full userptr support.
 
+config DRM_AMDGPU_WERROR
+   bool "Force the compiler to throw an error instead of a warning when 
compiling"
+   depends on DRM_AMDGPU
+   depends on EXPERT
+   depends on !COMPILE_TEST
+   default n
+   help
+ Add -Werror to the build flags for amdgpu.ko.
+ Only enable this if you are warning code for amdgpu.ko.
+
 source "drivers/gpu/drm/amd/acp/Kconfig"
 source "drivers/gpu/drm/amd/display/Kconfig"
 source "drivers/gpu/drm/amd/amdkfd/Kconfig"
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 74a9aa6fe18c..7ee68b1bbfed 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -39,6 +39,15 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
-I$(FULL_AMD_DISPLAY_PATH)/amdgpu_dm \
-I$(FULL_AMD_PATH)/amdkfd
 
+subdir-ccflags-y := -Wextra
+subdir-ccflags-y += -Wunused-but-set-variable
+subdir-ccflags-y += -Wno-unused-parameter
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-sign-compare
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-override-init
+subdir-ccflags-$(CONFIG_DRM_AMDGPU_WERROR) += -Werror
+
 amdgpu-y := amdgpu_drv.o
 
 # add KMS driver
-- 
2.40.1

Re: [PATCH v2] drm/amd/display: enable more strict compile checks

[Hamza Mahfooz]

+ Kees

On 5/24/23 15:50, Alex Deucher wrote:

On Wed, May 24, 2023 at 3:46 PM Felix Kuehling  wrote:


Sure, I think we tried enabling warnings as errors before and had to
revert it because of weird compiler quirks or the variety of compiler
versions that need to be supported.

Alex, are you planning to upstream this, or is this only to enforce more
internal discipline about not ignoring warnings?


I'd like to upstream it.  Upstream already has CONFIG_WERROR as a
config option, but it's been problematic to enable in CI because of
various breakages outside of the driver and in different compilers.
That said, I don't know how much trouble enabling it will cause with
various compilers in the wild.

Alex



Regards,
Felix


On 2023-05-24 15:41, Russell, Kent wrote:


[AMD Official Use Only - General]


(Adding Felix in CC)

I’m a fan of adding it to KFD as well. Felix, can you foresee any
issues here?

Kent

*From:* amd-gfx  *On Behalf Of
*Ho, Kenny
*Sent:* Wednesday, May 24, 2023 3:23 PM
*To:* Alex Deucher ; Mahfooz, Hamza

*Cc:* Li, Sun peng (Leo) ; Wentland, Harry
; Pan, Xinhui ; Siqueira,
Rodrigo ; linux-ker...@vger.kernel.org;
dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; Daniel
Vetter ; Deucher, Alexander
; David Airlie ; Koenig,
Christian 
*Subject:* Re: [PATCH v2] drm/amd/display: enable more strict compile
checks

[AMD Official Use Only - General]

[AMD Official Use Only - General]

(+ Felix)

Should we do the same for other modules under amd (amdkfd)?  I was
going to enable full kernel werror in the kconfig used by my CI but
this is probably better.

Kenny



*From:*Alex Deucher 
*Sent:* Wednesday, May 24, 2023 3:22 PM
*To:* Mahfooz, Hamza 
*Cc:* amd-...@lists.freedesktop.org ;
Li, Sun peng (Leo) ; Ho, Kenny ;
Pan, Xinhui ; Siqueira, Rodrigo
; linux-ker...@vger.kernel.org
; dri-devel@lists.freedesktop.org
; Daniel Vetter ;
Deucher, Alexander ; David Airlie
; Wentland, Harry ; Koenig,
Christian 
*Subject:* Re: [PATCH v2] drm/amd/display: enable more strict compile
checks

On Wed, May 24, 2023 at 3:20 PM Hamza Mahfooz 
wrote:


Currently, there are quite a number of issues that are quite easy for
the CI to catch, that slip through the cracks. Among them, there are
unused variable and indentation issues. Also, we should consider all
warnings to be compile errors, since the community will eventually end
up complaining about them. So, enable -Werror, -Wunused and
-Wmisleading-indentation for all kernel builds.

Cc: Alex Deucher 
Cc: Harry Wentland 
Cc: Kenny Ho 
Signed-off-by: Hamza Mahfooz 
---
v2: fix grammatical error
---
  drivers/gpu/drm/amd/display/Makefile | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/Makefile

b/drivers/gpu/drm/amd/display/Makefile

index 0d610cb376bb..3c44162ebe21 100644
--- a/drivers/gpu/drm/amd/display/Makefile
+++ b/drivers/gpu/drm/amd/display/Makefile
@@ -26,6 +26,8 @@

  AMDDALPATH = $(RELATIVE_AMD_DISPLAY_PATH)

+subdir-ccflags-y += -Werror -Wunused -Wmisleading-indentation
+


Care to enable this for the rest of amdgpu as well?  Or send out an
additional patch to do that?  Either way:
Reviewed-by: Alex Deucher 

Alex


  subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/
  subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/hw
  subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/clk_mgr
--
2.40.1




--
Hamza

Re: [PATCH v2] drm/amd/display: enable more strict compile checks

[Hamza Mahfooz]

On 5/24/23 15:54, Harry Wentland wrote:



On 5/24/23 15:27, Hamza Mahfooz wrote:

On 5/24/23 15:22, Alex Deucher wrote:

On Wed, May 24, 2023 at 3:20 PM Hamza Mahfooz  wrote:


Currently, there are quite a number of issues that are quite easy for
the CI to catch, that slip through the cracks. Among them, there are
unused variable and indentation issues. Also, we should consider all
warnings to be compile errors, since the community will eventually end
up complaining about them. So, enable -Werror, -Wunused and
-Wmisleading-indentation for all kernel builds.

Cc: Alex Deucher 
Cc: Harry Wentland 
Cc: Kenny Ho 
Signed-off-by: Hamza Mahfooz 
---
v2: fix grammatical error
---
   drivers/gpu/drm/amd/display/Makefile | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/Makefile 
b/drivers/gpu/drm/amd/display/Makefile
index 0d610cb376bb..3c44162ebe21 100644
--- a/drivers/gpu/drm/amd/display/Makefile
+++ b/drivers/gpu/drm/amd/display/Makefile
@@ -26,6 +26,8 @@

   AMDDALPATH = $(RELATIVE_AMD_DISPLAY_PATH)

+subdir-ccflags-y += -Werror -Wunused -Wmisleading-indentation
+


Care to enable this for the rest of amdgpu as well?  Or send out an
additional patch to do that?  Either way:
Reviewed-by: Alex Deucher 


As far as I can tell, if `CONFIG_DRM_AMD_DC` is set it will run these
checks on at least the base driver code.



It's probable best to put that into amdgpu/Makefile in that case.


I tried the following, but it doesn't seem to work:

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile

index 74a9aa6fe18c..d97bde0796dc 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -39,6 +39,8 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
-I$(FULL_AMD_DISPLAY_PATH)/amdgpu_dm \
-I$(FULL_AMD_PATH)/amdkfd

+ccflags-y += -Werror -Wunused -Wmisleading-indentation
+
 amdgpu-y := amdgpu_drv.o

 # add KMS driver



Harry



Alex


   subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/
   subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/hw
   subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/clk_mgr
--
2.40.1




--
Hamza

Re: [PATCH v2] drm/amd/display: enable more strict compile checks

[Hamza Mahfooz]

On 5/24/23 15:22, Alex Deucher wrote:

On Wed, May 24, 2023 at 3:20 PM Hamza Mahfooz  wrote:


Currently, there are quite a number of issues that are quite easy for
the CI to catch, that slip through the cracks. Among them, there are
unused variable and indentation issues. Also, we should consider all
warnings to be compile errors, since the community will eventually end
up complaining about them. So, enable -Werror, -Wunused and
-Wmisleading-indentation for all kernel builds.

Cc: Alex Deucher 
Cc: Harry Wentland 
Cc: Kenny Ho 
Signed-off-by: Hamza Mahfooz 
---
v2: fix grammatical error
---
  drivers/gpu/drm/amd/display/Makefile | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/Makefile 
b/drivers/gpu/drm/amd/display/Makefile
index 0d610cb376bb..3c44162ebe21 100644
--- a/drivers/gpu/drm/amd/display/Makefile
+++ b/drivers/gpu/drm/amd/display/Makefile
@@ -26,6 +26,8 @@

  AMDDALPATH = $(RELATIVE_AMD_DISPLAY_PATH)

+subdir-ccflags-y += -Werror -Wunused -Wmisleading-indentation
+


Care to enable this for the rest of amdgpu as well?  Or send out an
additional patch to do that?  Either way:
Reviewed-by: Alex Deucher 


As far as I can tell, if `CONFIG_DRM_AMD_DC` is set it will run these
checks on at least the base driver code.



Alex


  subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/
  subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/hw
  subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/clk_mgr
--
2.40.1


--
Hamza

[PATCH v2] drm/amd/display: enable more strict compile checks

[Hamza Mahfooz]

Currently, there are quite a number of issues that are quite easy for
the CI to catch, that slip through the cracks. Among them, there are
unused variable and indentation issues. Also, we should consider all
warnings to be compile errors, since the community will eventually end
up complaining about them. So, enable -Werror, -Wunused and
-Wmisleading-indentation for all kernel builds.

Cc: Alex Deucher 
Cc: Harry Wentland 
Cc: Kenny Ho 
Signed-off-by: Hamza Mahfooz 
---
v2: fix grammatical error
---
 drivers/gpu/drm/amd/display/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/Makefile 
b/drivers/gpu/drm/amd/display/Makefile
index 0d610cb376bb..3c44162ebe21 100644
--- a/drivers/gpu/drm/amd/display/Makefile
+++ b/drivers/gpu/drm/amd/display/Makefile
@@ -26,6 +26,8 @@
 
 AMDDALPATH = $(RELATIVE_AMD_DISPLAY_PATH)
 
+subdir-ccflags-y += -Werror -Wunused -Wmisleading-indentation
+
 subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/
 subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/hw
 subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/clk_mgr
-- 
2.40.1

[PATCH] drm/amd/display: enable more strict compile checks

[Hamza Mahfooz]

Currently, there are quite a number of issues that are quite easy for
the CI to catch, that slip through the cracks. Among them, there unused
variable and indentation issues. Also, we should consider all warnings
to be compile errors, since the community will eventually end up
complaining about them. So, enable -Werror, -Wunused and
-Wmisleading-indentation for all kernel builds.

Cc: Alex Deucher 
Cc: Harry Wentland 
Cc: Kenny Ho 
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/Makefile 
b/drivers/gpu/drm/amd/display/Makefile
index 0d610cb376bb..3c44162ebe21 100644
--- a/drivers/gpu/drm/amd/display/Makefile
+++ b/drivers/gpu/drm/amd/display/Makefile
@@ -26,6 +26,8 @@
 
 AMDDALPATH = $(RELATIVE_AMD_DISPLAY_PATH)
 
+subdir-ccflags-y += -Werror -Wunused -Wmisleading-indentation
+
 subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/
 subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/inc/hw
 subdir-ccflags-y += -I$(FULL_AMD_DISPLAY_PATH)/dc/clk_mgr
-- 
2.40.1

Re: [PATCH 1/2] drm/amd/display: clean up some inconsistent indenting

[Hamza Mahfooz]

On 5/24/23 04:57, Jiapeng Chong wrote:

No functional modification involved.

drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn314/dcn314_fpu.c:269 
dcn314_update_bw_bounding_box_fpu() warn: inconsistent indenting.

Reported-by: Abaci Robot 
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=5305
Signed-off-by: Jiapeng Chong 


I have applied the series, thanks!


---
  drivers/gpu/drm/amd/display/dc/dml/dcn314/dcn314_fpu.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn314/dcn314_fpu.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn314/dcn314_fpu.c
index 318b9c2bc9be..c9afddd11589 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn314/dcn314_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn314/dcn314_fpu.c
@@ -265,8 +265,7 @@ void dcn314_update_bw_bounding_box_fpu(struct dc *dc, 
struct clk_bw_params *bw_p
}
  
  	dcn20_patch_bounding_box(dc, _14_soc);

-
-   dml_init_instance(>dml, _14_soc, _14_ip, 
DML_PROJECT_DCN314);
+   dml_init_instance(>dml, _14_soc, _14_ip, 
DML_PROJECT_DCN314);
  }
  
  static bool is_dual_plane(enum surface_pixel_format format)

--
Hamza

Re: [PATCH -next 01/13] drm/amd/display: remove unused definition

[Hamza Mahfooz]

On 5/23/23 23:59, Yang Li wrote:

Eliminate the following warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_resource.c:889:43: 
warning: unused variable 'res_create_maximus_funcs'

Reported-by: Abaci Robot 
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=5296
Fixes: 00df97e1df57 ("drm/amd/display: Clean FPGA code in dc")
Signed-off-by: Yang Li 


I have applied the series, thanks!


---
  drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 7 ---
  1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index a0625209c86d..26ddf73fd5b1 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -886,13 +886,6 @@ static const struct resource_create_funcs res_create_funcs 
= {
.create_hwseq = dcn10_hwseq_create,
  };
  
-static const struct resource_create_funcs res_create_maximus_funcs = {

-   .read_dce_straps = NULL,
-   .create_audio = NULL,
-   .create_stream_encoder = NULL,
-   .create_hwseq = dcn10_hwseq_create,
-};
-
  static void dcn10_clock_source_destroy(struct clock_source **clk_src)
  {
kfree(TO_DCE110_CLK_SRC(*clk_src));

--
Hamza

Re: [PATCH] drm/amd/display: avoid calling missing .resync_fifo_dccg_dio()

[Hamza Mahfooz]

On 5/23/23 04:34, Arnd Bergmann wrote:

From: Arnd Bergmann 

The .resync_fifo_dccg_dio() callback pointer was added in an #ifdef block,
but is called unconditionally:

drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_hw_sequencer.c:2292:31: 
error: 'struct hwseq_private_funcs' has no member named 'resync_fifo_dccg_dio'

Add the same #ifdef around the caller as well.

Fixes: 6354b0dc3a7a ("drm/amd/display: Trigger DIO FIFO resync on commit 
streams")
Signed-off-by: Arnd Bergmann 


Applied, thanks!


---
  drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index c6fe2c00aedb..d4cacb8df631 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -2289,8 +2289,10 @@ enum dc_status dce110_apply_ctx_to_hw(
if (DC_OK != status)
return status;
  
+#ifdef CONFIG_DRM_AMD_DC_FP

if (hws->funcs.resync_fifo_dccg_dio)
hws->funcs.resync_fifo_dccg_dio(hws, dc, context);
+#endif
}
  
  	if (dc->fbc_compressor)

--
Hamza

Re: [PATCH] drm/amd/display: remove unused variables res_create_maximus_funcs and debug_defaults_diags

[Hamza Mahfooz]

On 5/23/23 07:49, Tom Rix wrote:

gcc with W=1 reports
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_resource.c:1069:43: error:
   ‘res_create_maximus_funcs’ defined but not used 
[-Werror=unused-const-variable=]
  1069 | static const struct resource_create_funcs res_create_maximus_funcs = {
   |   ^~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_resource.c:727:38: error:
   ‘debug_defaults_diags’ defined but not used [-Werror=unused-const-variable=]
   727 | static const struct dc_debug_options debug_defaults_diags = {
   |  ^~~~

These variables are not used so remove them.

Signed-off-by: Tom Rix 


Fixes: 00df97e1df57 ("drm/amd/display: Clean FPGA code in dc")

Applied, thanks!


---
  .../drm/amd/display/dc/dcn20/dcn20_resource.c | 23 ---
  1 file changed, 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 7dcae3183e07..6ef7e2634991 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -724,22 +724,6 @@ static const struct dc_debug_options debug_defaults_drv = {
.underflow_assert_delay_us = 0x,
  };
  
-static const struct dc_debug_options debug_defaults_diags = {

-   .disable_dmcu = false,
-   .force_abm_enable = false,
-   .timing_trace = true,
-   .clock_trace = true,
-   .disable_dpp_power_gate = true,
-   .disable_hubp_power_gate = true,
-   .disable_clock_gate = true,
-   .disable_pplib_clock_request = true,
-   .disable_pplib_wm_range = true,
-   .disable_stutter = true,
-   .scl_reset_length10 = true,
-   .underflow_assert_delay_us = 0x,
-   .enable_tri_buf = true,
-};
-
  void dcn20_dpp_destroy(struct dpp **dpp)
  {
kfree(TO_DCN20_DPP(*dpp));
@@ -1066,13 +1050,6 @@ static const struct resource_create_funcs 
res_create_funcs = {
.create_hwseq = dcn20_hwseq_create,
  };
  
-static const struct resource_create_funcs res_create_maximus_funcs = {

-   .read_dce_straps = NULL,
-   .create_audio = NULL,
-   .create_stream_encoder = NULL,
-   .create_hwseq = dcn20_hwseq_create,
-};
-
  static void dcn20_pp_smu_destroy(struct pp_smu_funcs **pp_smu);
  
  void dcn20_clock_source_destroy(struct clock_source **clk_src)

--
Hamza

[PATCH 3/3] drm/amd/display: drop unused count variable in create_eml_sink()

[Hamza Mahfooz]

Since, we are only interested in having
drm_edid_override_connector_update(), update the value of
connector->edid_blob_ptr. We don't care about the return value of
drm_edid_override_connector_update() here. So, drop count.

Fixes: 068553e14f86 ("drm/amd/display: assign edid_blob_ptr with edid from 
debugfs")
Reported-by: kernel test robot 
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

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 14b296e1d0f6..5a2d04f47276 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6396,9 +6396,8 @@ static void create_eml_sink(struct amdgpu_dm_connector 
*aconnector)
/* if connector->edid_override valid, pass
 * it to edid_override to edid_blob_ptr
 */
-   int count;
 
-   count = drm_edid_override_connector_update(>base);
+   drm_edid_override_connector_update(>base);
 
if (!aconnector->base.edid_blob_ptr) {
DRM_ERROR("No EDID firmware found on connector: %s 
,forcing to OFF!\n",
-- 
2.40.1

[PATCH 2/3] drm/amd/display: drop unused function set_abm_event()

[Hamza Mahfooz]

set_abm_event() is never actually used. So, drop it.

Fixes: b46c01aa0329 ("drm/amd/display: Refactor ABM feature")
Reported-by: kernel test robot 
Reported-by: Tom Rix 
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c | 12 
 drivers/gpu/drm/amd/display/dc/inc/hw/abm.h   |  2 --
 2 files changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c 
b/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c
index a66f83a61402..2fb9572ce25d 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c
@@ -131,17 +131,6 @@ static bool dmub_abm_set_pipe_ex(struct abm *abm, uint32_t 
otg_inst, uint32_t op
return ret;
 }
 
-static bool dmub_abm_set_event_ex(struct abm *abm, unsigned int full_screen, 
unsigned int video_mode,
-   unsigned int hdr_mode, unsigned int panel_inst)
-{
-   bool ret = false;
-   unsigned int feature_support;
-
-   feature_support = abm_feature_support(abm, panel_inst);
-
-   return ret;
-}
-
 static bool dmub_abm_set_backlight_level_pwm_ex(struct abm *abm,
unsigned int backlight_pwm_u16_16,
unsigned int frame_ramp,
@@ -167,7 +156,6 @@ static const struct abm_funcs abm_funcs = {
.init_abm_config = dmub_abm_init_config_ex,
.set_abm_pause = dmub_abm_set_pause_ex,
.set_pipe_ex = dmub_abm_set_pipe_ex,
-   .set_abm_event = dmub_abm_set_event_ex,
.set_backlight_level_pwm = dmub_abm_set_backlight_level_pwm_ex,
 };
 
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/abm.h 
b/drivers/gpu/drm/amd/display/dc/inc/hw/abm.h
index db5cf9acafe6..d2190a3320f6 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/abm.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/abm.h
@@ -59,8 +59,6 @@ struct abm_funcs {
unsigned int otg_inst,
unsigned int option,
unsigned int panel_inst);
-   bool (*set_abm_event)(struct abm *abm, unsigned int full_screen, 
unsigned int video_mode,
-   unsigned int hdr_mode, unsigned int panel_inst);
 };
 
 #endif
-- 
2.40.1

[PATCH 1/3] drm/amd/display: drop redundant memset() in get_available_dsc_slices()

[Hamza Mahfooz]

get_available_dsc_slices() returns the number of indices set, and all of
the users of get_available_dsc_slices() don't cross the returned bound
when iterating over available_slices[]. So, the memset() in
get_available_dsc_slices() is redundant and can be dropped.

Fixes: 97bda0322b8a ("drm/amd/display: Add DSC support for Navi (v2)")
Reported-by: Christophe JAILLET 
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c 
b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
index b9a05bb025db..58dd62cce4bb 100644
--- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
+++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c
@@ -645,8 +645,6 @@ static int get_available_dsc_slices(union 
dsc_enc_slice_caps slice_caps, int *av
 {
int idx = 0;
 
-   memset(available_slices, -1, MIN_AVAILABLE_SLICES_SIZE);
-
if (slice_caps.bits.NUM_SLICES_1)
available_slices[idx++] = 1;
 
-- 
2.40.1

Re: [PATCH] drm/amd/display: Simplify the calculation of variables

[Hamza Mahfooz]

On 5/12/23 03:04, Jiapeng Chong wrote:

./drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c:586:37-39: WARNING !A || A 
&& B is equivalent to !A || B.
./drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c:595:37-39: WARNING !A || A 
&& B is equivalent to !A || B.

Reported-by: Abaci Robot 
Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4941
Signed-off-by: Jiapeng Chong 


Applied, thanks!


---
  drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c
index 4950eaa4406b..2de910e0ce75 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_hwseq.c
@@ -583,8 +583,8 @@ void dcn32_update_force_pstate(struct dc *dc, struct 
dc_state *context)
struct pipe_ctx *pipe = >res_ctx.pipe_ctx[i];
struct hubp *hubp = pipe->plane_res.hubp;
  
-		if (!pipe->stream || (pipe->stream && !(pipe->stream->mall_stream_config.type == SUBVP_MAIN ||

-   pipe->stream->fpo_in_use))) {
+   if (!pipe->stream || !(pipe->stream->mall_stream_config.type == 
SUBVP_MAIN ||
+   pipe->stream->fpo_in_use)) {
if (hubp && 
hubp->funcs->hubp_update_force_pstate_disallow)

hubp->funcs->hubp_update_force_pstate_disallow(hubp, false);
}
@@ -592,7 +592,7 @@ void dcn32_update_force_pstate(struct dc *dc, struct 
dc_state *context)
/* Today only FPO uses cursor P-State force. Only clear cursor 
P-State force
 * if it's not FPO.
 */
-   if (!pipe->stream || (pipe->stream && 
!pipe->stream->fpo_in_use)) {
+   if (!pipe->stream || !pipe->stream->fpo_in_use) {
if (hubp && 
hubp->funcs->hubp_update_force_cursor_pstate_disallow)

hubp->funcs->hubp_update_force_cursor_pstate_disallow(hubp, false);
}

--
Hamza

Re: [PATCH V3] drm/amdgpu/display: Enable DC_FP for LoongArch

[Hamza Mahfooz]

On 5/7/23 23:09, Huacai Chen wrote:

LoongArch now provides kernel_fpu_begin() and kernel_fpu_end() that are
used like the x86 counterparts in commit 2b3bd32ea3a22ea2d ("LoongArch:
Provide kernel fpu functions"), so we can enable DC_FP on LoongArch for
supporting more DCN devices.

Signed-off-by: WANG Xuerui 
Signed-off-by: Huacai Chen 


Applied, thanks!


---
V2: Update commit message to add the commit which provides kernel fpu
 functions.
V3: Update commit message again and rebase on the latest code.

  drivers/gpu/drm/amd/display/Kconfig| 2 +-
  drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 6 --
  drivers/gpu/drm/amd/display/dc/dml/Makefile| 5 +
  3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index 2d8e55e29637..49df073962d5 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -8,7 +8,7 @@ config DRM_AMD_DC
depends on BROKEN || !CC_IS_CLANG || X86_64 || SPARC64 || ARM64
select SND_HDA_COMPONENT if SND_HDA_CORE
# !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
-   select DRM_AMD_DC_FP if (X86 || (PPC64 && ALTIVEC) || (ARM64 && 
KERNEL_MODE_NEON && !CC_IS_CLANG))
+   select DRM_AMD_DC_FP if (X86 || LOONGARCH || (PPC64 && ALTIVEC) || (ARM64 && 
KERNEL_MODE_NEON && !CC_IS_CLANG))
help
  Choose this option if you want to use the new display engine
  support for AMDGPU. This adds required support for Vega and
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
index c42aa947c969..172aa10a8800 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
@@ -33,6 +33,8 @@
  #include 
  #elif defined(CONFIG_ARM64)
  #include 
+#elif defined(CONFIG_LOONGARCH)
+#include 
  #endif
  
  /**

@@ -88,7 +90,7 @@ void dc_fpu_begin(const char *function_name, const int line)
*pcpu += 1;
  
  	if (*pcpu == 1) {

-#if defined(CONFIG_X86)
+#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
migrate_disable();
kernel_fpu_begin();
  #elif defined(CONFIG_PPC64)
@@ -128,7 +130,7 @@ void dc_fpu_end(const char *function_name, const int line)
pcpu = get_cpu_ptr(_recursion_depth);
*pcpu -= 1;
if (*pcpu <= 0) {
-#if defined(CONFIG_X86)
+#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
kernel_fpu_end();
migrate_enable();
  #elif defined(CONFIG_PPC64)
diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile 
b/drivers/gpu/drm/amd/display/dc/dml/Makefile
index 01db035589c5..77cf5545c94c 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
@@ -38,6 +38,11 @@ ifdef CONFIG_ARM64
  dml_rcflags := -mgeneral-regs-only
  endif
  
+ifdef CONFIG_LOONGARCH

+dml_ccflags := -mfpu=64
+dml_rcflags := -msoft-float
+endif
+
  ifdef CONFIG_CC_IS_GCC
  ifneq ($(call gcc-min-version, 70100),y)
  IS_OLD_GCC = 1

--
Hamza

Re: [PATCH V2] drm/amdgpu/display: Enable DC_FP for LoongArch

[Hamza Mahfooz]

Hey Huacai,

On 5/5/23 07:32, Huacai Chen wrote:

Now LoongArch provides kernel_fpu_begin() and kernel_fpu_end() in commit
2b3bd32ea3a22ea2d ("LoongArch: Provide kernel fpu functions"), so we can
enable DC_FP for DCN devices.


Have you had the chance to test how well this is working on actual
hardware, or was it only compile tested? If it was only compile tested,
it would be great if you could run some tests. Please see the following
for more details:
https://lore.kernel.org/amd-gfx/8eb69dfb-ae35-dbf2-3f82-e8cc00e53...@amd.com/



Signed-off-by: WANG Xuerui 
Signed-off-by: Huacai Chen 
---
V2: Update commit message to add the commit which provides kernel fpu
 functions.

  drivers/gpu/drm/amd/display/Kconfig| 2 +-
  drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 6 --
  drivers/gpu/drm/amd/display/dc/dml/Makefile| 5 +
  3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index 2d8e55e29637..49df073962d5 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -8,7 +8,7 @@ config DRM_AMD_DC
depends on BROKEN || !CC_IS_CLANG || X86_64 || SPARC64 || ARM64
select SND_HDA_COMPONENT if SND_HDA_CORE
# !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
-   select DRM_AMD_DC_FP if (X86 || (PPC64 && ALTIVEC) || (ARM64 && 
KERNEL_MODE_NEON && !CC_IS_CLANG))
+   select DRM_AMD_DC_FP if (X86 || LOONGARCH || (PPC64 && ALTIVEC) || (ARM64 && 
KERNEL_MODE_NEON && !CC_IS_CLANG))
help
  Choose this option if you want to use the new display engine
  support for AMDGPU. This adds required support for Vega and
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
index 1743ca0a3641..86f4c0e04654 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
@@ -33,6 +33,8 @@
  #include 
  #elif defined(CONFIG_ARM64)
  #include 
+#elif defined(CONFIG_LOONGARCH)
+#include 
  #endif
  
  /**

@@ -88,7 +90,7 @@ void dc_fpu_begin(const char *function_name, const int line)
*pcpu += 1;
  
  	if (*pcpu == 1) {

-#if defined(CONFIG_X86)
+#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
kernel_fpu_begin();
  #elif defined(CONFIG_PPC64)
if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
@@ -127,7 +129,7 @@ void dc_fpu_end(const char *function_name, const int line)
pcpu = get_cpu_ptr(_recursion_depth);
*pcpu -= 1;
if (*pcpu <= 0) {
-#if defined(CONFIG_X86)
+#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
kernel_fpu_end();
  #elif defined(CONFIG_PPC64)
if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile 
b/drivers/gpu/drm/amd/display/dc/dml/Makefile
index 01db035589c5..542962a93e8f 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
@@ -38,6 +38,11 @@ ifdef CONFIG_ARM64
  dml_rcflags := -mgeneral-regs-only
  endif
  
+ifdef CONFIG_LOONGARCH

+dml_ccflags := -mfpu=64
+dml_rcflags := -msoft-float
+endif
+
  ifdef CONFIG_CC_IS_GCC
  ifneq ($(call gcc-min-version, 70100),y)
  IS_OLD_GCC = 1

--
Hamza

[PATCH] drm/amdgpu: fix an amdgpu_irq_put() issue in gmc_v9_0_hw_fini()

[Hamza Mahfooz]

As made mention of, in commit 9128e6babf10 ("drm/amdgpu: fix
amdgpu_irq_put call trace in gmc_v10_0_hw_fini") and commit c094b8923bdd
("drm/amdgpu: fix amdgpu_irq_put call trace in gmc_v11_0_hw_fini"). It
is meaningless to call amdgpu_irq_put() for gmc.ecc_irq. So, remove it
from gmc_v9_0_hw_fini().

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2522
Fixes: 3029c855d79f ("drm/amdgpu: Fix desktop freezed after gpu-reset")
Signed-off-by: Hamza Mahfooz 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 290804a06e05..6ae5cee9b64b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1999,7 +1999,6 @@ static int gmc_v9_0_hw_fini(void *handle)
if (adev->mmhub.funcs->update_power_gating)
adev->mmhub.funcs->update_power_gating(adev, false);
 
-   amdgpu_irq_put(adev, >gmc.ecc_irq, 0);
amdgpu_irq_put(adev, >gmc.vm_fault, 0);
 
return 0;
-- 
2.40.0

Re: [PATCH] drm/amd/display: mark amdgpu_dm_connector_funcs_force static

[Hamza Mahfooz]

On 5/1/23 10:31, Arnd Bergmann wrote:

From: Arnd Bergmann 

A global function without a header prototype has made it into
linux-next during the merge window:

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6339:6: error: no 
previous prototype for 'amdgpu_dm_connector_funcs_force' 
[-Werror=missing-prototypes]

Mark the function static instead, as there are no other
callers outside this file.

Fixes: 0ba4a784a145 ("drm/amd/display: implement force function in 
amdgpu_dm_connector_funcs")
Reported-by: kernel test robot 
Link: https://lore.kernel.org/oe-kbuild-all/202304251640.jclqtim9-...@intel.com/
Signed-off-by: Arnd Bergmann 


Applied, thanks!


---
This was previously reported by a bot for the drm-next tree but remains
broken in linux-next-20230428. Sending it out as I needed this fix
for my local builds.
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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 3647d21d688f..2bbb2988942d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6336,7 +6336,7 @@ amdgpu_dm_connector_late_register(struct drm_connector 
*connector)
return 0;
  }
  
-void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)

+static void amdgpu_dm_connector_funcs_force(struct drm_connector *connector)
  {
struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);
struct dc_link *dc_link = aconnector->dc_link;

--
Hamza

Re: [PATCH next] drm/amd/display: Fix possible NULL dereference in dc_dmub_srv_cmd_run_list()

[Hamza Mahfooz]

On 4/26/23 15:24, Harshit Mogalapalli wrote:

We have a NULL check for 'dc_dmub_srv' in dc_dmub_srv_cmd_run_list()
but we are dereferencing it before checking.

Fix this moving the dereference next to NULL check.

This issue is found with Smatch(static analysis tool).

Fixes: e97cc04fe0fb ("drm/amd/display: refactor dmub commands into single 
function")
Signed-off-by: Harshit Mogalapalli 


Applied, thanks!


---
Only compile tested.
---
  drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c 
b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
index d15ec32243e2..62d3473c32bc 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
+++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c
@@ -125,7 +125,7 @@ bool dc_dmub_srv_cmd_run(struct dc_dmub_srv *dc_dmub_srv, 
union dmub_rb_cmd *cmd
  
  bool dc_dmub_srv_cmd_run_list(struct dc_dmub_srv *dc_dmub_srv, unsigned int count, union dmub_rb_cmd *cmd_list, enum dm_dmub_wait_type wait_type)

  {
-   struct dc_context *dc_ctx = dc_dmub_srv->ctx;
+   struct dc_context *dc_ctx;
struct dmub_srv *dmub;
enum dmub_status status;
int i;
@@ -133,6 +133,7 @@ bool dc_dmub_srv_cmd_run_list(struct dc_dmub_srv 
*dc_dmub_srv, unsigned int coun
if (!dc_dmub_srv || !dc_dmub_srv->dmub)
return false;
  
+	dc_ctx = dc_dmub_srv->ctx;

dmub = dc_dmub_srv->dmub;
  
  	for (i = 0 ; i < count; i++) {

--
Hamza