Re: [Freedreno] 2023 X.Org Foundation Membership deadline for voting in the election

2023-04-18 Thread Harald Koenig
On Apr 17, Laurent Pinchart wrote:

> I don't know if I'm the only one affected by this issue, but I've just
> received today two months of e-mails from x.org, including all the
> reminders aboud membership renewal and election nomination period. This
> isn't the first time this happens, and the last time I was told there
> was no automated process to quick the mail queues when errors happen,
> making mails pile up forever on x.org's side until someone handles it
> manually. This is something you really want to automate, or at least
> monitored.

same here for me: looking into the mail header,
both mails were stuck on server "gabe.freedesktop.org" 

Received: from gabe.freedesktop.org (localhost [127.0.0.1])
by gabe.freedesktop.org (Postfix) with ESMTP id BD01310E459;
Mon, 17 Apr 2023 11:42:45 + (UTC)
X-Original-To: eve...@lists.x.org
Delivered-To: eve...@lists.x.org
Received: from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])
 by gabe.freedesktop.org (Postfix) with ESMTPS id 6C54510E162;
 Wed, 15 Feb 2023 15:58:10 + (UTC)

and 

Received: from gabe.freedesktop.org (localhost [127.0.0.1])
by gabe.freedesktop.org (Postfix) with ESMTP id 6735010E46D;
Mon, 17 Apr 2023 11:42:45 + (UTC)
X-Original-To: eve...@lists.x.org
Delivered-To: eve...@lists.x.org
Received: from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])
 by gabe.freedesktop.org (Postfix) with ESMTPS id 98DB48953E;
 Mon, 13 Mar 2023 15:23:02 + (UTC)



Harald
-- 
"I hope to die  ___   _
before I *have* to use Microsoft Word.",   0--,|/OOO\
Donald E. Knuth, 02-Oct-2001 in Tuebingen.<_/  /  /OOO\
\  \/OOO\
  \ O|//
   \/\/\/\/\/\/\/\/\/
Harald Koenig   //  / \\  \
harald.koe...@mailbox.org  ^   ^


[Freedreno] [PATCH 1/4] drm/msm: add some cec register bitfield details

2023-04-18 Thread Arnaud Vrac
The register names and bitfields were determined from the downstream
msm-4.4 driver.

Signed-off-by: Arnaud Vrac 
---
 drivers/gpu/drm/msm/hdmi/hdmi.xml.h | 62 -
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h 
b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
index 973b460486a5a..b4dd6e8cba6b7 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
@@ -76,6 +76,13 @@ enum hdmi_acr_cts {
ACR_48 = 3,
 };
 
+enum hdmi_cec_tx_status {
+   CEC_TX_OK = 0,
+   CEC_TX_NACK = 1,
+   CEC_TX_ARB_LOSS = 2,
+   CEC_TX_MAX_RETRIES = 3,
+};
+
 #define REG_HDMI_CTRL  0x
 #define HDMI_CTRL_ENABLE   0x0001
 #define HDMI_CTRL_HDMI 0x0002
@@ -476,20 +483,73 @@ static inline uint32_t HDMI_DDC_REF_REFTIMER(uint32_t val)
 #define REG_HDMI_HDCP_SW_LOWER_AKSV0x0288
 
 #define REG_HDMI_CEC_CTRL  0x028c
+#define HDMI_CEC_CTRL_ENABLE   0x0001
+#define HDMI_CEC_CTRL_SEND_TRIGGER 0x0002
+#define HDMI_CEC_CTRL_FRAME_SIZE__MASK 0x01f0
+#define HDMI_CEC_CTRL_FRAME_SIZE__SHIFT4
+static inline uint32_t HDMI_CEC_CTRL_FRAME_SIZE(uint32_t val)
+{
+   return ((val) << HDMI_CEC_CTRL_FRAME_SIZE__SHIFT) & 
HDMI_CEC_CTRL_FRAME_SIZE__MASK;
+}
+#define HDMI_CEC_CTRL_LINE_OE  0x0200
 
 #define REG_HDMI_CEC_WR_DATA   0x0290
+#define HDMI_CEC_WR_DATA_BROADCAST 0x0001
+#define HDMI_CEC_WR_DATA_DATA__MASK0xff00
+#define HDMI_CEC_WR_DATA_DATA__SHIFT   8
+static inline uint32_t HDMI_CEC_WR_DATA_DATA(uint32_t val)
+{
+   return ((val) << HDMI_CEC_WR_DATA_DATA__SHIFT) & 
HDMI_CEC_WR_DATA_DATA__MASK;
+}
 
-#define REG_HDMI_CEC_CEC_RETRANSMIT0x0294
+#define REG_HDMI_CEC_RETRANSMIT
0x0294
+#define HDMI_CEC_RETRANSMIT_ENABLE 0x0001
+#define HDMI_CEC_RETRANSMIT_COUNT__MASK
0x00fe
+#define HDMI_CEC_RETRANSMIT_COUNT__SHIFT   1
+static inline uint32_t HDMI_CEC_RETRANSMIT_COUNT(uint32_t val)
+{
+   return ((val) << HDMI_CEC_RETRANSMIT_COUNT__SHIFT) & 
HDMI_CEC_RETRANSMIT_COUNT__MASK;
+}
 
 #define REG_HDMI_CEC_STATUS0x0298
+#define HDMI_CEC_STATUS_BUSY   0x0001
+#define HDMI_CEC_STATUS_TX_FRAME_DONE  0x0008
+#define HDMI_CEC_STATUS_TX_STATUS__MASK
0x00f0
+#define HDMI_CEC_STATUS_TX_STATUS__SHIFT   4
+static inline uint32_t HDMI_CEC_STATUS_TX_STATUS(enum hdmi_cec_tx_status val)
+{
+   return ((val) << HDMI_CEC_STATUS_TX_STATUS__SHIFT) & 
HDMI_CEC_STATUS_TX_STATUS__MASK;
+}
 
 #define REG_HDMI_CEC_INT   0x029c
+#define HDMI_CEC_INT_TX_DONE   0x0001
+#define HDMI_CEC_INT_TX_DONE_MASK  0x0002
+#define HDMI_CEC_INT_TX_ERROR  0x0004
+#define HDMI_CEC_INT_TX_ERROR_MASK 0x0008
+#define HDMI_CEC_INT_MONITOR   0x0010
+#define HDMI_CEC_INT_MONITOR_MASK  0x0020
+#define HDMI_CEC_INT_RX_DONE   0x0040
+#define HDMI_CEC_INT_RX_DONE_MASK  0x0080
 
 #define REG_HDMI_CEC_ADDR  0x02a0
 
 #define REG_HDMI_CEC_TIME  0x02a4
+#define HDMI_CEC_TIME_ENABLE   0x0001
+#define HDMI_CEC_TIME_SIGNAL_FREE_TIME__MASK   0xff80
+#define HDMI_CEC_TIME_SIGNAL_FREE_TIME__SHIFT  7
+static inline uint32_t HDMI_CEC_TIME_SIGNAL_FREE_TIME(uint32_t val)
+{
+   return ((val) << HDMI_CEC_TIME_SIGNAL_FREE_TIME__SHIFT) & 
HDMI_CEC_TIME_SIGNAL_FREE_TIME__MASK;
+}
 
 #define REG_HDMI_CEC_REFTIMER  0x02a8
+#define HDMI_CEC_REFTIMER_ENABLE   0x0001
+#define HDMI_CEC_REFTIMER_REFTIMER__MASK   0x
+#define HDMI_CEC_REFTIMER_REFTIMER__SHIFT  0
+static inline uint32_t HDMI_CEC_REFTIMER_REFTIMER(uint32_t val)
+{
+   return ((val) << HDMI_CEC_REFTIMER_REFTIMER__SHIFT) & 
HDMI_CEC_REFTIMER_REFTIMER__MASK;
+}
 
 #define REG_HDMI_CEC_RD_DATA   0x02ac
 

-- 
2.40.0



[Freedreno] [PATCH 2/4] drm/msm: add hdmi cec support

2023-04-18 Thread Arnaud Vrac
Some Qualcomm SoCs that support HDMI also support CEC, including MSM8996
and MSM8998. The hardware block can handle a single CEC logical address
and broadcast messages.

Port the CEC driver from downstream msm-4.4 kernel. It has been tested
on MSM8998 and passes the cec-compliance tool tests.

Signed-off-by: Arnaud Vrac 
---
 drivers/gpu/drm/msm/Kconfig |   8 ++
 drivers/gpu/drm/msm/Makefile|   1 +
 drivers/gpu/drm/msm/hdmi/hdmi.c |  15 ++
 drivers/gpu/drm/msm/hdmi/hdmi.h |  18 +++
 drivers/gpu/drm/msm/hdmi/hdmi_cec.c | 280 
 5 files changed, 322 insertions(+)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 85f5ab1d552c4..2a02c74207935 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -165,3 +165,11 @@ config DRM_MSM_HDMI_HDCP
default y
help
  Choose this option to enable HDCP state machine
+
+config DRM_MSM_HDMI_CEC
+   bool "Enable HDMI CEC support in MSM DRM driver"
+   depends on DRM_MSM && DRM_MSM_HDMI
+   select CEC_CORE
+   default y
+   help
+ Choose this option to enable CEC support
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 7274c41228ed9..0237a2f219ac2 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -131,6 +131,7 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
 
 msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
 
+msm-$(CONFIG_DRM_MSM_HDMI_CEC) += hdmi/hdmi_cec.o
 msm-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o
 
 msm-$(CONFIG_DRM_MSM_DSI) += dsi/dsi.o \
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 3132105a2a433..1dde3890e25c0 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -11,6 +11,8 @@
 #include 
 #include 
 
+#include 
+
 #include 
 #include "hdmi.h"
 
@@ -53,6 +55,9 @@ static irqreturn_t msm_hdmi_irq(int irq, void *dev_id)
if (hdmi->hdcp_ctrl)
msm_hdmi_hdcp_irq(hdmi->hdcp_ctrl);
 
+   /* Process CEC: */
+   msm_hdmi_cec_irq(hdmi);
+
/* TODO audio.. */
 
return IRQ_HANDLED;
@@ -66,6 +71,8 @@ static void msm_hdmi_destroy(struct hdmi *hdmi)
 */
if (hdmi->workq)
destroy_workqueue(hdmi->workq);
+
+   msm_hdmi_cec_exit(hdmi);
msm_hdmi_hdcp_destroy(hdmi);
 
if (hdmi->i2c)
@@ -139,6 +146,8 @@ static int msm_hdmi_init(struct hdmi *hdmi)
hdmi->hdcp_ctrl = NULL;
}
 
+   msm_hdmi_cec_init(hdmi);
+
return 0;
 
 fail:
@@ -198,6 +207,12 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
 
drm_connector_attach_encoder(hdmi->connector, hdmi->encoder);
 
+   if (hdmi->cec_adap) {
+   struct cec_connector_info conn_info;
+   cec_fill_conn_info_from_drm(_info, hdmi->connector);
+   cec_s_conn_info(hdmi->cec_adap, _info);
+   }
+
ret = devm_request_irq(dev->dev, hdmi->irq,
msm_hdmi_irq, IRQF_TRIGGER_HIGH,
"hdmi_isr", hdmi);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index e8dbee50637fa..c639bd87f4b8f 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -29,6 +29,7 @@ struct hdmi_audio {
 };
 
 struct hdmi_hdcp_ctrl;
+struct cec_adapter;
 
 struct hdmi {
struct drm_device *dev;
@@ -73,6 +74,7 @@ struct hdmi {
struct workqueue_struct *workq;
 
struct hdmi_hdcp_ctrl *hdcp_ctrl;
+   struct cec_adapter *cec_adap;
 
/*
* spinlock to protect registers shared by different execution
@@ -261,4 +263,20 @@ static inline void msm_hdmi_hdcp_off(struct hdmi_hdcp_ctrl 
*hdcp_ctrl) {}
 static inline void msm_hdmi_hdcp_irq(struct hdmi_hdcp_ctrl *hdcp_ctrl) {}
 #endif
 
+/*
+ * cec
+ */
+#ifdef CONFIG_DRM_MSM_HDMI_CEC
+int msm_hdmi_cec_init(struct hdmi *hdmi);
+void msm_hdmi_cec_exit(struct hdmi *hdmi);
+void msm_hdmi_cec_irq(struct hdmi *hdmi);
+#else
+static inline int msm_hdmi_cec_init(struct hdmi *hdmi)
+{
+   return -ENXIO;
+}
+static inline void msm_hdmi_cec_exit(struct hdmi *hdmi) {}
+static inline void msm_hdmi_cec_irq(struct hdmi *hdmi) {}
+#endif
+
 #endif /* __HDMI_CONNECTOR_H__ */
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_cec.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_cec.c
new file mode 100644
index 0..51326e493e5da
--- /dev/null
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_cec.c
@@ -0,0 +1,280 @@
+#include 
+#include 
+
+#include "hdmi.h"
+
+#define HDMI_CEC_INT_MASK ( \
+   HDMI_CEC_INT_TX_DONE_MASK | \
+   HDMI_CEC_INT_TX_ERROR_MASK | \
+   HDMI_CEC_INT_RX_DONE_MASK)
+
+struct hdmi_cec_ctrl {
+   struct hdmi *hdmi;
+   struct work_struct work;
+   spinlock_t lock;
+   u32 irq_status;
+   u32 tx_status;
+   u32 tx_retransmits;
+};
+
+static int msm_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
+{
+   

[Freedreno] [PATCH 3/4] drm/msm: expose edid to hdmi cec adapter

2023-04-18 Thread Arnaud Vrac
When edid has been read after hpd, pass it to the cec adapter so that it
can extract the physical address of the device on the cec bus.
Invalidate the physical address when hpd is low.

Signed-off-by: Arnaud Vrac 
---
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 ++
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 17 +
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 9b1391d27ed39..efc3bd4908e83 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "msm_kms.h"
 #include "hdmi.h"
@@ -256,6 +257,7 @@ static struct edid *msm_hdmi_bridge_get_edid(struct 
drm_bridge *bridge,
hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl | HDMI_CTRL_ENABLE);
 
edid = drm_get_edid(connector, hdmi->i2c);
+   cec_s_phys_addr_from_edid(hdmi->cec_adap, edid);
 
hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl);
 
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index bfa827b479897..cb3eb2625ff63 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "msm_kms.h"
 #include "hdmi.h"
@@ -230,15 +231,17 @@ enum drm_connector_status msm_hdmi_bridge_detect(
 {
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
-   enum drm_connector_status stat_gpio, stat_reg;
+   enum drm_connector_status status, stat_gpio, stat_reg;
int retry = 20;
 
/*
 * some platforms may not have hpd gpio. Rely only on the status
 * provided by REG_HDMI_HPD_INT_STATUS in this case.
 */
-   if (!hdmi->hpd_gpiod)
-   return detect_reg(hdmi);
+   if (!hdmi->hpd_gpiod) {
+   status = detect_reg(hdmi);
+   goto out;
+   }
 
do {
stat_gpio = detect_gpio(hdmi);
@@ -259,5 +262,11 @@ enum drm_connector_status msm_hdmi_bridge_detect(
DBG("hpd gpio tells us: %d", stat_gpio);
}
 
-   return stat_gpio;
+   status = stat_gpio;
+
+out:
+   if (!status)
+   cec_phys_addr_invalidate(hdmi->cec_adap);
+
+   return status;
 }

-- 
2.40.0



[Freedreno] [PATCH 4/4] arm64: dts: qcom: msm8998: add hdmi cec pinctrl nodes

2023-04-18 Thread Arnaud Vrac
HDMI is not enabled yet on msm8998 so the pinctrl nodes are not
used.

Signed-off-by: Arnaud Vrac 
---
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi 
b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index b150437a83558..fb4aa376ef117 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -1312,6 +1312,20 @@ blsp2_i2c6_sleep: blsp2-i2c6-sleep-state {
drive-strength = <2>;
bias-pull-up;
};
+
+   hdmi_cec_default: hdmi-cec-default-state {
+   pins = "gpio31";
+   function = "hdmi_cec";
+   drive-strength = <2>;
+   bias-pull-up;
+   };
+
+   hdmi_cec_sleep: hdmi-cec-sleep-state {
+   pins = "gpio31";
+   function = "hdmi_cec";
+   drive-strength = <2>;
+   bias-pull-up;
+   };
};
 
remoteproc_mss: remoteproc@408 {

-- 
2.40.0



[Freedreno] [PATCH 0/4] Support HDMI CEC on Qualcomm SoCs

2023-04-18 Thread Arnaud Vrac
Some Qualcomm SoCs that support HDMI also support CEC, including MSM8996
and MSM8998. The hardware block can handle a single CEC logical address
and broadcast messages.

Port the CEC driver from downstream msm-4.4 kernel. It has been tested
on MSM8998 and passes the cec-compliance tool tests. The equivalent
downstream driver also passed CEC CTS tests using a Quantum Data QD882E
analyzer.

Some registers bitfield definitions were added to make the code clearer,
and those will also be proposed for upstream in the original xml file
from which the header was generated, in the mesa project.

Note HDMI support is not yet included upstream for MSM8998, I would
appreciate if someone can verify this driver at least works on MSM8996,
for which adding the pinctrl nodes for CEC should be sufficient.

Signed-off-by: Arnaud Vrac 
---
Arnaud Vrac (4):
  drm/msm: add some cec register bitfield details
  drm/msm: add hdmi cec support
  drm/msm: expose edid to hdmi cec adapter
  arm64: dts: qcom: msm8998: add hdmi cec pinctrl nodes

 arch/arm64/boot/dts/qcom/msm8998.dtsi  |  14 ++
 drivers/gpu/drm/msm/Kconfig|   8 +
 drivers/gpu/drm/msm/Makefile   |   1 +
 drivers/gpu/drm/msm/hdmi/hdmi.c|  15 ++
 drivers/gpu/drm/msm/hdmi/hdmi.h|  18 +++
 drivers/gpu/drm/msm/hdmi/hdmi.xml.h|  62 +++-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |   2 +
 drivers/gpu/drm/msm/hdmi/hdmi_cec.c| 280 +
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c|  17 +-
 9 files changed, 412 insertions(+), 5 deletions(-)
---
base-commit: e3342532ecd39bbd9c2ab5b9001cec1589bc37e9
change-id: 20230418-msm8998-hdmi-cec-08b5890bf41e

Best regards,
-- 
Arnaud Vrac 



[Freedreno] [PATCH] drm/msm/atomic: Don't try async if crtc not active

2023-04-18 Thread Rob Clark
From: Rob Clark 

For a similar reason as commit f2c7ca890182 ("drm/atomic-helper: Don't
set deadline for modesets"), we need the crtc to be already active in
order to compute a target vblank time for an async commit.  Otherwise
we get this splat reminding us that we are doing it wrong:

   [ cut here ]
   msm_dpu ae01000.mdp: drm_WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev))
   WARNING: CPU: 7 PID: 1923 at drivers/gpu/drm/drm_vblank.c:728 
drm_crtc_vblank_helper_get_vblank_timestamp_internal+0x148/0x370
   Modules linked in: snd_seq_dummy snd_seq snd_seq_device bridge stp llc tun 
vhost_vsock vhost vhost_iotlb vmw_vsock_virtio_transport_common vsock uinput 
rfcomm algif_hash algif_skcipher af_alg veth venus_dec venus_enc cros_ec_typec 
typec qcom_spmi_temp_alarm qcom_spmi_adc_tm5 qcom_spmi_adc5 xt_cgroup 
qcom_vadc_common qcom_stats hci_uart btqca xt_MASQUERADE venus_core 8021q 
coresight_tmc coresight_funnel coresight_etm4x coresight_replicator 
snd_soc_lpass_sc7180 coresight snd_soc_sc7180 ip6table_nat fuse ath10k_snoc 
ath10k_core ath mac80211 iio_trig_sysfs bluetooth cfg80211 cros_ec_sensors 
cros_ec_sensors_core ecdh_generic industrialio_triggered_buffer ecc kfifo_buf 
cros_ec_sensorhub r8153_ecm cdc_ether usbnet r8152 mii lzo_rle lzo_compress 
zram hid_vivaldi hid_google_hammer hid_vivaldi_common joydev
   CPU: 7 PID: 1923 Comm: DrmThread Not tainted 
5.15.107-18853-g3be267609a0b-dirty #16 a1ffc1a66e79c21c3536d8c9a42e819236e39714
   Hardware name: Google Wormdingler rev1+ BOE panel board (DT)
   pstate: 6049 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
   pc : drm_crtc_vblank_helper_get_vblank_timestamp_internal+0x148/0x370
   lr : drm_crtc_vblank_helper_get_vblank_timestamp_internal+0x144/0x370
   sp : ffc012e2b800
   x29: ffc012e2b840 x28: ff8083676094 x27: ffc012e2bb28
   x26: ff8084539800 x25:  x24: ff8083676000
   x23: ffd3c8cdc5a0 x22: ff80845b9d00 x21: ffc012e2b8b4
   x20: ffc012e2b910 x19: 0001 x18: 
   x17:  x16: 0010 x15: ffd3c8451a88
   x14: 0003 x13: 0004 x12: 0001
   x11: c000dfff x10: ffd3c973ef58 x9 : 8ea3526b3cc95900
   x8 : 8ea3526b3cc95900 x7 :  x6 : 003a
   x5 : ffd3c99676cd x4 :  x3 : ffc012e2b4b8
   x2 : ffc012e2b4c0 x1 : dfff x0 : 
   Call trace:
drm_crtc_vblank_helper_get_vblank_timestamp_internal+0x148/0x370
drm_crtc_vblank_helper_get_vblank_timestamp+0x20/0x30
drm_crtc_get_last_vbltimestamp+0x68/0xb0
drm_crtc_next_vblank_start+0x5c/0xa8
msm_atomic_commit_tail+0x264/0x664
commit_tail+0xac/0x160
drm_atomic_helper_commit+0x160/0x168
drm_atomic_commit+0xfc/0x128
drm_atomic_helper_disable_plane+0x8c/0x110
__setplane_atomic+0x10c/0x138
drm_mode_cursor_common+0x3a8/0x410
drm_mode_cursor_ioctl+0x48/0x70
drm_ioctl_kernel+0xe0/0x158
drm_ioctl+0x25c/0x4d8
__arm64_sys_ioctl+0x98/0xd0
invoke_syscall+0x4c/0x100
el0_svc_common+0x98/0x104
do_el0_svc+0x30/0x90
el0_svc+0x20/0x50
el0t_64_sync_handler+0x78/0x108
el0t_64_sync+0x1a4/0x1a8
   ---[ end trace a0f587e1ab9589e8 ]---

Fixes: 52ff0d3073d2 ("drm/msm/atomic: Switch to vblank_start helper")
Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/msm_atomic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index c5e71c05f038..3871a6f2d8e1 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -155,6 +155,8 @@ static bool can_do_async(struct drm_atomic_state *state,
for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
if (drm_atomic_crtc_needs_modeset(crtc_state))
return false;
+   if (!crtc_state->active)
+   return false;
if (++num_crtcs > 1)
return false;
*async_crtc = crtc;
-- 
2.39.2



Re: [Freedreno] [PATCH 2/3] drm/msm/dpu: Assign missing writeback log_mask

2023-04-18 Thread Abhinav Kumar




On 4/17/2023 4:14 PM, Marijn Suijten wrote:

The WB debug log mask ended up never being assigned, leading to writes
to this block to never be logged even if the mask is enabled in
dpu_hw_util_log_mask via sysfs.


This should be debugfs not sysfs.



Fixes: 84a33d0fd921 ("drm/msm/dpu: add dpu_hw_wb abstraction for writeback 
blocks")
Signed-off-by: Marijn Suijten 
---


With that fixed,

Reviewed-by: Abhinav Kumar 


Re: [Freedreno] [RFC 3/3] drm/msm: Add comm/cmdline fields

2023-04-18 Thread Rob Clark
On Tue, Apr 18, 2023 at 1:53 AM Tvrtko Ursulin
 wrote:
>
>
> On 17/04/2023 21:12, Rob Clark wrote:
> > From: Rob Clark 
> >
> > Normally this would be the same information that can be obtained in
> > other ways.  But in some cases the process opening the drm fd is merely
> > a sort of proxy for the actual process using the GPU.  This is the case
> > for guest VM processes using the GPU via virglrenderer, in which case
> > the msm native-context renderer in virglrenderer overrides the comm/
> > cmdline to be the guest process's values.
> >
> > Exposing this via fdinfo allows tools like gputop to show something more
> > meaningful than just a bunch of "pcivirtio-gpu" users.
>
> You also later expanded with:
>
> """
> I should have also mentioned, in the VM/proxy scenario we have a
> single process with separate drm_file's for each guest VM process.  So
> it isn't an option to just change the proxy process's name to match
> the client.
> """
>
> So how does that work - this single process temporarily changes it's
> name for each drm fd it opens and creates a context or it is actually in
> the native context protocol?

It is part of the protocol, the mesa driver in the VM sends[1] this
info to the native-context "shim" in host userspace which uses the
SET_PARAM ioctl to pass this to the kernel.  In the host userspace
there is just a single process (you see the host PID below) but it
does a separate open() of the drm dev for each guest process (so that
they each have their own GPU address space for isolation):

DRM minor 128
PIDMEM ACTIV  NAMEgpu
5297  200M   82M com.mojang.minecr |██▏|
1859  199M0Bchrome |█▉ |
5297   64M9Msurfaceflinger |   |
5297   12M0B org.chromium.arc. |   |
5297   12M0B com.android.syste |   |
5297   12M0B org.chromium.arc. |   |
5297   26M0B com.google.androi |   |
5297   65M0B system_server |   |


[1] 
https://gitlab.freedesktop.org/virgl/virglrenderer/-/blob/master/src/drm/msm/msm_proto.h#L326
[2] 
https://gitlab.freedesktop.org/virgl/virglrenderer/-/blob/master/src/drm/msm/msm_renderer.c#L1050

> >
> > Signed-off-by: Rob Clark 
> > ---
> >   Documentation/gpu/drm-usage-stats.rst |  8 
> >   drivers/gpu/drm/msm/msm_gpu.c | 14 ++
> >   2 files changed, 22 insertions(+)
> >
> > diff --git a/Documentation/gpu/drm-usage-stats.rst 
> > b/Documentation/gpu/drm-usage-stats.rst
> > index 8e00d53231e0..bc90bed455e3 100644
> > --- a/Documentation/gpu/drm-usage-stats.rst
> > +++ b/Documentation/gpu/drm-usage-stats.rst
> > @@ -148,6 +148,14 @@ percentage utilization of the engine, whereas 
> > drm-engine- only reflects
> >   time active without considering what frequency the engine is operating as 
> > a
> >   percentage of it's maximum frequency.
> >
> > +- drm-comm: 
> > +
> > +Returns the clients executable path.
>
> Full path and not just current->comm? In this case probably give it a
> more descriptive name here.
>
> drm-client-executable
> drm-client-command-line
>
> So we stay in the drm-client- namespace?
>
> Or if the former is absolute path could one key be enough for both?
>
> drm-client-command-line: /path/to/executable --arguments

comm and cmdline can be different. Android seems to change the comm to
the apk name, for example (and w/ the zygote stuff cmdline isn't
really a thing)

I guess it could be drm-client-comm and drm-client-cmdline?  Although
comm/cmdline aren't the best names, they are just following what the
kernel calls them elsewhere.

> > +
> > +- drm-cmdline: 
> > +
> > +Returns the clients cmdline.
>
> I think drm-usage-stats.rst text should provide some more text with
> these two. To precisely define their content and outline the use case
> under which driver authors may want to add them, and fdinfo consumer
> therefore expect to see them. Just so everything is completely clear and
> people do not start adding them for drivers which do not support native
> context (or like).

I really was just piggy-backing on existing comm/cmdline.. but I'll
try to write up something better.

I think it maybe should not be limited just to native context.. for
ex. if the browser did somehow manage to create different displays
associated with different drm_file instances (I guess it would have to
use gbm to do this?) it would be nice to see browser tab names.

> But on the overall it sounds reasonable to me - it would be really cool
> to not just see pcivirtio-gpu as you say. Even if the standard virtiogpu
> use case (not native context) could show real users.

For vrend/virgl, we'd first need to solve the issue that there is just
a single drm_file for all 

Re: [Freedreno] [RFC 2/3] drm/msm: Rework get_comm_cmdline() helper

2023-04-18 Thread Rob Clark
On Tue, Apr 18, 2023 at 1:34 AM Daniel Vetter  wrote:
>
> On Tue, Apr 18, 2023 at 09:27:49AM +0100, Tvrtko Ursulin wrote:
> >
> > On 17/04/2023 21:12, Rob Clark wrote:
> > > From: Rob Clark 
> > >
> > > Make it work in terms of ctx so that it can be re-used for fdinfo.
> > >
> > > Signed-off-by: Rob Clark 
> > > ---
> > >   drivers/gpu/drm/msm/adreno/adreno_gpu.c |  4 ++--
> > >   drivers/gpu/drm/msm/msm_drv.c   |  2 ++
> > >   drivers/gpu/drm/msm/msm_gpu.c   | 13 ++---
> > >   drivers/gpu/drm/msm/msm_gpu.h   | 12 ++--
> > >   drivers/gpu/drm/msm/msm_submitqueue.c   |  1 +
> > >   5 files changed, 21 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> > > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > index bb38e728864d..43c4e1fea83f 100644
> > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > @@ -412,7 +412,7 @@ int adreno_set_param(struct msm_gpu *gpu, struct 
> > > msm_file_private *ctx,
> > > /* Ensure string is null terminated: */
> > > str[len] = '\0';
> > > -   mutex_lock(>lock);
> > > +   mutex_lock(>lock);
> > > if (param == MSM_PARAM_COMM) {
> > > paramp = >comm;
> > > @@ -423,7 +423,7 @@ int adreno_set_param(struct msm_gpu *gpu, struct 
> > > msm_file_private *ctx,
> > > kfree(*paramp);
> > > *paramp = str;
> > > -   mutex_unlock(>lock);
> > > +   mutex_unlock(>lock);
> > > return 0;
> > > }
> > > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > > index 3d73b98d6a9c..ca0e89e46e13 100644
> > > --- a/drivers/gpu/drm/msm/msm_drv.c
> > > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > > @@ -581,6 +581,8 @@ static int context_init(struct drm_device *dev, 
> > > struct drm_file *file)
> > > rwlock_init(>queuelock);
> > > kref_init(>ref);
> > > +   ctx->pid = get_pid(task_pid(current));
> >
> > Would it simplify things for msm if DRM core had an up to date file->pid as
> > proposed in
> > https://patchwork.freedesktop.org/patch/526752/?series=109902=4 ? It
> > gets updated if ioctl issuer is different than fd opener and this being
> > context_init here reminded me of it. Maybe you wouldn't have to track the
> > pid in msm?

The problem is that we also need this for gpu devcore dumps, which
could happen after the drm_file is closed.  The ctx can outlive the
file.

But the ctx->pid has the same problem as the existing file->pid when
it comes to Xorg.. hopefully over time that problem just goes away.  I
guess I could do a similar dance to your patch to update the pid
whenever (for ex) a submitqueue is created.

> Can we go one step further and let the drm fdinfo stuff print these new
> additions? Consistency across drivers and all that.

Hmm, I guess I could _also_ store the overridden comm/cmdline in
drm_file.  I still need to track it in ctx (msm_file_private) because
I could need it after the file is closed.

Maybe it could be useful to have a gl extension to let the app set a
name on the context so that this is useful beyond native-ctx (ie.
maybe it would be nice to see that "chrome: lwn.net" is using less gpu
memory than "chrome: phoronix.com", etc)

BR,
-R

> Also for a generic trigger I think any driver ioctl is good enough (we
> only really need to avoid the auth dance when you're not on a render
> node).
> -Daniel
>
> >
> > Regards,
> >
> > Tvrtko
> >
> > > +   mutex_init(>lock);
> > > msm_submitqueue_init(dev, ctx);
> > > ctx->aspace = msm_gpu_create_private_address_space(priv->gpu, 
> > > current);
> > > diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> > > index c403912d13ab..f0f4f845c32d 100644
> > > --- a/drivers/gpu/drm/msm/msm_gpu.c
> > > +++ b/drivers/gpu/drm/msm/msm_gpu.c
> > > @@ -327,18 +327,17 @@ find_submit(struct msm_ringbuffer *ring, uint32_t 
> > > fence)
> > >   static void retire_submits(struct msm_gpu *gpu);
> > > -static void get_comm_cmdline(struct msm_gem_submit *submit, char **comm, 
> > > char **cmd)
> > > +static void get_comm_cmdline(struct msm_file_private *ctx, char **comm, 
> > > char **cmd)
> > >   {
> > > -   struct msm_file_private *ctx = submit->queue->ctx;
> > > struct task_struct *task;
> > > -   WARN_ON(!mutex_is_locked(>gpu->lock));
> > > -
> > > /* Note that kstrdup will return NULL if argument is NULL: */
> > > +   mutex_lock(>lock);
> > > *comm = kstrdup(ctx->comm, GFP_KERNEL);
> > > *cmd  = kstrdup(ctx->cmdline, GFP_KERNEL);
> > > +   mutex_unlock(>lock);
> > > -   task = get_pid_task(submit->pid, PIDTYPE_PID);
> > > +   task = get_pid_task(ctx->pid, PIDTYPE_PID);
> > > if (!task)
> > > return;
> > > @@ -372,7 +371,7 @@ static void recover_worker(struct kthread_work *work)
> > > if (submit->aspace)
> > > submit->aspace->faults++;
> > > -   

Re: [Freedreno] [PATCH v2 1/5] dt-bindings: display/msm: Add reg bus interconnect

2023-04-18 Thread Krzysztof Kozlowski
On 18/04/2023 14:10, Konrad Dybcio wrote:
> Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
> another path that needs to be handled to ensure MDSS functions properly,
> namely the "reg bus", a.k.a the CPU-MDSS interconnect.
> 
> Gating that path may have a variety of effects.. from none to otherwise
> inexplicable DSI timeouts..
> 
> Describe it in bindings to allow for use in device trees.
> 
> Signed-off-by: Konrad Dybcio 

Acked-by: Krzysztof Kozlowski 

Best regards,
Krzysztof



Re: [Freedreno] [PATCH v2 17/17] drm/msm/dpu: Remove intr_rdptr from DPU >= 5.0.0 pingpong config

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> Now that newer DPU platforms use a readpointer-done interrupt on the
> INTF block, stop providing the unused interrupt on the PINGPONG block.
> 
> Signed-off-by: Marijn Suijten 
> Reviewed-by: Dmitry Baryshkov 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h  |  8 
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h |  8 
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h  |  8 
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h  |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h  |  8 
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h  | 10 +-
>  7 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> index e8d25a45d6b3..a6dbc4c8acb8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> @@ -130,16 +130,16 @@ static const struct dpu_dspp_cfg sm8150_dspp[] = {
>  static const struct dpu_pingpong_cfg sm8150_pp[] = {
>   PP_BLK("pingpong_0", PINGPONG_0, 0x7, PINGPONG_SM8150_TE2_MASK, 
> MERGE_3D_0, sdm845_pp_sblk_te,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> + -1),
>   PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_TE2_MASK, 
> MERGE_3D_0, sdm845_pp_sblk_te,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
> + -1),
>   PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, 
> MERGE_3D_1, sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)),
> + -1),
>   PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, 
> MERGE_3D_1, sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
> + -1),
>   PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, 
> MERGE_3D_2, sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
>   -1),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> index 62857288ad91..14d5ead8d40c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> @@ -118,16 +118,16 @@ static const struct dpu_lm_cfg sc8180x_lm[] = {
>  static const struct dpu_pingpong_cfg sc8180x_pp[] = {
>   PP_BLK("pingpong_0", PINGPONG_0, 0x7, PINGPONG_SM8150_TE2_MASK, 
> MERGE_3D_0, sdm845_pp_sblk_te,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> + -1),
>   PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_TE2_MASK, 
> MERGE_3D_0, sdm845_pp_sblk_te,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
> + -1),
>   PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, 
> MERGE_3D_1, sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)),
> + -1),
>   PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, 
> MERGE_3D_1, sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
> + -1),
>   PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, 
> MERGE_3D_2, sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
>   -1),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> index f77329ab397d..f98ca0f1e4a9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> @@ -131,16 +131,16 @@ static const struct dpu_dspp_cfg sm8250_dspp[] = {
>  static const struct dpu_pingpong_cfg sm8250_pp[] = {
>   PP_BLK("pingpong_0", PINGPONG_0, 0x7, PINGPONG_SM8150_TE2_MASK, 
> MERGE_3D_0, sdm845_pp_sblk_te,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> + -1),
>   PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_TE2_MASK, 
> MERGE_3D_0, sdm845_pp_sblk_te,
>   

Re: [Freedreno] [PATCH v2 14/17] drm/msm/dpu: Document and enable TEAR interrupts on DSI interfaces

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> All SoCs since DPU 5.0.0 have the tear interrupt registers moved out of
> the PINGPONG block and into the INTF block.  Wire up these interrupts
> and IRQ masks on all supported hardware.
> 
> Signed-off-by: Marijn Suijten 
> ---
Acked-by: Konrad Dybcio 

Konrad
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 12 ++
>  .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h| 12 ++
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 12 ++
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h |  8 ---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h |  8 ---
>  .../drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h|  8 ---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 12 ++
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h |  6 +++--
>  .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h   | 12 ++
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 12 ++
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 12 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 15 
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |  6 +++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c  | 27 
> ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h  |  4 
>  15 files changed, 125 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> index e0f62f84b3cf..e8d25a45d6b3 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> @@ -165,12 +165,14 @@ static const struct dpu_intf_cfg sm8150_intf[] = {
>   INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, 0, 24, 
> INTF_SC7180_MASK,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 25)),
> - INTF_BLK("intf_1", INTF_1, 0x6a800, 0x2bc, INTF_DSI, 0, 24, 
> INTF_SC7180_MASK,
> + INTF_BLK_DSI_TE("intf_1", INTF_1, 0x6a800, 0x2bc, INTF_DSI, 0, 24, 
> INTF_SC7180_MASK,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 26),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 27)),
> - INTF_BLK("intf_2", INTF_2, 0x6b000, 0x2bc, INTF_DSI, 1, 24, 
> INTF_SC7180_MASK,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 27),
> + DPU_IRQ_IDX(MDP_INTF1_TEAR_INTR, 2)),
> + INTF_BLK_DSI_TE("intf_2", INTF_2, 0x6b000, 0x2bc, INTF_DSI, 1, 24, 
> INTF_SC7180_MASK,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 28),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 29)),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 29),
> + DPU_IRQ_IDX(MDP_INTF2_TEAR_INTR, 2)),
>   INTF_BLK("intf_3", INTF_3, 0x6b800, 0x280, INTF_DP, 1, 24, 
> INTF_SC7180_MASK,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 30),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 31)),
> @@ -236,7 +238,9 @@ const struct dpu_mdss_cfg dpu_sm8150_cfg = {
>BIT(MDP_SSPP_TOP0_HIST_INTR) | \
>BIT(MDP_INTF0_INTR) | \
>BIT(MDP_INTF1_INTR) | \
> +  BIT(MDP_INTF1_TEAR_INTR) | \
>BIT(MDP_INTF2_INTR) | \
> +  BIT(MDP_INTF2_TEAR_INTR) | \
>BIT(MDP_INTF3_INTR) | \
>BIT(MDP_AD4_0_INTR) | \
>BIT(MDP_AD4_1_INTR),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> index fbcfbbd74875..62857288ad91 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> @@ -146,12 +146,14 @@ static const struct dpu_intf_cfg sc8180x_intf[] = {
>   INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, 
> MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 25)),
> - INTF_BLK("intf_1", INTF_1, 0x6a800, 0x2bc, INTF_DSI, 0, 24, 
> INTF_SC7180_MASK,
> + INTF_BLK_DSI_TE("intf_1", INTF_1, 0x6a800, 0x2bc, INTF_DSI, 0, 24, 
> INTF_SC7180_MASK,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 26),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 27)),
> - INTF_BLK("intf_2", INTF_2, 0x6b000, 0x2bc, INTF_DSI, 1, 24, 
> INTF_SC7180_MASK,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 27),
> + DPU_IRQ_IDX(MDP_INTF1_TEAR_INTR, 2)),
> + INTF_BLK_DSI_TE("intf_2", INTF_2, 0x6b000, 0x2bc, INTF_DSI, 1, 24, 
> INTF_SC7180_MASK,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 28),
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 29)),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 29),
> + 

Re: [Freedreno] [PATCH v2 13/17] drm/msm/dpu: Factor out shared interrupt register in INTF_BLK macro

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> As the INTF block is going to attain more interrupts that don't share
> the same MDP_SSPP_TOP0_INTR register, factor out the _reg argument for
> the caller to construct the right interrupt index (register and bit
> index) to not make the interrupt bit arguments depend on one of multiple
> interrupt register indices.  This brings us more in line with how PP_BLK
> specifies its interrupts and allows for better wrapping in the arrays.
> 
> Signed-off-by: Marijn Suijten 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h| 16 +++---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 16 +++---
>  .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h| 24 +++
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 16 +++---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h |  8 +++--
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h |  6 ++--
>  .../drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h|  6 ++--
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 16 +++---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 12 ++--
>  .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h   | 36 
> --
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 16 +++---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 16 +++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c |  6 ++--
>  14 files changed, 155 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> index 6906f8046b9e..c0dd4776f539 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> @@ -134,10 +134,18 @@ static const struct dpu_dspp_cfg msm8998_dspp[] = {
>  };
>  
>  static const struct dpu_intf_cfg msm8998_intf[] = {
> - INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, 0, 25, 
> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
> - INTF_BLK("intf_1", INTF_1, 0x6a800, 0x280, INTF_DSI, 0, 25, 
> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
> - INTF_BLK("intf_2", INTF_2, 0x6b000, 0x280, INTF_DSI, 1, 25, 
> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29),
> - INTF_BLK("intf_3", INTF_3, 0x6b800, 0x280, INTF_HDMI, 0, 25, 
> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31),
> + INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, 0, 25, 
> INTF_SDM845_MASK,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 25)),
> + INTF_BLK("intf_1", INTF_1, 0x6a800, 0x280, INTF_DSI, 0, 25, 
> INTF_SDM845_MASK,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 26),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 27)),
> + INTF_BLK("intf_2", INTF_2, 0x6b000, 0x280, INTF_DSI, 1, 25, 
> INTF_SDM845_MASK,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 28),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 29)),
> + INTF_BLK("intf_3", INTF_3, 0x6b800, 0x280, INTF_HDMI, 0, 25, 
> INTF_SDM845_MASK,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 30),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 31)),
>  };
>  
>  static const struct dpu_perf_cfg msm8998_perf_data = {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> index 14ce397800d5..b109757b0672 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> @@ -132,10 +132,18 @@ static const struct dpu_dsc_cfg sdm845_dsc[] = {
>  };
>  
>  static const struct dpu_intf_cfg sdm845_intf[] = {
> - INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, 0, 24, 
> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
> - INTF_BLK("intf_1", INTF_1, 0x6a800, 0x280, INTF_DSI, 0, 24, 
> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
> - INTF_BLK("intf_2", INTF_2, 0x6b000, 0x280, INTF_DSI, 1, 24, 
> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 28, 29),
> - INTF_BLK("intf_3", INTF_3, 0x6b800, 0x280, INTF_DP, 1, 24, 
> INTF_SDM845_MASK, MDP_SSPP_TOP0_INTR, 30, 31),
> + INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, 0, 24, 
> INTF_SDM845_MASK,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 25)),
> + INTF_BLK("intf_1", INTF_1, 0x6a800, 0x280, INTF_DSI, 0, 24, 
> INTF_SDM845_MASK,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 26),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 27)),
> + INTF_BLK("intf_2", INTF_2, 0x6b000, 0x280, INTF_DSI, 1, 24, 
> INTF_SDM845_MASK,
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 28),
> + DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 29)),
> + INTF_BLK("intf_3", 

Re: [Freedreno] [PATCH v2 10/17] drm/msm/dpu: Disable pingpong TE on DPU 5.0.0 and above

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> Since hardware revision 5.0.0 the TE configuration moved out of the
> PINGPONG block into the INTF block.  Writing these registers has no
> effect, and is omitted downstream via the DPU/SDE_PINGPONG_TE feature
> flag.  This flag is only added to PINGPONG blocks used by hardware prior
> to 5.0.0.
> 
> The existing PP_BLK_TE macro has been removed in favour of directly
> passing this feature flag, which has thus far been the only difference
> with PP_BLK.  PP_BLK_DITHER has been left in place as its embedded
> feature flag already excludes this DPU_PINGPONG_TE bit and differs by
> setting the block length to zero, as it only contains a DITHER subblock.
> 
> The code that writes to these registers in the INTF block will follow in
> subsequent patches.
> 
> Signed-off-by: Marijn Suijten 
> ---
I believe everything here is correct, even though there's
quite a bunch of stuff involved:

Reviewed-by: Konrad Dybcio 

Konrad
>  .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h|  8 +++
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h |  8 +++
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 12 +--
>  .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h| 12 +--
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 12 +--
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h |  4 ++--
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_3_sm6115.h |  2 +-
>  .../drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h|  2 +-
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 12 +--
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h |  8 +++
>  .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h   | 24 ++---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 16 +++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 25 
> ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c| 12 ++-
>  14 files changed, 78 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> index 2b3ae84057df..b7845591c384 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> @@ -112,16 +112,16 @@ static const struct dpu_lm_cfg msm8998_lm[] = {
>  };
>  
>  static const struct dpu_pingpong_cfg msm8998_pp[] = {
> - PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, 0, sdm845_pp_sblk_te,
> + PP_BLK("pingpong_0", PINGPONG_0, 0x7, PINGPONG_SDM845_TE2_MASK, 0, 
> sdm845_pp_sblk_te,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> - PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, 0, sdm845_pp_sblk_te,
> + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, 
> sdm845_pp_sblk_te,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
> - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, 0, sdm845_pp_sblk,
> + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, 
> sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)),
> - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, 0, sdm845_pp_sblk,
> + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, 
> sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
>  };
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> index ceca741e93c9..5b9b3b99f1b5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> @@ -110,16 +110,16 @@ static const struct dpu_lm_cfg sdm845_lm[] = {
>  };
>  
>  static const struct dpu_pingpong_cfg sdm845_pp[] = {
> - PP_BLK_TE("pingpong_0", PINGPONG_0, 0x7, 0, sdm845_pp_sblk_te,
> + PP_BLK("pingpong_0", PINGPONG_0, 0x7, PINGPONG_SDM845_TE2_MASK, 0, 
> sdm845_pp_sblk_te,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> - PP_BLK_TE("pingpong_1", PINGPONG_1, 0x70800, 0, sdm845_pp_sblk_te,
> + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, 
> sdm845_pp_sblk_te,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
> - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, 0, sdm845_pp_sblk,
> + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, 
> sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)),
> - PP_BLK("pingpong_3", PINGPONG_3, 

Re: [Freedreno] [PATCH v2 08/17] drm/msm/dpu: Drop unused poll_timeout_wr_ptr PINGPONG callback

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> This callback was migrated from downstream when DPU1 was first
> introduced to mainline, but never used by any component.  Drop it to
> save some lines and unnecessary confusion.
> 
> Suggested-by: Dmitry Baryshkov 
> Signed-off-by: Marijn Suijten 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 18 --
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h |  6 --
>  2 files changed, 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> index 0fcad9760b6f..b18efd640abd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> @@ -144,23 +144,6 @@ static bool dpu_hw_pp_get_autorefresh_config(struct 
> dpu_hw_pingpong *pp,
>   return !!((val & BIT(31)) >> 31);
>  }
>  
> -static int dpu_hw_pp_poll_timeout_wr_ptr(struct dpu_hw_pingpong *pp,
> - u32 timeout_us)
> -{
> - struct dpu_hw_blk_reg_map *c;
> - u32 val;
> - int rc;
> -
> - if (!pp)
> - return -EINVAL;
> -
> - c = >hw;
> - rc = readl_poll_timeout(c->blk_addr + PP_LINE_COUNT,
> - val, (val & 0x) >= 1, 10, timeout_us);
> -
> - return rc;
> -}
> -
>  static int dpu_hw_pp_enable_te(struct dpu_hw_pingpong *pp, bool enable)
>  {
>   struct dpu_hw_blk_reg_map *c;
> @@ -280,7 +263,6 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
>   c->ops.get_vsync_info = dpu_hw_pp_get_vsync_info;
>   c->ops.setup_autorefresh = dpu_hw_pp_setup_autorefresh_config;
>   c->ops.get_autorefresh = dpu_hw_pp_get_autorefresh_config;
> - c->ops.poll_timeout_wr_ptr = dpu_hw_pp_poll_timeout_wr_ptr;
>   c->ops.get_line_count = dpu_hw_pp_get_line_count;
>   c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
>   c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
> index c00223441d99..cf94b4ab603b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.h
> @@ -107,12 +107,6 @@ struct dpu_hw_pingpong_ops {
>   bool (*get_autorefresh)(struct dpu_hw_pingpong *pp,
>   u32 *frame_count);
>  
> - /**
> -  * poll until write pointer transmission starts
> -  * @Return: 0 on success, -ETIMEDOUT on timeout
> -  */
> - int (*poll_timeout_wr_ptr)(struct dpu_hw_pingpong *pp, u32 timeout_us);
> -
>   /**
>* Obtain current vertical line counter
>*/
> 


Re: [Freedreno] [PATCH v2 07/17] drm/msm/dpu: Sort INTF registers numerically

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> A bunch of registers were appended at the end in e.g. 91143873a05d
> ("drm/msm/dpu: Add MISR register support for interface") rather than
> being inserted in a place that maintains numerical sorting.  Restore
> that.
> 
> Signed-off-by: Marijn Suijten 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index 1d22d7dc99b8..1491568f86fc 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -36,6 +36,10 @@
>  #define INTF_CONFIG20x060
>  #define INTF_DISPLAY_DATA_HCTL  0x064
>  #define INTF_ACTIVE_DATA_HCTL   0x068
> +
> +#define INTF_DSI_CMD_MODE_TRIGGER_EN0x084
> +#define INTF_PANEL_FORMAT   0x090
> +
>  #define INTF_FRAME_LINE_COUNT_EN0x0A8
>  #define INTF_FRAME_COUNT0x0AC
>  #define INTF_LINE_COUNT 0x0B0
> @@ -44,8 +48,6 @@
>  #define INTF_DEFLICKER_STRNG_COEFF  0x0F4
>  #define INTF_DEFLICKER_WEAK_COEFF   0x0F8
>  
> -#define INTF_DSI_CMD_MODE_TRIGGER_EN0x084
> -#define INTF_PANEL_FORMAT   0x090
>  #define INTF_TPG_ENABLE 0x100
>  #define INTF_TPG_MAIN_CONTROL   0x104
>  #define INTF_TPG_VIDEO_CONFIG   0x108
> @@ -57,6 +59,9 @@
>  #define INTF_PROG_FETCH_START   0x170
>  #define INTF_PROG_ROT_START 0x174
>  
> +#define INTF_MISR_CTRL  0x180
> +#define INTF_MISR_SIGNATURE 0x184
> +
>  #define INTF_MUX0x25C
>  #define INTF_STATUS 0x26C
>  
> @@ -66,9 +71,6 @@
>  #define INTF_CFG2_DATABUS_WIDEN  BIT(0)
>  #define INTF_CFG2_DATA_HCTL_EN   BIT(4)
>  
> -#define INTF_MISR_CTRL   0x180
> -#define INTF_MISR_SIGNATURE  0x184
> -
>  static const struct dpu_intf_cfg *_intf_offset(enum dpu_intf intf,
>   const struct dpu_mdss_cfg *m,
>   void __iomem *addr,
> 


Re: [Freedreno] [PATCH v2 06/17] drm/msm/dpu: Remove extraneous register define indentation

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> A bunch of registers are indented with two extra spaces, looking as if
> these are values corresponding to the previous register which is not the
> case, rather these are simply also register offsets and should only have
> a single space separating them and the #define keyword.
> 
> Signed-off-by: Marijn Suijten 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 41 
> +++--
>  1 file changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index b9dddf576c02..1d22d7dc99b8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -38,26 +38,27 @@
>  #define INTF_ACTIVE_DATA_HCTL   0x068
>  #define INTF_FRAME_LINE_COUNT_EN0x0A8
>  #define INTF_FRAME_COUNT0x0AC
> -#define   INTF_LINE_COUNT   0x0B0
> -
> -#define   INTF_DEFLICKER_CONFIG 0x0F0
> -#define   INTF_DEFLICKER_STRNG_COEFF0x0F4
> -#define   INTF_DEFLICKER_WEAK_COEFF 0x0F8
> -
> -#define   INTF_DSI_CMD_MODE_TRIGGER_EN  0x084
> -#define   INTF_PANEL_FORMAT 0x090
> -#define   INTF_TPG_ENABLE   0x100
> -#define   INTF_TPG_MAIN_CONTROL 0x104
> -#define   INTF_TPG_VIDEO_CONFIG 0x108
> -#define   INTF_TPG_COMPONENT_LIMITS 0x10C
> -#define   INTF_TPG_RECTANGLE0x110
> -#define   INTF_TPG_INITIAL_VALUE0x114
> -#define   INTF_TPG_BLK_WHITE_PATTERN_FRAMES   0x118
> -#define   INTF_TPG_RGB_MAPPING  0x11C
> -#define   INTF_PROG_FETCH_START 0x170
> -#define   INTF_PROG_ROT_START   0x174
> -#define   INTF_MUX  0x25C
> -#define   INTF_STATUS   0x26C
> +#define INTF_LINE_COUNT 0x0B0
> +
> +#define INTF_DEFLICKER_CONFIG   0x0F0
> +#define INTF_DEFLICKER_STRNG_COEFF  0x0F4
> +#define INTF_DEFLICKER_WEAK_COEFF   0x0F8
> +
> +#define INTF_DSI_CMD_MODE_TRIGGER_EN0x084
> +#define INTF_PANEL_FORMAT   0x090
> +#define INTF_TPG_ENABLE 0x100
> +#define INTF_TPG_MAIN_CONTROL   0x104
> +#define INTF_TPG_VIDEO_CONFIG   0x108
> +#define INTF_TPG_COMPONENT_LIMITS   0x10C
> +#define INTF_TPG_RECTANGLE  0x110
> +#define INTF_TPG_INITIAL_VALUE  0x114
> +#define INTF_TPG_BLK_WHITE_PATTERN_FRAMES 0x118
> +#define INTF_TPG_RGB_MAPPING0x11C
> +#define INTF_PROG_FETCH_START   0x170
> +#define INTF_PROG_ROT_START 0x174
> +
> +#define INTF_MUX0x25C
> +#define INTF_STATUS 0x26C
>  
>  #define INTF_CFG_ACTIVE_H_EN BIT(29)
>  #define INTF_CFG_ACTIVE_V_EN BIT(30)
> 


Re: [Freedreno] [PATCH v2 05/17] drm/msm/dpu: Remove duplicate register defines from INTF

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> The INTF_FRAME_LINE_COUNT_EN, INTF_FRAME_COUNT and INTF_LINE_COUNT
> registers are already defined higher up, in the right place when sorted
> numerically.
> 
> Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
> Signed-off-by: Marijn Suijten 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index 84ee2efa9c66..b9dddf576c02 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -56,11 +56,6 @@
>  #define   INTF_TPG_RGB_MAPPING  0x11C
>  #define   INTF_PROG_FETCH_START 0x170
>  #define   INTF_PROG_ROT_START   0x174
> -
> -#define   INTF_FRAME_LINE_COUNT_EN  0x0A8
> -#define   INTF_FRAME_COUNT  0x0AC
> -#define   INTF_LINE_COUNT   0x0B0
> -
>  #define   INTF_MUX  0x25C
>  #define   INTF_STATUS   0x26C
>  
> 


Re: [Freedreno] [PATCH v2 04/17] drm/msm/dpu: Fix PP_BLK_DIPHER -> DITHER typo

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> SM8550 only comes with a DITHER subblock inside the PINGPONG block,
> hence the name and a block length of zero.  However, the PP_BLK macro
> name was typo'd to DIPHER rather than DITHER.
> 
> Fixes: efcd0107727c ("drm/msm/dpu: add support for SM8550")
> Signed-off-by: Marijn Suijten 
> ---
lol

Reviewed-by: Konrad Dybcio 

Konrad
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 16 
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c |  2 +-
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> index 9e403034093f..d0ab351b6a8b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> @@ -132,28 +132,28 @@ static const struct dpu_dspp_cfg sm8550_dspp[] = {
>_dspp_sblk),
>  };
>  static const struct dpu_pingpong_cfg sm8550_pp[] = {
> - PP_BLK_DIPHER("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, 
> sc7280_pp_sblk,
> + PP_BLK_DITHER("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, 
> sc7280_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
>   -1),
> - PP_BLK_DIPHER("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, 
> sc7280_pp_sblk,
> + PP_BLK_DITHER("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, 
> sc7280_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
>   -1),
> - PP_BLK_DIPHER("pingpong_2", PINGPONG_2, 0x6b000, MERGE_3D_1, 
> sc7280_pp_sblk,
> + PP_BLK_DITHER("pingpong_2", PINGPONG_2, 0x6b000, MERGE_3D_1, 
> sc7280_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
>   -1),
> - PP_BLK_DIPHER("pingpong_3", PINGPONG_3, 0x6c000, MERGE_3D_1, 
> sc7280_pp_sblk,
> + PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, MERGE_3D_1, 
> sc7280_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
>   -1),
> - PP_BLK_DIPHER("pingpong_4", PINGPONG_4, 0x6d000, MERGE_3D_2, 
> sc7280_pp_sblk,
> + PP_BLK_DITHER("pingpong_4", PINGPONG_4, 0x6d000, MERGE_3D_2, 
> sc7280_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
>   -1),
> - PP_BLK_DIPHER("pingpong_5", PINGPONG_5, 0x6e000, MERGE_3D_2, 
> sc7280_pp_sblk,
> + PP_BLK_DITHER("pingpong_5", PINGPONG_5, 0x6e000, MERGE_3D_2, 
> sc7280_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
>   -1),
> - PP_BLK_DIPHER("pingpong_6", PINGPONG_6, 0x66000, MERGE_3D_3, 
> sc7280_pp_sblk,
> + PP_BLK_DITHER("pingpong_6", PINGPONG_6, 0x66000, MERGE_3D_3, 
> sc7280_pp_sblk,
>   -1,
>   -1),
> - PP_BLK_DIPHER("pingpong_7", PINGPONG_7, 0x66400, MERGE_3D_3, 
> sc7280_pp_sblk,
> + PP_BLK_DITHER("pingpong_7", PINGPONG_7, 0x66400, MERGE_3D_3, 
> sc7280_pp_sblk,
>   -1,
>   -1),
>  };
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index 03f162af1a50..ca8a02debda9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -491,7 +491,7 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk 
> = {
>   .len = 0x20, .version = 0x2},
>  };
>  
> -#define PP_BLK_DIPHER(_name, _id, _base, _merge_3d, _sblk, _done, _rdptr) \
> +#define PP_BLK_DITHER(_name, _id, _base, _merge_3d, _sblk, _done, _rdptr) \
>   {\
>   .name = _name, .id = _id, \
>   .base = _base, .len = 0, \
> 


Re: [Freedreno] [PATCH v2 03/17] drm/msm/dpu: Move non-MDP_TOP INTF_INTR offsets out of hwio header

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> These offsets do not fall under the MDP TOP block and do not fit the
> comment right above.  Move them to dpu_hw_interrupts.c next to the
> repsective MDP_INTF_x_OFF interrupt block offsets.
> 
> Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
> Signed-off-by: Marijn Suijten 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 5 -
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h  | 3 ---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> index 53326f25e40e..85c0bda3ff90 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> @@ -15,7 +15,7 @@
>  
>  /*
>   * Register offsets in MDSS register file for the interrupt registers
> - * w.r.t. to the MDP base
> + * w.r.t. the MDP base
>   */
>  #define MDP_SSPP_TOP0_OFF0x0
>  #define MDP_INTF_0_OFF   0x6A000
> @@ -24,6 +24,9 @@
>  #define MDP_INTF_3_OFF   0x6B800
>  #define MDP_INTF_4_OFF   0x6C000
>  #define MDP_INTF_5_OFF   0x6C800
> +#define INTF_INTR_EN 0x1c0
> +#define INTF_INTR_STATUS 0x1c4
> +#define INTF_INTR_CLEAR  0x1c8
>  #define MDP_AD4_0_OFF0x7C000
>  #define MDP_AD4_1_OFF0x7D000
>  #define MDP_AD4_INTR_EN_OFF  0x41c
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
> index feb9a729844a..5acd5683d25a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
> @@ -21,9 +21,6 @@
>  #define HIST_INTR_EN0x01c
>  #define HIST_INTR_STATUS0x020
>  #define HIST_INTR_CLEAR 0x024
> -#define INTF_INTR_EN0x1C0
> -#define INTF_INTR_STATUS0x1C4
> -#define INTF_INTR_CLEAR 0x1C8
>  #define SPLIT_DISPLAY_EN0x2F4
>  #define SPLIT_DISPLAY_UPPER_PIPE_CTRL   0x2F8
>  #define DSPP_IGC_COLOR0_RAM_LUTN0x300
> 


Re: [Freedreno] [PATCH v2 02/17] drm/msm/dpu: Remove TE2 block and feature from DPU >= 7.0.0 hardware

2023-04-18 Thread Konrad Dybcio



On 17.04.2023 22:21, Marijn Suijten wrote:
> No hardware beyond kona (sm8250) defines the TE2 PINGPONG sub-block
> offset downstream.  Even though neither downstream nor upstream utilizes
> these registers in any way, remove the erroneous specification for
> SC8280XP, SM8350 and SM8450 to prevent confusion.
> 
> Note that downstream enables the PPSPLIT (split-FIFO) topology (single
> LM for 2 PP and 2 INTF) based on the presence of a TE2 block.
> 
> Fixes: f0a1bdf64dd7 ("drm/msm/dpu: Introduce SC8280XP")
> Fixes: 0a72f23f6ef8 ("drm/msm/dpu: Add SM8350 to hw catalog")
> Fixes: 8cbbc3396065 ("drm/msm/dpu: add support for SM8450")
> Signed-off-by: Marijn Suijten 
> ---
Reviewed-by: Konrad Dybcio 

Konrad
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h   |  4 ++--
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 12 ++--
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h   |  4 ++--
>  3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
> index ca107ca8de46..41ef0c8fc993 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
> @@ -127,10 +127,10 @@ static const struct dpu_dspp_cfg sm8350_dspp[] = {
>  };
>  
>  static const struct dpu_pingpong_cfg sm8350_pp[] = {
> - PP_BLK_TE("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, 
> sdm845_pp_sblk_te,
> + PP_BLK("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> - PP_BLK_TE("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, 
> sdm845_pp_sblk_te,
> + PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
>   PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, MERGE_3D_1, sdm845_pp_sblk,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
> index 9aab110b8c44..12c14d15e386 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
> @@ -121,17 +121,17 @@ static const struct dpu_dspp_cfg sc8280xp_dspp[] = {
>  };
>  
>  static const struct dpu_pingpong_cfg sc8280xp_pp[] = {
> - PP_BLK_TE("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, 
> sdm845_pp_sblk_te,
> + PP_BLK("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, sdm845_pp_sblk,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8), -1),
> - PP_BLK_TE("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, 
> sdm845_pp_sblk_te,
> + PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, sdm845_pp_sblk,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9), -1),
> - PP_BLK_TE("pingpong_2", PINGPONG_2, 0x6b000, MERGE_3D_1, 
> sdm845_pp_sblk_te,
> + PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, MERGE_3D_1, sdm845_pp_sblk,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10), -1),
> - PP_BLK_TE("pingpong_3", PINGPONG_3, 0x6c000, MERGE_3D_1, 
> sdm845_pp_sblk_te,
> + PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, MERGE_3D_1, sdm845_pp_sblk,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11), -1),
> - PP_BLK_TE("pingpong_4", PINGPONG_4, 0x6d000, MERGE_3D_2, 
> sdm845_pp_sblk_te,
> + PP_BLK("pingpong_4", PINGPONG_4, 0x6d000, MERGE_3D_2, sdm845_pp_sblk,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30), -1),
> - PP_BLK_TE("pingpong_5", PINGPONG_5, 0x6e000, MERGE_3D_2, 
> sdm845_pp_sblk_te,
> + PP_BLK("pingpong_5", PINGPONG_5, 0x6e000, MERGE_3D_2, sdm845_pp_sblk,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31), -1),
>  };
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h 
> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
> index 02a259b6b426..e409c119b0a2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
> @@ -128,10 +128,10 @@ static const struct dpu_dspp_cfg sm8450_dspp[] = {
>  };
>  /* FIXME: interrupts */
>  static const struct dpu_pingpong_cfg sm8450_pp[] = {
> - PP_BLK_TE("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, 
> sdm845_pp_sblk_te,
> + PP_BLK("pingpong_0", PINGPONG_0, 0x69000, MERGE_3D_0, sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> - PP_BLK_TE("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, 
> sdm845_pp_sblk_te,
> + PP_BLK("pingpong_1", PINGPONG_1, 0x6a000, MERGE_3D_0, sdm845_pp_sblk,
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
>   DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
>   PP_BLK("pingpong_2", PINGPONG_2, 0x6b000, MERGE_3D_1, 

[Freedreno] [PATCH v2 4/5] drm/msm/mdss: Handle the reg bus ICC path

2023-04-18 Thread Konrad Dybcio
Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
another path that needs to be handled to ensure MDSS functions properly,
namely the "reg bus", a.k.a the CPU-MDSS interconnect.

Gating that path may have a variety of effects.. from none to otherwise
inexplicable DSI timeouts..

On the MDSS side, we only have to ensure that it's on at what Qualcomm
downstream calls "77 MHz", a.k.a 76.8 Mbps and turn it off at suspend.

To achieve that, make msm_mdss_icc_request_bw() accept a boolean to
indicate whether we want the busses to be on or off, as this function's
only use is to vote for minimum or no bandwidth at all.

Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/msm_mdss.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 9e2ce7f22677..4d126d20d661 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -50,6 +50,7 @@ struct msm_mdss {
const struct msm_mdss_data *mdss_data;
struct icc_path *mdp_path[2];
u32 num_mdp_paths;
+   struct icc_path *reg_bus_path;
 };
 
 static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
@@ -57,6 +58,7 @@ static int msm_mdss_parse_data_bus_icc_path(struct device 
*dev,
 {
struct icc_path *path0;
struct icc_path *path1;
+   struct icc_path *reg_bus_path;
 
path0 = of_icc_get(dev, "mdp0-mem");
if (IS_ERR_OR_NULL(path0))
@@ -71,6 +73,10 @@ static int msm_mdss_parse_data_bus_icc_path(struct device 
*dev,
msm_mdss->num_mdp_paths++;
}
 
+   reg_bus_path = of_icc_get(dev, "cpu-cfg");
+   if (!IS_ERR_OR_NULL(reg_bus_path))
+   msm_mdss->reg_bus_path = reg_bus_path;
+
return 0;
 }
 
@@ -83,12 +89,15 @@ static void msm_mdss_put_icc_path(void *data)
icc_put(msm_mdss->mdp_path[i]);
 }
 
-static void msm_mdss_icc_request_bw(struct msm_mdss *msm_mdss, unsigned long 
bw)
+static void msm_mdss_icc_request_bw(struct msm_mdss *msm_mdss, bool enable)
 {
int i;
 
for (i = 0; i < msm_mdss->num_mdp_paths; i++)
-   icc_set_bw(msm_mdss->mdp_path[i], 0, Bps_to_icc(bw));
+   icc_set_bw(msm_mdss->mdp_path[i], 0, enable ? 
Bps_to_icc(MIN_IB_BW) : 0);
+
+   if (msm_mdss->reg_bus_path)
+   icc_set_bw(msm_mdss->reg_bus_path, 0, enable ? 76800 : 0);
 }
 
 static void msm_mdss_irq(struct irq_desc *desc)
@@ -241,7 +250,7 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss)
 * the interconnect is enabled (non-zero bandwidth). Let's make sure
 * that the interconnects are at least at a minimum amount.
 */
-   msm_mdss_icc_request_bw(msm_mdss, MIN_IB_BW);
+   msm_mdss_icc_request_bw(msm_mdss, true);
 
ret = clk_bulk_prepare_enable(msm_mdss->num_clocks, msm_mdss->clocks);
if (ret) {
@@ -289,7 +298,7 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss)
 static int msm_mdss_disable(struct msm_mdss *msm_mdss)
 {
clk_bulk_disable_unprepare(msm_mdss->num_clocks, msm_mdss->clocks);
-   msm_mdss_icc_request_bw(msm_mdss, 0);
+   msm_mdss_icc_request_bw(msm_mdss, false);
 
return 0;
 }

-- 
2.40.0



[Freedreno] [PATCH v2 5/5] drm/msm/dpu1: Handle the reg bus ICC path

2023-04-18 Thread Konrad Dybcio
Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
another path that needs to be handled to ensure MDSS functions properly,
namely the "reg bus", a.k.a the CPU-MDSS interconnect.

Gating that path may have a variety of effects.. from none to otherwise
inexplicable DSI timeouts..

On the DPU side, we need to keep the bus alive. The vendor driver
kickstarts it to max (300Mbps) throughput on first commit, but in
exchange for some battery life in rare DPU-enabled-panel-disabled
usecases, we can request it at DPU init and gate it at suspend.

Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 22 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 +
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index dd6c1c40ab9e..5e1ed338114d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -384,15 +384,17 @@ static int dpu_kms_global_obj_init(struct dpu_kms 
*dpu_kms)
return 0;
 }
 
-static int dpu_kms_parse_data_bus_icc_path(struct dpu_kms *dpu_kms)
+static int dpu_kms_parse_icc_paths(struct dpu_kms *dpu_kms)
 {
struct icc_path *path0;
struct icc_path *path1;
+   struct icc_path *reg_bus_path;
struct drm_device *dev = dpu_kms->dev;
struct device *dpu_dev = dev->dev;
 
path0 = msm_icc_get(dpu_dev, "mdp0-mem");
path1 = msm_icc_get(dpu_dev, "mdp1-mem");
+   reg_bus_path = msm_icc_get(dpu_dev, "cpu-cfg");
 
if (IS_ERR_OR_NULL(path0))
return PTR_ERR_OR_ZERO(path0);
@@ -404,6 +406,10 @@ static int dpu_kms_parse_data_bus_icc_path(struct dpu_kms 
*dpu_kms)
dpu_kms->mdp_path[1] = path1;
dpu_kms->num_mdp_paths++;
}
+
+   if (!IS_ERR_OR_NULL(reg_bus_path))
+   dpu_kms->reg_bus_path = reg_bus_path;
+
return 0;
 }
 
@@ -1039,7 +1045,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
DPU_DEBUG("REG_DMA is not defined");
}
 
-   dpu_kms_parse_data_bus_icc_path(dpu_kms);
+   dpu_kms_parse_icc_paths(dpu_kms);
 
rc = pm_runtime_resume_and_get(_kms->pdev->dev);
if (rc < 0)
@@ -1241,6 +1247,9 @@ static int __maybe_unused dpu_runtime_suspend(struct 
device *dev)
for (i = 0; i < dpu_kms->num_mdp_paths; i++)
icc_set_bw(dpu_kms->mdp_path[i], 0, 0);
 
+   if (dpu_kms->reg_bus_path)
+   icc_set_bw(dpu_kms->reg_bus_path, 0, 0);
+
return 0;
 }
 
@@ -1261,6 +1270,15 @@ static int __maybe_unused dpu_runtime_resume(struct 
device *dev)
return rc;
}
 
+   /*
+* The vendor driver supports setting 76.8 / 150 / 300 MBps on this
+* path, but it seems to go for the highest level when display output
+* is enabled and zero otherwise. For simplicity, we can assume that
+* DPU being enabled and running implies that.
+*/
+   if (dpu_kms->reg_bus_path)
+   icc_set_bw(dpu_kms->reg_bus_path, 0, MBps_to_icc(300));
+
dpu_vbif_init_memtypes(dpu_kms);
 
drm_for_each_encoder(encoder, ddev)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index d5d9bec90705..c332381d58c4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -111,6 +111,7 @@ struct dpu_kms {
atomic_t bandwidth_ref;
struct icc_path *mdp_path[2];
u32 num_mdp_paths;
+   struct icc_path *reg_bus_path;
 };
 
 struct vsync_info {

-- 
2.40.0



[Freedreno] [PATCH v2 3/5] drm/msm/mdss: Rename path references to mdp_path

2023-04-18 Thread Konrad Dybcio
The DPU1 driver needs to handle all MDPn<->DDR paths, as well as
CPU<->SLAVE_DISPLAY_CFG. The former ones share how their values are
calculated, but the latter one has static predefines spanning all SoCs.

In preparation for supporting the CPU<->SLAVE_DISPLAY_CFG path, rename
the path-related struct members to include "mdp_".

Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/msm_mdss.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index e8c93731aaa1..9e2ce7f22677 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -48,8 +48,8 @@ struct msm_mdss {
struct irq_domain *domain;
} irq_controller;
const struct msm_mdss_data *mdss_data;
-   struct icc_path *path[2];
-   u32 num_paths;
+   struct icc_path *mdp_path[2];
+   u32 num_mdp_paths;
 };
 
 static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
@@ -62,13 +62,13 @@ static int msm_mdss_parse_data_bus_icc_path(struct device 
*dev,
if (IS_ERR_OR_NULL(path0))
return PTR_ERR_OR_ZERO(path0);
 
-   msm_mdss->path[0] = path0;
-   msm_mdss->num_paths = 1;
+   msm_mdss->mdp_path[0] = path0;
+   msm_mdss->num_mdp_paths = 1;
 
path1 = of_icc_get(dev, "mdp1-mem");
if (!IS_ERR_OR_NULL(path1)) {
-   msm_mdss->path[1] = path1;
-   msm_mdss->num_paths++;
+   msm_mdss->mdp_path[1] = path1;
+   msm_mdss->num_mdp_paths++;
}
 
return 0;
@@ -79,16 +79,16 @@ static void msm_mdss_put_icc_path(void *data)
struct msm_mdss *msm_mdss = data;
int i;
 
-   for (i = 0; i < msm_mdss->num_paths; i++)
-   icc_put(msm_mdss->path[i]);
+   for (i = 0; i < msm_mdss->num_mdp_paths; i++)
+   icc_put(msm_mdss->mdp_path[i]);
 }
 
 static void msm_mdss_icc_request_bw(struct msm_mdss *msm_mdss, unsigned long 
bw)
 {
int i;
 
-   for (i = 0; i < msm_mdss->num_paths; i++)
-   icc_set_bw(msm_mdss->path[i], 0, Bps_to_icc(bw));
+   for (i = 0; i < msm_mdss->num_mdp_paths; i++)
+   icc_set_bw(msm_mdss->mdp_path[i], 0, Bps_to_icc(bw));
 }
 
 static void msm_mdss_irq(struct irq_desc *desc)

-- 
2.40.0



[Freedreno] [PATCH v2 1/5] dt-bindings: display/msm: Add reg bus interconnect

2023-04-18 Thread Konrad Dybcio
Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
another path that needs to be handled to ensure MDSS functions properly,
namely the "reg bus", a.k.a the CPU-MDSS interconnect.

Gating that path may have a variety of effects.. from none to otherwise
inexplicable DSI timeouts..

Describe it in bindings to allow for use in device trees.

Signed-off-by: Konrad Dybcio 
---
 Documentation/devicetree/bindings/display/msm/mdss-common.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml 
b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
index ccd7d6417523..30a8aed4289a 100644
--- a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
+++ b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
@@ -66,12 +66,14 @@ properties:
 items:
   - description: Interconnect path from mdp0 (or a single mdp) port to the 
data bus
   - description: Interconnect path from mdp1 port to the data bus
+  - description: Interconnect path from CPU to the reg bus
 
   interconnect-names:
 minItems: 1
 items:
   - const: mdp0-mem
   - const: mdp1-mem
+  - const: cpu-cfg
 
   resets:
 items:

-- 
2.40.0



[Freedreno] [PATCH v2 2/5] drm/msm/dpu1: Rename path references to mdp_path

2023-04-18 Thread Konrad Dybcio
The DPU1 driver needs to handle all MDPn<->DDR paths, as well as
CPU<->SLAVE_DISPLAY_CFG. The former ones share how their values are
calculated, but the latter one has static predefines spanning all SoCs.

In preparation for supporting the CPU<->SLAVE_DISPLAY_CFG path, rename
the path-related struct members to include "mdp_".

Signed-off-by: Konrad Dybcio 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 10 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   | 12 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h   |  4 ++--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 1d9d83d7b99e..349c6cb3301d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -230,18 +230,18 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms 
*kms,
 
DRM_DEBUG_ATOMIC("crtc=%d bw=%llu paths:%d\n",
  tmp_crtc->base.id,
- dpu_cstate->new_perf.bw_ctl, kms->num_paths);
+ dpu_cstate->new_perf.bw_ctl, 
kms->num_mdp_paths);
}
}
 
-   if (!kms->num_paths)
+   if (!kms->num_mdp_paths)
return 0;
 
avg_bw = perf.bw_ctl;
-   do_div(avg_bw, (kms->num_paths * 1000)); /*Bps_to_icc*/
+   do_div(avg_bw, (kms->num_mdp_paths * 1000)); /*Bps_to_icc*/
 
-   for (i = 0; i < kms->num_paths; i++)
-   icc_set_bw(kms->path[i], avg_bw, perf.max_per_pipe_ib);
+   for (i = 0; i < kms->num_mdp_paths; i++)
+   icc_set_bw(kms->mdp_path[i], avg_bw, perf.max_per_pipe_ib);
 
return ret;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 0e7a68714e9e..dd6c1c40ab9e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -397,12 +397,12 @@ static int dpu_kms_parse_data_bus_icc_path(struct dpu_kms 
*dpu_kms)
if (IS_ERR_OR_NULL(path0))
return PTR_ERR_OR_ZERO(path0);
 
-   dpu_kms->path[0] = path0;
-   dpu_kms->num_paths = 1;
+   dpu_kms->mdp_path[0] = path0;
+   dpu_kms->num_mdp_paths = 1;
 
if (!IS_ERR_OR_NULL(path1)) {
-   dpu_kms->path[1] = path1;
-   dpu_kms->num_paths++;
+   dpu_kms->mdp_path[1] = path1;
+   dpu_kms->num_mdp_paths++;
}
return 0;
 }
@@ -1238,8 +1238,8 @@ static int __maybe_unused dpu_runtime_suspend(struct 
device *dev)
dev_pm_opp_set_rate(dev, 0);
clk_bulk_disable_unprepare(dpu_kms->num_clocks, dpu_kms->clocks);
 
-   for (i = 0; i < dpu_kms->num_paths; i++)
-   icc_set_bw(dpu_kms->path[i], 0, 0);
+   for (i = 0; i < dpu_kms->num_mdp_paths; i++)
+   icc_set_bw(dpu_kms->mdp_path[i], 0, 0);
 
return 0;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index aca39a4689f4..d5d9bec90705 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -109,8 +109,8 @@ struct dpu_kms {
 * when disabled.
 */
atomic_t bandwidth_ref;
-   struct icc_path *path[2];
-   u32 num_paths;
+   struct icc_path *mdp_path[2];
+   u32 num_mdp_paths;
 };
 
 struct vsync_info {

-- 
2.40.0



[Freedreno] [PATCH v2 0/5] MDSS reg bus interconnect

2023-04-18 Thread Konrad Dybcio
v1 -> v2:
- Fix "Mbps" -> "MBps" [5/5]
- Add an interconnects: entry in dt-bindings (and not only -names..) [1/5]

v1: 
https://lore.kernel.org/r/20230417-topic-dpu_regbus-v1-0-06fbdc164...@linaro.org

Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
another path that needs to be handled to ensure MDSS functions properly,
namely the "reg bus", a.k.a the CPU-MDSS interconnect.

Gating that path may have a variety of effects.. from none to otherwise
inexplicable DSI timeouts..

This series tries to address the lack of that.

Example path:

interconnects = < MASTER_AMPSS_M0 0 _noc SLAVE_DISPLAY_CFG 0>;

Signed-off-by: Konrad Dybcio 
---
Konrad Dybcio (5):
  dt-bindings: display/msm: Add reg bus interconnect
  drm/msm/dpu1: Rename path references to mdp_path
  drm/msm/mdss: Rename path references to mdp_path
  drm/msm/mdss: Handle the reg bus ICC path
  drm/msm/dpu1: Handle the reg bus ICC path

 .../bindings/display/msm/mdss-common.yaml  |  2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c  | 10 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 34 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h|  5 ++--
 drivers/gpu/drm/msm/msm_mdss.c | 35 ++
 5 files changed, 58 insertions(+), 28 deletions(-)
---
base-commit: 4aa1da8d99724f6c0b762b58a71cee7c5e2e109b
change-id: 20230417-topic-dpu_regbus-abc94a770952

Best regards,
-- 
Konrad Dybcio 



Re: [Freedreno] [Nouveau] XDC 2023: Registration & Call for Proposals now open!

2023-04-18 Thread Luna Jernberg
Signed up will only attend virtually Online however just fyi

On Mon, Apr 17, 2023 at 1:41 PM Samuel Iglesias Gonsálvez
 wrote:
>
> Hello!
>
> Registration & Call for Proposals are now open for XDC 2023, which will
> take place on October 17-19, 2023.
>
> https://xdc2023.x.org
>
> As usual, the conference is free of charge and open to the general
> public. If you plan on attending, please make sure to register as early
> as possible!
>
> In order to register as attendee, you will therefore need to register
> via the XDC website.
>
> https://indico.freedesktop.org/event/4/registrations/
>
> In addition to registration, the CfP is now open for talks, workshops
> and demos at XDC 2023. While any serious proposal will be gratefully
> considered, topics of interest to X.Org and freedesktop.org developers
> are encouraged. The program focus is on new development, ongoing
> challenges and anything else that will spark discussions among
> attendees in the hallway track.
>
> We are open to talks across all layers of the graphics stack, from the
> kernel to desktop environments / graphical applications and about how
> to make things better for the developers who build them. Head to the
> CfP page to learn more:
>
> https://indico.freedesktop.org/event/4/abstracts/
>
> The deadline for submissions is Monday, 17 July 2023 (23:59 CEST)
>
> Check out our Reimbursement Policy to accept speaker expenses:
>
> https://www.x.org/wiki/XorgFoundation/Policies/Reimbursement/
>
> If you have any questions, please send me an email to
> siglesias AT igalia.com, adding on Cc the X.org board (board
> at foundation.x.org).
>
> And please keep in mind, you can follow us on Twitter for all the latest
> updates and to stay connected:
>
> https://twitter.com/XOrgDevConf
>
> Best,
>
> Sam


Re: [Freedreno] [RFC 0/3] drm: Add comm/cmdline fdinfo fields

2023-04-18 Thread Konrad Dybcio
Looks like the 'PATCH' part of your subject was cut off!

Konrad

On 17.04.2023 22:12, Rob Clark wrote:
> From: Rob Clark 
> 
> When many of the things using the GPU are processes in a VM guest, the
> actual client process is just a proxy.  The msm driver has a way to let
> the proxy tell the kernel the actual VM client process's executable name
> and command-line, which has until now been used simply for GPU crash
> devcore dumps.  Lets also expose this via fdinfo so that tools can
> expose who the actual user of the GPU is.
> 
> Rob Clark (3):
>   drm/doc: Relax fdinfo string constraints
>   drm/msm: Rework get_comm_cmdline() helper
>   drm/msm: Add comm/cmdline fields
> 
>  Documentation/gpu/drm-usage-stats.rst   | 37 +++--
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c |  4 +--
>  drivers/gpu/drm/msm/msm_drv.c   |  2 ++
>  drivers/gpu/drm/msm/msm_gpu.c   | 27 +-
>  drivers/gpu/drm/msm/msm_gpu.h   | 12 ++--
>  drivers/gpu/drm/msm/msm_submitqueue.c   |  1 +
>  6 files changed, 58 insertions(+), 25 deletions(-)
> 


Re: [Freedreno] [RFC 3/3] drm/msm: Add comm/cmdline fields

2023-04-18 Thread Tvrtko Ursulin



On 17/04/2023 21:12, Rob Clark wrote:

From: Rob Clark 

Normally this would be the same information that can be obtained in
other ways.  But in some cases the process opening the drm fd is merely
a sort of proxy for the actual process using the GPU.  This is the case
for guest VM processes using the GPU via virglrenderer, in which case
the msm native-context renderer in virglrenderer overrides the comm/
cmdline to be the guest process's values.

Exposing this via fdinfo allows tools like gputop to show something more
meaningful than just a bunch of "pcivirtio-gpu" users.


You also later expanded with:

"""
I should have also mentioned, in the VM/proxy scenario we have a
single process with separate drm_file's for each guest VM process.  So
it isn't an option to just change the proxy process's name to match
the client.
"""

So how does that work - this single process temporarily changes it's 
name for each drm fd it opens and creates a context or it is actually in 
the native context protocol?




Signed-off-by: Rob Clark 
---
  Documentation/gpu/drm-usage-stats.rst |  8 
  drivers/gpu/drm/msm/msm_gpu.c | 14 ++
  2 files changed, 22 insertions(+)

diff --git a/Documentation/gpu/drm-usage-stats.rst 
b/Documentation/gpu/drm-usage-stats.rst
index 8e00d53231e0..bc90bed455e3 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -148,6 +148,14 @@ percentage utilization of the engine, whereas 
drm-engine- only reflects
  time active without considering what frequency the engine is operating as a
  percentage of it's maximum frequency.
  
+- drm-comm: 

+
+Returns the clients executable path.


Full path and not just current->comm? In this case probably give it a 
more descriptive name here.


drm-client-executable
drm-client-command-line

So we stay in the drm-client- namespace?

Or if the former is absolute path could one key be enough for both?

drm-client-command-line: /path/to/executable --arguments


+
+- drm-cmdline: 
+
+Returns the clients cmdline.


I think drm-usage-stats.rst text should provide some more text with 
these two. To precisely define their content and outline the use case 
under which driver authors may want to add them, and fdinfo consumer 
therefore expect to see them. Just so everything is completely clear and 
people do not start adding them for drivers which do not support native 
context (or like).


But on the overall it sounds reasonable to me - it would be really cool 
to not just see pcivirtio-gpu as you say. Even if the standard virtiogpu 
use case (not native context) could show real users.


Regards,

Tvrtko


+
  Implementation Details
  ==
  
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c

index f0f4f845c32d..1150dcbf28aa 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -148,12 +148,26 @@ int msm_gpu_pm_suspend(struct msm_gpu *gpu)
return 0;
  }
  
+static void get_comm_cmdline(struct msm_file_private *ctx, char **comm, char **cmd);

+
  void msm_gpu_show_fdinfo(struct msm_gpu *gpu, struct msm_file_private *ctx,
 struct drm_printer *p)
  {
+   char *comm, *cmdline;
+
+   get_comm_cmdline(ctx, , );
+
drm_printf(p, "drm-engine-gpu:\t%llu ns\n", ctx->elapsed_ns);
drm_printf(p, "drm-cycles-gpu:\t%llu\n", ctx->cycles);
drm_printf(p, "drm-maxfreq-gpu:\t%u Hz\n", gpu->fast_rate);
+
+   if (comm)
+   drm_printf(p, "drm-comm:\t%s\n", comm);
+   if (cmdline)
+   drm_printf(p, "drm-cmdline:\t%s\n", cmdline);
+
+   kfree(comm);
+   kfree(cmdline);
  }
  
  int msm_gpu_hw_init(struct msm_gpu *gpu)


Re: [Freedreno] [RFC 2/3] drm/msm: Rework get_comm_cmdline() helper

2023-04-18 Thread Daniel Vetter
On Tue, Apr 18, 2023 at 09:27:49AM +0100, Tvrtko Ursulin wrote:
> 
> On 17/04/2023 21:12, Rob Clark wrote:
> > From: Rob Clark 
> > 
> > Make it work in terms of ctx so that it can be re-used for fdinfo.
> > 
> > Signed-off-by: Rob Clark 
> > ---
> >   drivers/gpu/drm/msm/adreno/adreno_gpu.c |  4 ++--
> >   drivers/gpu/drm/msm/msm_drv.c   |  2 ++
> >   drivers/gpu/drm/msm/msm_gpu.c   | 13 ++---
> >   drivers/gpu/drm/msm/msm_gpu.h   | 12 ++--
> >   drivers/gpu/drm/msm/msm_submitqueue.c   |  1 +
> >   5 files changed, 21 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index bb38e728864d..43c4e1fea83f 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -412,7 +412,7 @@ int adreno_set_param(struct msm_gpu *gpu, struct 
> > msm_file_private *ctx,
> > /* Ensure string is null terminated: */
> > str[len] = '\0';
> > -   mutex_lock(>lock);
> > +   mutex_lock(>lock);
> > if (param == MSM_PARAM_COMM) {
> > paramp = >comm;
> > @@ -423,7 +423,7 @@ int adreno_set_param(struct msm_gpu *gpu, struct 
> > msm_file_private *ctx,
> > kfree(*paramp);
> > *paramp = str;
> > -   mutex_unlock(>lock);
> > +   mutex_unlock(>lock);
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > index 3d73b98d6a9c..ca0e89e46e13 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -581,6 +581,8 @@ static int context_init(struct drm_device *dev, struct 
> > drm_file *file)
> > rwlock_init(>queuelock);
> > kref_init(>ref);
> > +   ctx->pid = get_pid(task_pid(current));
> 
> Would it simplify things for msm if DRM core had an up to date file->pid as
> proposed in
> https://patchwork.freedesktop.org/patch/526752/?series=109902=4 ? It
> gets updated if ioctl issuer is different than fd opener and this being
> context_init here reminded me of it. Maybe you wouldn't have to track the
> pid in msm?

Can we go one step further and let the drm fdinfo stuff print these new
additions? Consistency across drivers and all that.

Also for a generic trigger I think any driver ioctl is good enough (we
only really need to avoid the auth dance when you're not on a render
node).
-Daniel

> 
> Regards,
> 
> Tvrtko
> 
> > +   mutex_init(>lock);
> > msm_submitqueue_init(dev, ctx);
> > ctx->aspace = msm_gpu_create_private_address_space(priv->gpu, current);
> > diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> > index c403912d13ab..f0f4f845c32d 100644
> > --- a/drivers/gpu/drm/msm/msm_gpu.c
> > +++ b/drivers/gpu/drm/msm/msm_gpu.c
> > @@ -327,18 +327,17 @@ find_submit(struct msm_ringbuffer *ring, uint32_t 
> > fence)
> >   static void retire_submits(struct msm_gpu *gpu);
> > -static void get_comm_cmdline(struct msm_gem_submit *submit, char **comm, 
> > char **cmd)
> > +static void get_comm_cmdline(struct msm_file_private *ctx, char **comm, 
> > char **cmd)
> >   {
> > -   struct msm_file_private *ctx = submit->queue->ctx;
> > struct task_struct *task;
> > -   WARN_ON(!mutex_is_locked(>gpu->lock));
> > -
> > /* Note that kstrdup will return NULL if argument is NULL: */
> > +   mutex_lock(>lock);
> > *comm = kstrdup(ctx->comm, GFP_KERNEL);
> > *cmd  = kstrdup(ctx->cmdline, GFP_KERNEL);
> > +   mutex_unlock(>lock);
> > -   task = get_pid_task(submit->pid, PIDTYPE_PID);
> > +   task = get_pid_task(ctx->pid, PIDTYPE_PID);
> > if (!task)
> > return;
> > @@ -372,7 +371,7 @@ static void recover_worker(struct kthread_work *work)
> > if (submit->aspace)
> > submit->aspace->faults++;
> > -   get_comm_cmdline(submit, , );
> > +   get_comm_cmdline(submit->queue->ctx, , );
> > if (comm && cmd) {
> > DRM_DEV_ERROR(dev->dev, "%s: offending task: %s (%s)\n",
> > @@ -460,7 +459,7 @@ static void fault_worker(struct kthread_work *work)
> > goto resume_smmu;
> > if (submit) {
> > -   get_comm_cmdline(submit, , );
> > +   get_comm_cmdline(submit->queue->ctx, , );
> > /*
> >  * When we get GPU iova faults, we can get 1000s of them,
> > diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> > index 7a4fa1b8655b..b2023a42116b 100644
> > --- a/drivers/gpu/drm/msm/msm_gpu.h
> > +++ b/drivers/gpu/drm/msm/msm_gpu.h
> > @@ -377,17 +377,25 @@ struct msm_file_private {
> >  */
> > int sysprof;
> > +   /** @pid: Process that opened this file. */
> > +   struct pid *pid;
> > +
> > +   /**
> > +* lock: Protects comm and cmdline
> > +*/
> > +   struct mutex lock;
> > +
> > /**
> >  * comm: Overridden task comm, see MSM_PARAM_COMM
> >  *
> > 

Re: [Freedreno] [RFC 2/3] drm/msm: Rework get_comm_cmdline() helper

2023-04-18 Thread Tvrtko Ursulin



On 17/04/2023 21:12, Rob Clark wrote:

From: Rob Clark 

Make it work in terms of ctx so that it can be re-used for fdinfo.

Signed-off-by: Rob Clark 
---
  drivers/gpu/drm/msm/adreno/adreno_gpu.c |  4 ++--
  drivers/gpu/drm/msm/msm_drv.c   |  2 ++
  drivers/gpu/drm/msm/msm_gpu.c   | 13 ++---
  drivers/gpu/drm/msm/msm_gpu.h   | 12 ++--
  drivers/gpu/drm/msm/msm_submitqueue.c   |  1 +
  5 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index bb38e728864d..43c4e1fea83f 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -412,7 +412,7 @@ int adreno_set_param(struct msm_gpu *gpu, struct 
msm_file_private *ctx,
/* Ensure string is null terminated: */
str[len] = '\0';
  
-		mutex_lock(>lock);

+   mutex_lock(>lock);
  
  		if (param == MSM_PARAM_COMM) {

paramp = >comm;
@@ -423,7 +423,7 @@ int adreno_set_param(struct msm_gpu *gpu, struct 
msm_file_private *ctx,
kfree(*paramp);
*paramp = str;
  
-		mutex_unlock(>lock);

+   mutex_unlock(>lock);
  
  		return 0;

}
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 3d73b98d6a9c..ca0e89e46e13 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -581,6 +581,8 @@ static int context_init(struct drm_device *dev, struct 
drm_file *file)
rwlock_init(>queuelock);
  
  	kref_init(>ref);

+   ctx->pid = get_pid(task_pid(current));


Would it simplify things for msm if DRM core had an up to date file->pid 
as proposed in 
https://patchwork.freedesktop.org/patch/526752/?series=109902=4 ? It 
gets updated if ioctl issuer is different than fd opener and this being 
context_init here reminded me of it. Maybe you wouldn't have to track 
the pid in msm?


Regards,

Tvrtko


+   mutex_init(>lock);
msm_submitqueue_init(dev, ctx);
  
  	ctx->aspace = msm_gpu_create_private_address_space(priv->gpu, current);

diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index c403912d13ab..f0f4f845c32d 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -327,18 +327,17 @@ find_submit(struct msm_ringbuffer *ring, uint32_t fence)
  
  static void retire_submits(struct msm_gpu *gpu);
  
-static void get_comm_cmdline(struct msm_gem_submit *submit, char **comm, char **cmd)

+static void get_comm_cmdline(struct msm_file_private *ctx, char **comm, char 
**cmd)
  {
-   struct msm_file_private *ctx = submit->queue->ctx;
struct task_struct *task;
  
-	WARN_ON(!mutex_is_locked(>gpu->lock));

-
/* Note that kstrdup will return NULL if argument is NULL: */
+   mutex_lock(>lock);
*comm = kstrdup(ctx->comm, GFP_KERNEL);
*cmd  = kstrdup(ctx->cmdline, GFP_KERNEL);
+   mutex_unlock(>lock);
  
-	task = get_pid_task(submit->pid, PIDTYPE_PID);

+   task = get_pid_task(ctx->pid, PIDTYPE_PID);
if (!task)
return;
  
@@ -372,7 +371,7 @@ static void recover_worker(struct kthread_work *work)

if (submit->aspace)
submit->aspace->faults++;
  
-		get_comm_cmdline(submit, , );

+   get_comm_cmdline(submit->queue->ctx, , );
  
  		if (comm && cmd) {

DRM_DEV_ERROR(dev->dev, "%s: offending task: %s (%s)\n",
@@ -460,7 +459,7 @@ static void fault_worker(struct kthread_work *work)
goto resume_smmu;
  
  	if (submit) {

-   get_comm_cmdline(submit, , );
+   get_comm_cmdline(submit->queue->ctx, , );
  
  		/*

 * When we get GPU iova faults, we can get 1000s of them,
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 7a4fa1b8655b..b2023a42116b 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -377,17 +377,25 @@ struct msm_file_private {
 */
int sysprof;
  
+	/** @pid: Process that opened this file. */

+   struct pid *pid;
+
+   /**
+* lock: Protects comm and cmdline
+*/
+   struct mutex lock;
+
/**
 * comm: Overridden task comm, see MSM_PARAM_COMM
 *
-* Accessed under msm_gpu::lock
+* Accessed under msm_file_private::lock
 */
char *comm;
  
  	/**

 * cmdline: Overridden task cmdline, see MSM_PARAM_CMDLINE
 *
-* Accessed under msm_gpu::lock
+* Accessed under msm_file_private::lock
 */
char *cmdline;
  
diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c b/drivers/gpu/drm/msm/msm_submitqueue.c

index 0e803125a325..0444ba04fa06 100644
--- a/drivers/gpu/drm/msm/msm_submitqueue.c
+++ b/drivers/gpu/drm/msm/msm_submitqueue.c
@@ -61,6 +61,7 @@ void __msm_file_private_destroy(struct kref *kref)

Re: [Freedreno] [PATCH 1/3] drm/msm/dpu: Drop unused members from HW structs

2023-04-18 Thread Marijn Suijten
On 2023-04-17 18:54:18, Abhinav Kumar wrote:
> 
> On 4/17/2023 4:14 PM, Marijn Suijten wrote:
> > Some of these members were initialized while never read, while others
> > were not even assigned any value at all.  Drop them to save some space,
> > and above all confusion when looking at these members.
> > 
> > Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
> > Fixes: 84a33d0fd921 ("drm/msm/dpu: add dpu_hw_wb abstraction for writeback 
> > blocks")
> > Signed-off-by: Marijn Suijten 
> > ---
> 
> It seems like WB UBWC formats are not supported today. Because otherwise 
> ctx->mdp would be used for writeback. I guess we can add a ubwc member 
> similar to hw_sspp, when we do add the support for this. Hence this is,

That seems preferable to me.  SSPP does the same in patch 3/3, and I
have been thinking to replace the dpu_mdss_cfg *catalog parameter in
dpu_hw_sspp_init with just the ubwc cfg pointer.  Likewise
dpu_hw_ctl_init also takes the full dpu_mdss_cfg only to take out the
mixers.

- Marijn

> Reviewed-by: Abhinav Kumar 


Re: [Freedreno] [PATCH 1/5] dt-bindings: display/msm: Add reg bus interconnect

2023-04-18 Thread Krzysztof Kozlowski
On 17/04/2023 17:30, Konrad Dybcio wrote:
> Apart from the already handled data bus (MAS_MDP_Pn<->DDR), there's
> another path that needs to be handled to ensure MDSS functions properly,
> namely the "reg bus", a.k.a the CPU-MDSS interconnect.
> 
> Gating that path may have a variety of effects.. from none to otherwise
> inexplicable DSI timeouts..
> 
> Describe it in bindings to allow for use in device trees.
> 
> Signed-off-by: Konrad Dybcio 
> ---
>  Documentation/devicetree/bindings/display/msm/mdss-common.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml 
> b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
> index ccd7d6417523..9eb5b6d3e0b9 100644
> --- a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
> @@ -72,6 +72,7 @@ properties:
>  items:
>- const: mdp0-mem
>- const: mdp1-mem
> +  - const: cpu-cfg

You added only interconnect-name, not actual interconnect.

Best regards,
Krzysztof



Re: [Freedreno] [PATCH v9 01/10] drm/hdcp: Add drm_hdcp_atomic_check()

2023-04-18 Thread Jani Nikula
On Tue, 18 Apr 2023, "Kandpal, Suraj"  wrote:
> Yacoub
>> ; linux-ker...@vger.kernel.org
>> Subject: [PATCH v9 01/10] drm/hdcp: Add drm_hdcp_atomic_check()
>>
>> From: Sean Paul 
>>
>> Move the hdcp atomic check from i915 to drm_hdcp so other drivers can use
>> it. No functional changes, just cleaned up some of the code when moving it
>> over.
>>
>> Acked-by: Jani Nikula 
>> Reviewed-by: Rodrigo Vivi 
>> Reviewed-by: Dmitry Baryshkov 
>> Signed-off-by: Sean Paul 
>> Signed-off-by: Mark Yacoub 
>>
>> ---
>> Changes in v2:
>> -None
>> Changes in v3:
>> -None
>> Changes in v4:
>> -None
>> Changes in v5:
>> -None
>> Changes in v6:
>> -Rebase: move helper from drm_hdcp.c to drm_hdcp_helper.c Changes in
>> v7:
>> -Removed links to patch from commit msg (Dmitry Baryshkov)
>>
>>  drivers/gpu/drm/display/drm_hdcp_helper.c   | 64
>> +
>>  drivers/gpu/drm/i915/display/intel_atomic.c |  4 +-
>>  drivers/gpu/drm/i915/display/intel_hdcp.c   | 47 ---
>>  drivers/gpu/drm/i915/display/intel_hdcp.h   |  3 -
>>  include/drm/display/drm_hdcp_helper.h   |  3 +
>>  5 files changed, 69 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_hdcp_helper.c
>> b/drivers/gpu/drm/display/drm_hdcp_helper.c
>> index e78999c72bd77..7ca390b3ea106 100644
>> --- a/drivers/gpu/drm/display/drm_hdcp_helper.c
>> +++ b/drivers/gpu/drm/display/drm_hdcp_helper.c
>> @@ -20,6 +20,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  static inline void drm_hdcp_print_ksv(const u8 *ksv)  { @@ -419,3 +420,66
>> @@ void drm_hdcp_update_content_protection(struct drm_connector
>> *connector,
>>dev-
>> >mode_config.content_protection_property);
>>  }
>>  EXPORT_SYMBOL(drm_hdcp_update_content_protection);
>> +
>> +/**
>> + * drm_hdcp_atomic_check - Helper for drivers to call during
>> +connector->atomic_check
>> + *
>> + * @state: pointer to the atomic state being checked
>> + * @connector: drm_connector on which content protection state needs an
>> +update
>> + *
>> + * This function can be used by display drivers to perform an atomic
>> +check on the
>> + * hdcp state elements. If hdcp state has changed, this function will
>> +set
>> + * mode_changed on the crtc driving the connector so it can update its
>> +hardware
>> + * to match the hdcp state.
>> + */
>> +void drm_hdcp_atomic_check(struct drm_connector *connector,
>> +struct drm_atomic_state *state)
>> +{
>> + struct drm_connector_state *new_conn_state, *old_conn_state;
>> + struct drm_crtc_state *new_crtc_state;
>> + u64 old_hdcp, new_hdcp;
>> +
>> + old_conn_state = drm_atomic_get_old_connector_state(state,
>> connector);
>> + old_hdcp = old_conn_state->content_protection;
>> +
>> + new_conn_state = drm_atomic_get_new_connector_state(state,
>> connector);
>> + new_hdcp = new_conn_state->content_protection;
>> +
>
> Nit: Why not assign these as right when they are declared we would end up 
> using less lines

The rule of thumb is to only initialize trivial stuff at declaration.

BR,
Jani.

>
>> + if (!new_conn_state->crtc) {
>> + /*
>> +  * If the connector is being disabled with CP enabled, mark it
>> +  * desired so it's re-enabled when the connector is brought
>> back
>> +  */
>> + if (old_hdcp ==
>> DRM_MODE_CONTENT_PROTECTION_ENABLED)
>> + new_conn_state->content_protection =
>> +
>>   DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> + return;
>> + }
>> +
>> + new_crtc_state =
>> + drm_atomic_get_new_crtc_state(state, new_conn_state-
>> >crtc);
>> + if (drm_atomic_crtc_needs_modeset(new_crtc_state) &&
>> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
>> +  new_hdcp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED))
>> + new_conn_state->content_protection =
>> + DRM_MODE_CONTENT_PROTECTION_DESIRED;
>> +
>> + /*
>> +  * Nothing to do if content type is unchanged and one of:
>> +  *  - state didn't change
>> +  *  - HDCP was activated since the last commit
>> +  *  - attempting to set to desired while already enabled
>> +  */
>> + if (old_hdcp == new_hdcp ||
>> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
>> +  new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
>> + (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
>> +  new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
>> + if (old_conn_state->hdcp_content_type ==
>> + new_conn_state->hdcp_content_type)
>> + return;
>> + }
>> +
>> + new_crtc_state->mode_changed = true;
>> +}
>> +EXPORT_SYMBOL(drm_hdcp_atomic_check);
>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
>> b/drivers/gpu/drm/i915/display/intel_atomic.c
>> index a9a3f3715279d..e9d00b6a63d39 100644
>> --- 

Re: [Freedreno] [PATCH v9 07/10] drm/i915/hdcp: Use HDCP helpers for i915

2023-04-18 Thread Kandpal, Suraj



> -Original Message-
> From: Mark Yacoub 
> Sent: Wednesday, April 12, 2023 12:52 AM
> To: Jani Nikula ; Joonas Lahtinen
> ; Vivi, Rodrigo ;
> Tvrtko Ursulin ; David Airlie
> ; Daniel Vetter 
> Cc: seanp...@chromium.org; Kandpal, Suraj ;
> diand...@chromium.org; dmitry.barysh...@linaro.org; dri-
> de...@lists.freedesktop.org; freedreno@lists.freedesktop.org; intel-
> g...@lists.freedesktop.org; Nikula, Jani ; Mark Yacoub
> ; linux-ker...@vger.kernel.org
> Subject: [PATCH v9 07/10] drm/i915/hdcp: Use HDCP helpers for i915
> 
> From: Sean Paul 
> 
> Now that all of the HDCP 1.x logic has been migrated to the central HDCP
> helpers, use it in the i915 driver.
> 
> The majority of the driver code for HDCP 1.x will live in intel_hdcp.c,
> however there are a few helper hooks which are connector-specific and
> need to be partially or fully implemented in the intel_dp_hdcp.c or
> intel_hdmi.c.
> 
> We'll leave most of the HDCP 2.x code alone since we don't have another
> implementation of HDCP 2.x to use as reference for what should and
> should not live in the drm helpers. The helper will call the overly
> general enable/disable/is_capable HDCP 2.x callbacks and leave the
> interesting stuff for the driver. Once we have another HDCP 2.x
> implementation, we should do a similar migration.
> 
> Acked-by: Jani Nikula 
> Acked-by: Rodrigo Vivi 
> Signed-off-by: Sean Paul 
> Signed-off-by: Mark Yacoub 
> 
> ---
> Changes in v2:
> -Fix mst helper function pointer reported by 0-day
> Changes in v3:
> -Add forward declaration for drm_atomic_state in intel_hdcp.h identified
>  by 0-day
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Rebased.
> Changes in v7:
> -Added to drm_hdcp_helper_funcs new functions that are unique between
> DP
> and HDMI
> -Adjusted the function signatures to take "driver data"
> Changes in v8:
> -None
> Changes in v9:
> -rename dev_priv to i915
> -remove display type specific hdcp calls init from driver
> 
>  drivers/gpu/drm/i915/display/intel_ddi.c  |  32 +-
>  .../drm/i915/display/intel_display_debugfs.c  |   7 +-
>  .../drm/i915/display/intel_display_types.h|  51 +-
>  drivers/gpu/drm/i915/display/intel_dp_hdcp.c  | 352 +++
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  16 +-
>  drivers/gpu/drm/i915/display/intel_hdcp.c | 984 --
>  drivers/gpu/drm/i915/display/intel_hdcp.h |  43 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 267 ++---
>  8 files changed, 475 insertions(+), 1277 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 254559abedfba..8a2f20c929e9c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -28,6 +28,7 @@
>  #include 
> 
>  #include 
> +#include 
>  #include 
> 
>  #include "i915_drv.h"
> @@ -2956,6 +2957,10 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
>const struct intel_crtc_state *crtc_state,
>const struct drm_connector_state *conn_state)
>  {
> + struct intel_connector *connector =
> + to_intel_connector(conn_state->connector);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +
>   drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
> 
>   if (!intel_crtc_is_bigjoiner_slave(crtc_state))
> @@ -2975,12 +2980,10 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
>   else
>   intel_enable_ddi_dp(state, encoder, crtc_state, conn_state);
> 
> - /* Enable hdcp if it's desired */
> - if (conn_state->content_protection ==
> - DRM_MODE_CONTENT_PROTECTION_DESIRED)
> - intel_hdcp_enable(to_intel_connector(conn_state-
> >connector),
> -   crtc_state,
> -   (u8)conn_state->hdcp_content_type);
> + if (connector->hdcp_helper_data)
> + drm_hdcp_helper_atomic_commit(connector-
> >hdcp_helper_data,
> +   >base,
> +   _port->hdcp_mutex);
>  }
> 
>  static void intel_disable_ddi_dp(struct intel_atomic_state *state,
> @@ -3026,7 +3029,14 @@ static void intel_disable_ddi(struct
> intel_atomic_state *state,
> const struct intel_crtc_state *old_crtc_state,
> const struct drm_connector_state
> *old_conn_state)
>  {
> - intel_hdcp_disable(to_intel_connector(old_conn_state->connector));
> + struct intel_connector *connector =
> + to_intel_connector(old_conn_state->connector);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +
> + if (connector->hdcp_helper_data)
> + drm_hdcp_helper_atomic_commit(connector-
> >hdcp_helper_data,
> +   >base,
> +