Re: [Freedreno] 2023 X.Org Foundation Membership deadline for voting in the election
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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!
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
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
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
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
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
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
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()
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
> -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, > +