| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <myumofurrdyjpfs7k6m2q4wmfcot3pdnymtoj4wd3zwlgmkhtq@mjtlpsb5by2j>
Date: Sun, 8 Dec 2024 08:20:19 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Abhinav Kumar <quic_abhinavk@...cinc.com>
Cc: Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Stephen Boyd <swboyd@...omium.org>,
Chandan Uddaraju <chandanu@...eaurora.org>, Guenter Roeck <groeck@...omium.org>,
Kuogee Hsieh <quic_khsieh@...cinc.com>, Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Vara Reddy <quic_varar@...cinc.com>, Rob Clark <robdclark@...omium.org>,
Tanmay Shah <tanmay@...eaurora.org>, linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
Jessica Zhang <quic_jesszhan@...cinc.com>, Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH 22/45] drm/msm/dp: add support to send vcpf packets in dp
controller
On Thu, Dec 05, 2024 at 08:31:53PM -0800, Abhinav Kumar wrote:
> VC payload fill sequence is inserted by the DP controller in the
> absence of stream symbols that is before stream is disabled. Add
> support to send the VCPF sequence for msm dp controller.
>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
> ---
> drivers/gpu/drm/msm/dp/dp_catalog.c | 25 +++++++++++++++++++++++
> drivers/gpu/drm/msm/dp/dp_catalog.h | 4 ++++
> drivers/gpu/drm/msm/dp/dp_ctrl.c | 40 +++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 +
> drivers/gpu/drm/msm/dp/dp_display.c | 5 ++++-
> drivers/gpu/drm/msm/dp/dp_reg.h | 3 ++-
> 6 files changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index f9d21444d7891bcd043d282b31ae75711add4817..4826a698979ce7c37112812299879411c5743fa9 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -50,6 +50,11 @@
> (PSR_UPDATE_INT | PSR_CAPTURE_INT | PSR_EXIT_INT | \
> PSR_UPDATE_ERROR_INT | PSR_WAKE_ERROR_INT)
>
> +#define DP_INTERRUPT_STATUS5 \
> + (DP_INTR_DP0_VCPF_SENT | DP_INTR_DP1_VCPF_SENT)
> +#define DP_INTERRUPT_STATUS5_MASK \
> + (DP_INTERRUPT_STATUS5 << DP_INTERRUPT_STATUS_MASK_SHIFT)
> +
> #define DP_INTERRUPT_MASK4 \
> (PSR_UPDATE_MASK | PSR_CAPTURE_MASK | PSR_EXIT_MASK | \
> PSR_UPDATE_ERROR_MASK | PSR_WAKE_ERROR_MASK)
> @@ -694,9 +699,12 @@ void msm_dp_catalog_ctrl_enable_irq(struct msm_dp_catalog *msm_dp_catalog,
> DP_INTERRUPT_STATUS1_MASK);
> msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS2,
> DP_INTERRUPT_STATUS2_MASK);
> + msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS5,
> + DP_INTERRUPT_STATUS5_MASK);
> } else {
> msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS, 0x00);
> msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS2, 0x00);
> + msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS5, 0x00);
> }
> }
>
> @@ -850,6 +858,23 @@ int msm_dp_catalog_ctrl_get_interrupt(struct msm_dp_catalog *msm_dp_catalog)
> return intr;
> }
>
> +int msm_dp_catalog_ctrl_get_interrupt_5(struct msm_dp_catalog *msm_dp_catalog)
> +{
> + struct msm_dp_catalog_private *catalog = container_of(msm_dp_catalog,
> + struct msm_dp_catalog_private,
> + msm_dp_catalog);
> + u32 intr, intr_ack;
> +
> + intr = msm_dp_read_ahb(catalog, REG_DP_INTR_STATUS5);
> + intr &= ~DP_INTERRUPT_STATUS5_MASK;
> + intr_ack = (intr & DP_INTERRUPT_STATUS5)
> + << DP_INTERRUPT_STATUS_ACK_SHIFT;
> + msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS5,
> + intr_ack | DP_INTERRUPT_STATUS5_MASK);
> +
> + return intr;
> +}
> +
> void msm_dp_catalog_ctrl_phy_reset(struct msm_dp_catalog *msm_dp_catalog)
> {
> struct msm_dp_catalog_private *catalog = container_of(msm_dp_catalog,
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h b/drivers/gpu/drm/msm/dp/dp_catalog.h
> index 560016e2f929d4b92d6ea764d81a099c09c0e668..323858c587f85996d296156c7b8b201cdb7b7eb4 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.h
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
> @@ -28,6 +28,9 @@
> #define DP_INTR_FRAME_END BIT(6)
> #define DP_INTR_CRC_UPDATED BIT(9)
>
> +#define DP_INTR_DP0_VCPF_SENT BIT(0)
> +#define DP_INTR_DP1_VCPF_SENT BIT(3)
> +
> #define DP_HW_VERSION_1_0 0x10000000
> #define DP_HW_VERSION_1_2 0x10020000
>
> @@ -103,6 +106,7 @@ u32 msm_dp_catalog_link_is_connected(struct msm_dp_catalog *msm_dp_catalog);
> u32 msm_dp_catalog_hpd_get_intr_status(struct msm_dp_catalog *msm_dp_catalog);
> void msm_dp_catalog_ctrl_phy_reset(struct msm_dp_catalog *msm_dp_catalog);
> int msm_dp_catalog_ctrl_get_interrupt(struct msm_dp_catalog *msm_dp_catalog);
> +int msm_dp_catalog_ctrl_get_interrupt_5(struct msm_dp_catalog *msm_dp_catalog);
> u32 msm_dp_catalog_ctrl_read_psr_interrupt_status(struct msm_dp_catalog *msm_dp_catalog);
I'd prefer some uniformity here. read_psr_interrupt_status() returns
REG_DP_INTR_STATUS4, get_interrupt() returns REG_DP_INTR_STATUS2 value,
Now you are adding get_interrupt_5(). Could you please make that similar
and logical?
> void msm_dp_catalog_ctrl_update_transfer_unit(struct msm_dp_catalog *msm_dp_catalog,
> u32 msm_dp_tu, u32 valid_boundary,
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 14562def1e70b769434243d1ce72661a7b4d4c6b..2288c379283c721a01c81302f8d307d0b3c76527 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -27,6 +27,11 @@
>
> #define DP_CTRL_INTR_READY_FOR_VIDEO BIT(0)
> #define DP_CTRL_INTR_IDLE_PATTERN_SENT BIT(3)
> +#define DP_CTRL_INTR_DP0_VCPF_SENT BIT(0)
> +#define DP_CTRL_INTR_DP1_VCPF_SENT BIT(3)
> +
> +#define MST_DP0_PUSH_VCPF BIT(12)
> +#define MST_DP1_PUSH_VCPF BIT(14)
>
> #define MR_LINK_TRAINING1 0x8
> #define MR_LINK_SYMBOL_ERM 0x80
> @@ -144,6 +149,34 @@ void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl)
> drm_dbg_dp(ctrl->drm_dev, "mainlink off\n");
> }
>
> +void msm_dp_ctrl_push_vcpf(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *msm_dp_panel)
> +{
> + u32 state = 0x0;
> + struct msm_dp_ctrl_private *ctrl;
> +
> + ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
> +
> + if (msm_dp_panel->stream_id >= DP_STREAM_MAX) {
> + DRM_ERROR("invalid input\n");
> + return;
> + }
> +
> + if (msm_dp_panel->stream_id == DP_STREAM_0)
> + state |= MST_DP0_PUSH_VCPF;
> + else
> + state |= MST_DP1_PUSH_VCPF;
> +
> + reinit_completion(&ctrl->idle_comp);
> +
> + msm_dp_catalog_ctrl_state_ctrl(ctrl->catalog, state);
> +
> + if (!wait_for_completion_timeout(&ctrl->idle_comp,
> + IDLE_PATTERN_COMPLETION_TIMEOUT_JIFFIES))
> + pr_warn("PUSH_VCPF pattern timedout\n");
> +
> + drm_dbg_dp(ctrl->drm_dev, "mainlink off\n");
??
The comment seems misplaced or wrong to me. The msm_dp_ctrl_push_vcpf()
doesn't turn link off.
> +}
> +
> static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl,
> struct msm_dp_panel *msm_dp_panel)
> {
> @@ -2332,6 +2365,13 @@ irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl)
> ret = IRQ_HANDLED;
> }
>
> + isr = msm_dp_catalog_ctrl_get_interrupt_5(ctrl->catalog);
> + if (isr & (DP_INTR_DP0_VCPF_SENT | DP_INTR_DP1_VCPF_SENT)) {
> + drm_dbg_dp(ctrl->drm_dev, "vcpf sent\n");
> + complete(&ctrl->idle_comp);
> + ret = IRQ_HANDLED;
> + }
> +
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index b126651da24b3abdaf540268758b37dca9fe1291..9ad7022d6217572395d69294c3cc4d4dbaddf0ac 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -52,5 +52,6 @@ void msm_dp_ctrl_mst_stream_channel_slot_setup(struct msm_dp_ctrl *msm_dp_ctrl,
> void msm_dp_ctrl_set_mst_channel_info(struct msm_dp_ctrl *msm_dp_ctrl,
> enum msm_dp_stream_id strm,
> u32 start_slot, u32 tot_slots);
> +void msm_dp_ctrl_push_vcpf(struct msm_dp_ctrl *dp_ctrl, struct msm_dp_panel *msm_dp_panel);
>
> #endif /* _DP_CTRL_H_ */
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 2a4a79317153817cb24537ea95fad07c9bc20715..1dfc82211c50bb4ed239f9730b91c33c4897c78f 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1646,7 +1646,10 @@ void msm_dp_display_atomic_disable(struct msm_dp *dp)
>
> msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
>
> - msm_dp_ctrl_push_idle(msm_dp_display->ctrl);
> + if (msm_dp_display->max_stream > DEFAULT_STREAM_COUNT)
Same comment regarding default vs SST
> + msm_dp_ctrl_push_vcpf(msm_dp_display->ctrl, msm_dp_display->panel);
> + else
> + msm_dp_ctrl_push_idle(msm_dp_display->ctrl);
>
> if (msm_dp_display->max_stream > DEFAULT_STREAM_COUNT) {
> msm_dp_ctrl_mst_stream_channel_slot_setup(msm_dp_display->ctrl,
> diff --git a/drivers/gpu/drm/msm/dp/dp_reg.h b/drivers/gpu/drm/msm/dp/dp_reg.h
> index 8bc2a431462fc1fb45b1fe8e43a0a0ec7f75e5b1..c7532217b369c6235b2fe5fe9c86642d5c2712cb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_reg.h
> +++ b/drivers/gpu/drm/msm/dp/dp_reg.h
> @@ -24,8 +24,9 @@
> #define REG_DP_INTR_STATUS (0x00000020)
> #define REG_DP_INTR_STATUS2 (0x00000024)
> #define REG_DP_INTR_STATUS3 (0x00000028)
> -
> #define REG_DP_INTR_STATUS4 (0x0000002C)
> +#define REG_DP_INTR_STATUS5 (0x00000034)
> +
> #define PSR_UPDATE_INT (0x00000001)
> #define PSR_CAPTURE_INT (0x00000004)
> #define PSR_EXIT_INT (0x00000010)
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists