Re: [Freedreno] [PATCH v3 04/14] drm/hdcp: Expand HDCP helper library for enable/disable/check
On 2021-10-01 08:11, Sean Paul wrote:
From: Sean Paul
This patch expands upon the HDCP helper library to manage HDCP
enable, disable, and check.
Previous to this patch, the majority of the state management and sink
interaction is tucked inside the Intel driver with the understanding
that once a new platform supported HDCP we could make good decisions
about what should be centralized. With the addition of HDCP support
for Qualcomm, it's time to migrate the protocol-specific bits of HDCP
authentication, key exchange, and link checks to the HDCP helper.
In terms of functionality, this migration is 1:1 with the Intel driver,
however things are laid out a bit differently than with intel_hdcp.c,
which is why this is a separate patch from the i915 transition to the
helper. On i915, the "shim" vtable is used to account for HDMI vs. DP
vs. DP-MST differences whereas the helper library uses a LUT to
account for the register offsets and a remote read function to route
the messages. On i915, storing the sink information in the source is
done inline whereas now we use the new drm_hdcp_helper_funcs vtable
to store and fetch information to/from source hw. Finally, instead of
calling enable/disable directly from the driver, we'll leave that
decision to the helper and by calling drm_hdcp_helper_atomic_commit()
from the driver. All told, this will centralize the protocol and state
handling in the helper, ensuring we collect all of our bugs^Wlogic
in one place.
Cc: Abhinav Kumar
Acked-by: Jani Nikula
Signed-off-by: Sean Paul
For vendors/chipsets supporting HW polling, this needs rework to skip
the SW
polling, as agreed this will be done in a follow up change. Hence,
Reviewed-by: Abhinav Kumar
Link:
https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-5-s...@poorly.run
#v1
Link:
https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-5-s...@poorly.run
#v2
Changes in v2:
-Fixed set-but-unused variable identified by 0-day
Changes in v3:
-Fixed uninitialized variable warning identified by 0-day
---
drivers/gpu/drm/drm_hdcp.c | 1103
include/drm/drm_hdcp.h | 191 +++
2 files changed, 1294 insertions(+)
diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
index 8c851d40cd45..2bfa07fc3fbc 100644
--- a/drivers/gpu/drm/drm_hdcp.c
+++ b/drivers/gpu/drm/drm_hdcp.c
@@ -6,15 +6,20 @@
* Ramalingam C
*/
+#include
#include
#include
#include
+#include
+#include
#include
#include
#include
+#include
#include
#include
+#include
#include
#include
#include
@@ -513,3 +518,1101 @@ bool drm_hdcp_atomic_check(struct drm_connector
*connector,
return old_hdcp != new_hdcp;
}
EXPORT_SYMBOL(drm_hdcp_atomic_check);
+
+struct drm_hdcp_helper_data {
+ struct mutex mutex;
+ struct mutex *driver_mutex;
+
+ struct drm_connector *connector;
+ const struct drm_hdcp_helper_funcs *funcs;
+
+ u64 value;
+ unsigned int enabled_type;
+
+ struct delayed_work check_work;
+ struct work_struct prop_work;
+
+ struct drm_dp_aux *aux;
+ const struct drm_hdcp_hdcp1_receiver_reg_lut *hdcp1_lut;
+};
+
+struct drm_hdcp_hdcp1_receiver_reg_lut {
+ unsigned int bksv;
+ unsigned int ri;
+ unsigned int aksv;
+ unsigned int an;
+ unsigned int ainfo;
+ unsigned int v[5];
+ unsigned int bcaps;
+ unsigned int bcaps_mask_repeater_present;
+ unsigned int bstatus;
+};
+
+static const struct drm_hdcp_hdcp1_receiver_reg_lut
drm_hdcp_hdcp1_ddc_lut = {
+ .bksv = DRM_HDCP_DDC_BKSV,
+ .ri = DRM_HDCP_DDC_RI_PRIME,
+ .aksv = DRM_HDCP_DDC_AKSV,
+ .an = DRM_HDCP_DDC_AN,
+ .ainfo = DRM_HDCP_DDC_AINFO,
+ .v = { DRM_HDCP_DDC_V_PRIME(0), DRM_HDCP_DDC_V_PRIME(1),
+ DRM_HDCP_DDC_V_PRIME(2), DRM_HDCP_DDC_V_PRIME(3),
+ DRM_HDCP_DDC_V_PRIME(4) },
+ .bcaps = DRM_HDCP_DDC_BCAPS,
+ .bcaps_mask_repeater_present = DRM_HDCP_DDC_BCAPS_REPEATER_PRESENT,
+ .bstatus = DRM_HDCP_DDC_BSTATUS,
+};
+
+static const struct drm_hdcp_hdcp1_receiver_reg_lut
drm_hdcp_hdcp1_dpcd_lut = {
+ .bksv = DP_AUX_HDCP_BKSV,
+ .ri = DP_AUX_HDCP_RI_PRIME,
+ .aksv = DP_AUX_HDCP_AKSV,
+ .an = DP_AUX_HDCP_AN,
+ .ainfo = DP_AUX_HDCP_AINFO,
+ .v = { DP_AUX_HDCP_V_PRIME(0), DP_AUX_HDCP_V_PRIME(1),
+ DP_AUX_HDCP_V_PRIME(2), DP_AUX_HDCP_V_PRIME(3),
+ DP_AUX_HDCP_V_PRIME(4) },
+ .bcaps = DP_AUX_HDCP_BCAPS,
+ .bcaps_mask_repeater_present = DP_BCAPS_REPEATER_PRESENT,
+
+ /*
+* For some reason the HDMI and DP HDCP specs call this register
+ * definition by different names. In the HDMI spec, it's called
BSTATUS,
+* but in DP it's called BINFO.
+*/
+ .bstatus = DP_AUX_HDCP_BINFO,
+};
+
+static int drm_hdcp_remote_ddc_read(struct i2c_adapter *i2c,
+ unsigned int offset,
Re: [Freedreno] [PATCH v3 09/14] drm/msm/dpu: Remove useless checks in dpu_encoder
On 2021-10-01 08:11, Sean Paul wrote:
From: Sean Paul
A couple more useless checks to remove in dpu_encoder.
Reviewed-by: Stephen Boyd
Signed-off-by: Sean Paul
Reviewed-by: Abhinav Kumar
Link:
https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-10-s...@poorly.run
#v1
Link:
https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-10-s...@poorly.run
#v2
Changes in v2:
-None
Changes in v3:
-None
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12
1 file changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0e9d3fa1544b..984f8a59cb73 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1153,10 +1153,6 @@ static void dpu_encoder_virt_enable(struct
drm_encoder *drm_enc)
struct msm_drm_private *priv;
struct drm_display_mode *cur_mode = NULL;
- if (!drm_enc) {
- DPU_ERROR("invalid encoder\n");
- return;
- }
dpu_enc = to_dpu_encoder_virt(drm_enc);
mutex_lock(_enc->enc_lock);
@@ -1203,14 +1199,6 @@ static void dpu_encoder_virt_disable(struct
drm_encoder *drm_enc)
struct msm_drm_private *priv;
int i = 0;
- if (!drm_enc) {
- DPU_ERROR("invalid encoder\n");
- return;
- } else if (!drm_enc->dev) {
- DPU_ERROR("invalid dev\n");
- return;
- }
-
dpu_enc = to_dpu_encoder_virt(drm_enc);
DPU_DEBUG_ENC(dpu_enc, "\n");
Re: [Freedreno] [PATCH v3 03/14] drm/hdcp: Update property value on content type and user changes
On 2021-10-01 08:11, Sean Paul wrote:
From: Sean Paul
This patch updates the connector's property value in 2 cases which were
previously missed:
1- Content type changes. The value should revert back to DESIRED from
ENABLED in case the driver must re-authenticate the link due to the
new content type.
2- Userspace sets value to DESIRED while ENABLED. In this case, the
value should be reset immediately to ENABLED since the link is
actively being encrypted.
To accommodate these changes, I've split up the conditionals to make
things a bit more clear (as much as one can with this mess of state).
Acked-by: Jani Nikula
Signed-off-by: Sean Paul
Reviewed-by: Abhinav Kumar
Link:
https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-4-s...@poorly.run
#v1
Link:
https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-4-s...@poorly.run
#v2
Changes in v2:
-None
Changes in v3:
-Fixed indentation issue identified by 0-day
---
drivers/gpu/drm/drm_hdcp.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
index dd8fa91c51d6..8c851d40cd45 100644
--- a/drivers/gpu/drm/drm_hdcp.c
+++ b/drivers/gpu/drm/drm_hdcp.c
@@ -487,21 +487,29 @@ bool drm_hdcp_atomic_check(struct drm_connector
*connector,
return true;
/*
-* Nothing to do if content type is unchanged and one of:
-* - state didn't change
+* Content type changes require an HDCP disable/enable cycle.
+*/
+ if (new_conn_state->hdcp_content_type !=
old_conn_state->hdcp_content_type) {
+ new_conn_state->content_protection =
+ DRM_MODE_CONTENT_PROTECTION_DESIRED;
+ return true;
+ }
+
+ /*
+* Ignore meaningless state changes:
* - HDCP was activated since the last commit
-* - attempting to set to desired while already enabled
+* - Attempting to set to desired while already enabled
*/
- if (old_hdcp == new_hdcp ||
- (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
+ if ((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 false;
+ new_conn_state->content_protection =
+ DRM_MODE_CONTENT_PROTECTION_ENABLED;
+ return false;
}
- return true;
+ /* Finally, if state changes, we need action */
+ return old_hdcp != new_hdcp;
}
EXPORT_SYMBOL(drm_hdcp_atomic_check);
Re: [Freedreno] [PATCH v3 01/14] drm/hdcp: Add drm_hdcp_atomic_check()
On 2021-10-01 08:11, Sean Paul wrote:
From: Sean Paul
This patch moves 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
Signed-off-by: Sean Paul
For the drm/hdcp pieces:
Reviewed-by: Abhinav Kumar
Link:
https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-2-s...@poorly.run
#v1
Link:
https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-2-s...@poorly.run
#v2
Changes in v2:
-None
Changes in v3:
-None
---
drivers/gpu/drm/drm_hdcp.c | 71 -
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/drm_hdcp.h | 3 +
5 files changed, 75 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
index ca9b8f697202..522326b03e66 100644
--- a/drivers/gpu/drm/drm_hdcp.c
+++ b/drivers/gpu/drm/drm_hdcp.c
@@ -13,13 +13,14 @@
#include
#include
+#include
+#include
#include
#include
#include
#include
#include
#include
-#include
#include "drm_internal.h"
@@ -421,3 +422,71 @@ 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;
+
+ 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);
+ /*
+ * Fix the HDCP uapi content protection state in case of modeset.
+ * FIXME: As per HDCP content protection property uapi doc, an
uevent()
+ * need to be sent if there is transition from ENABLED->DESIRED.
+ */
+ 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 b4e7ac51aa31..1e306e8427ec 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -32,13 +32,13 @@
#include
#include
#include
+#include
#include
#include "intel_atomic.h"
#include "intel_cdclk.h"
#include "intel_display_types.h"
#include "intel_global_state.h"
-#include "intel_hdcp.h"
#include "intel_psr.h"
#include
Re: [Freedreno] [PATCH v3 10/14] drm/msm/dpu: Remove encoder->enable() hack
On 2021-10-01 08:11, Sean Paul wrote:
From: Sean Paul
encoder->commit() was being misused because there were some global
resources which needed to be tweaked in encoder->enable() which were
not
accessible in dpu_encoder.c. That is no longer true and the redirect
serves no purpose any longer. So remove the indirection.
Reviewed-by: Stephen Boyd
Tested-by: Stephen Boyd
Signed-off-by: Sean Paul
Reviewed-by: Abhinav Kumar
Link:
https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-11-s...@poorly.run
#v1
Link:
https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-11-s...@poorly.run
#v2
Changes in v2:
-None
Changes in v3:
-None
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 22 -
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 2 --
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 4
4 files changed, 1 insertion(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 984f8a59cb73..ddc542a0d41f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2122,11 +2122,8 @@ static void
dpu_encoder_frame_done_timeout(struct timer_list *t)
static const struct drm_encoder_helper_funcs dpu_encoder_helper_funcs
= {
.mode_set = dpu_encoder_virt_mode_set,
.disable = dpu_encoder_virt_disable,
- .enable = dpu_kms_encoder_enable,
+ .enable = dpu_encoder_virt_enable,
.atomic_check = dpu_encoder_virt_atomic_check,
-
- /* This is called by dpu_kms_encoder_enable */
- .commit = dpu_encoder_virt_enable,
};
static const struct drm_encoder_funcs dpu_encoder_funcs = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index fb0d9f781c66..4a0b55d145ad 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -381,28 +381,6 @@ static void dpu_kms_flush_commit(struct msm_kms
*kms, unsigned crtc_mask)
}
}
-/*
- * Override the encoder enable since we need to setup the inline
rotator and do
- * some crtc magic before enabling any bridge that might be present.
- */
-void dpu_kms_encoder_enable(struct drm_encoder *encoder)
-{
- const struct drm_encoder_helper_funcs *funcs =
encoder->helper_private;
- struct drm_device *dev = encoder->dev;
- struct drm_crtc *crtc;
-
- /* Forward this enable call to the commit hook */
- if (funcs && funcs->commit)
- funcs->commit(encoder);
-
- drm_for_each_crtc(crtc, dev) {
- if (!(crtc->state->encoder_mask & drm_encoder_mask(encoder)))
- continue;
-
- trace_dpu_kms_enc_enable(DRMID(crtc));
- }
-}
-
static void dpu_kms_complete_commit(struct msm_kms *kms, unsigned
crtc_mask)
{
struct dpu_kms *dpu_kms = to_dpu_kms(kms);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 323a6bce9e64..f1ebb60dacab 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -248,8 +248,6 @@ void *dpu_debugfs_get_root(struct dpu_kms
*dpu_kms);
int dpu_enable_vblank(struct msm_kms *kms, struct drm_crtc *crtc);
void dpu_disable_vblank(struct msm_kms *kms, struct drm_crtc *crtc);
-void dpu_kms_encoder_enable(struct drm_encoder *encoder);
-
/**
* dpu_kms_get_clk_rate() - get the clock rate
* @dpu_kms: pointer to dpu_kms structure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 37bba57675a8..54d74341e690 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -266,10 +266,6 @@ DEFINE_EVENT(dpu_drm_obj_template,
dpu_crtc_complete_commit,
TP_PROTO(uint32_t drm_id),
TP_ARGS(drm_id)
);
-DEFINE_EVENT(dpu_drm_obj_template, dpu_kms_enc_enable,
- TP_PROTO(uint32_t drm_id),
- TP_ARGS(drm_id)
-);
DEFINE_EVENT(dpu_drm_obj_template, dpu_kms_commit,
TP_PROTO(uint32_t drm_id),
TP_ARGS(drm_id)
Re: [Freedreno] [PATCH v5 13/21] drm/bridge: sn65dsi83: Fix bridge removal
On 10/21/21 9:39 AM, Maxime Ripard wrote:
Commit 24417d5b0c00 ("drm/bridge: ti-sn65dsi83: Implement .detach
callback") moved the unregistration of the bridge DSI device and bridge
itself to the detach callback.
While this is correct for the DSI device detach and unregistration, the
bridge is added in the driver probe, and should thus be removed as part
of its remove callback.
Cc: Marek Vasut
Fixes: 24417d5b0c00 ("drm/bridge: ti-sn65dsi83: Implement .detach callback")
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 52030a82f3e1..3bfd07caf8d7 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -297,7 +297,6 @@ static void sn65dsi83_detach(struct drm_bridge *bridge)
mipi_dsi_detach(ctx->dsi);
mipi_dsi_device_unregister(ctx->dsi);
- drm_bridge_remove(>bridge);
ctx->dsi = NULL;
}
@@ -693,6 +692,7 @@ static int sn65dsi83_remove(struct i2c_client *client)
{
struct sn65dsi83 *ctx = i2c_get_clientdata(client);
+ drm_bridge_remove(>bridge);
of_node_put(ctx->host_node);
return 0;
Yes, the above is correct, thanks.
Reviewed-by: Marek Vasut
Re: [Freedreno] Revert "arm64: dts: qcom: sm8250: remove bus clock from the mdss node for sm8250 target"
On Fri, 15 Oct 2021 at 02:53, Dmitry Baryshkov wrote: > > On Thu, 14 Oct 2021 at 19:54, Vladimir Zapolskiy > wrote: > > > > Hi Dmitry, > > > > On 10/14/21 4:54 PM, Dmitry Baryshkov wrote: > > > From: Amit Pundir > > > > > > This reverts commit 001ce9785c0674d913531345e86222c965fc8bf4. > > > > > > This upstream commit broke AOSP (post Android 12 merge) build > > > on RB5. The device either silently crashes into USB crash mode > > > after android boot animation or we see a blank blue screen > > > with following dpu errors in dmesg: > > > > > > [ T444] hw recovery is not complete for ctl:3 > > > [ T444] [drm:dpu_encoder_phys_vid_prepare_for_kickoff:539] [dpu > > > error]enc31 intf1 ctl 3 reset failure: -22 > > > [ T444] [drm:dpu_encoder_phys_vid_wait_for_commit_done:513] [dpu > > > error]vblank timeout > > > [ T444] [drm:dpu_kms_wait_for_commit_done:454] [dpu error]wait for > > > commit done returned -110 > > > [C7] [drm:dpu_encoder_frame_done_timeout:2127] [dpu error]enc31 frame > > > done timeout > > > [ T444] [drm:dpu_encoder_phys_vid_wait_for_commit_done:513] [dpu > > > error]vblank timeout > > > [ T444] [drm:dpu_kms_wait_for_commit_done:454] [dpu error]wait for > > > commit done returned -110 > > > > > > Signed-off-by: Amit Pundir > > > > your sob tag is missing. > > True. I hope this is fine: > > Signed-off-by: Dmitry Baryshkov > Hi, Any update on this? I'd really like to see this or a relevant fix to land in v5.15, because I can't boot AOSP on RB5 otherwise. Regards, Amit Pundir > > > > > --- > > > arch/arm64/boot/dts/qcom/sm8250.dtsi | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi > > > b/arch/arm64/boot/dts/qcom/sm8250.dtsi > > > index 8c15d9fed08f..d12e4cbfc852 100644 > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi > > > @@ -2590,9 +2590,10 @@ > > > power-domains = < MDSS_GDSC>; > > > > > > clocks = < DISP_CC_MDSS_AHB_CLK>, > > > + < GCC_DISP_HF_AXI_CLK>, > > >< GCC_DISP_SF_AXI_CLK>, > > >< DISP_CC_MDSS_MDP_CLK>; > > > - clock-names = "iface", "nrt_bus", "core"; > > > + clock-names = "iface", "bus", "nrt_bus", "core"; > > > > > > assigned-clocks = < DISP_CC_MDSS_MDP_CLK>; > > > assigned-clock-rates = <46000>; > > > > > > > -- > > Best wishes, > > Vladimir > > > > -- > With best wishes > Dmitry
Re: [Freedreno] [PATCH 2/2] drm/msm/dpu: Remove dynamic allocation from atomic context
On 10/22/2021 10:20 AM, Rob Clark wrote:
From: Rob Clark
We know the upper bound on # of mixers (ie. two), so lets just allocate
this on the stack.
Fixes:
BUG: sleeping function called from invalid context at
include/linux/sched/mm.h:201
in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/0
INFO: lockdep is turned off.
irq event stamp: 43642
hardirqs last enabled at (43641): []
cpuidle_enter_state+0x158/0x25c
hardirqs last disabled at (43642): []
enter_el1_irq_or_nmi+0x10/0x1c
softirqs last enabled at (43620): []
__do_softirq+0x1e4/0x464
softirqs last disabled at (43615): []
__irq_exit_rcu+0x104/0x150
CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW 5.15.0-rc3-debug+
#105
Hardware name: Google Lazor (rev1 - 2) with LTE (DT)
Call trace:
dump_backtrace+0x0/0x18c
show_stack+0x24/0x30
dump_stack_lvl+0xa0/0xd4
dump_stack+0x18/0x34
___might_sleep+0x1e0/0x1f0
__might_sleep+0x78/0x8c
slab_pre_alloc_hook.constprop.0+0x48/0x6c
__kmalloc+0xc8/0x21c
dpu_crtc_vblank_callback+0x158/0x1f8
dpu_encoder_vblank_callback+0x70/0xc4
dpu_encoder_phys_vid_vblank_irq+0x50/0x12c
dpu_core_irq+0x1bc/0x1d0
dpu_irq+0x1c/0x28
msm_irq+0x34/0x40
__handle_irq_event_percpu+0x15c/0x308
handle_irq_event_percpu+0x3c/0x90
handle_irq_event+0x54/0x98
handle_level_irq+0xa0/0xd0
handle_irq_desc+0x2c/0x44
generic_handle_domain_irq+0x28/0x34
dpu_mdss_irq+0x90/0xe8
handle_irq_desc+0x2c/0x44
handle_domain_irq+0x54/0x80
gic_handle_irq+0xd4/0x148
call_on_irq_stack+0x2c/0x54
do_interrupt_handler+0x4c/0x64
el1_interrupt+0x30/0xd0
el1h_64_irq_handler+0x18/0x24
el1h_64_irq+0x78/0x7c
arch_local_irq_enable+0xc/0x14
cpuidle_enter+0x44/0x5c
do_idle+0x248/0x268
cpu_startup_entry+0x30/0x48
rest_init+0x188/0x19c
arch_call_rest_init+0x1c/0x28
start_kernel+0x704/0x744
__primary_switched+0xc0/0xc8
Fixes: 78d9b458cc21 ("drm/msm/dpu: Add CRC support for DPU")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0ae397044310..80c0ae688734 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -182,21 +182,19 @@ static int dpu_crtc_get_crc(struct drm_crtc *crtc)
{
struct dpu_crtc_state *crtc_state;
struct dpu_crtc_mixer *m;
- u32 *crcs;
+ u32 crcs[CRTC_DUAL_MIXERS];
int i = 0;
int rc = 0;
crtc_state = to_dpu_crtc_state(crtc->state);
- crcs = kcalloc(crtc_state->num_mixers, sizeof(*crcs), GFP_KERNEL);
- if (!crcs)
- return -ENOMEM;
+ static_assert(ARRAY_SIZE(crcs) == ARRAY_SIZE(crtc_state->mixers));
Getting a C90 compiler warning for static_assert():
In file included from ./include/linux/bits.h:22,
from ./include/linux/bitops.h:6,
from ./include/linux/kernel.h:12,
from ./include/linux/list.h:9,
from ./include/linux/wait.h:7,
from ./include/linux/wait_bit.h:8,
from ./include/linux/fs.h:6,
from ./include/linux/debugfs.h:15,
from drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:10:
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c: In function
‘dpu_crtc_get_crc’:
./include/linux/build_bug.h:78:41: warning: ISO C90 forbids mixed
declarations and code [-Wdeclaration-after-statement]
78 | #define __static_assert(expr, msg, ...)
_Static_assert(expr, msg)
| ^~
./include/linux/build_bug.h:77:34: note: in expansion of macro
‘__static_assert’
77 | #define static_assert(expr, ...) __static_assert(expr,
##__VA_ARGS__, #expr)
| ^~~
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:192:2: note: in expansion
of macro ‘static_assert’
192 | static_assert(ARRAY_SIZE(crcs) ==
ARRAY_SIZE(crtc_state->mixers));
| ^
Can be fixed by moving the static_assert() before `crtc_state = ...`
Thanks,
Jessica Zhang
/* Skip first 2 frames in case of "uncooked" CRCs */
if (crtc_state->crc_frame_skip_count < 2) {
crtc_state->crc_frame_skip_count++;
- goto cleanup;
+ return 0;
}
for (i = 0; i < crtc_state->num_mixers; ++i) {
@@ -210,16 +208,12 @@ static int dpu_crtc_get_crc(struct drm_crtc *crtc)
if (rc) {
DRM_DEBUG_DRIVER("MISR read failed\n");
- goto cleanup;
+ return rc;
}
}
- rc = drm_crtc_add_crc_entry(crtc, true,
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: Remove impossible NULL check
On 10/22/2021 10:20 AM, Rob Clark wrote:
From: Rob Clark
Signed-off-by: Rob Clark
Reviewed-by: Jessica Zhang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index e91568d4f09a..0ae397044310 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -187,11 +187,6 @@ static int dpu_crtc_get_crc(struct drm_crtc *crtc)
int i = 0;
int rc = 0;
- if (!crtc) {
- DPU_ERROR("Invalid crtc\n");
- return -EINVAL;
- }
-
crtc_state = to_dpu_crtc_state(crtc->state);
crcs = kcalloc(crtc_state->num_mixers, sizeof(*crcs), GFP_KERNEL);
Re: [Freedreno] [PATCH] drm/msm/a5xx: Add support for Adreno 506 GPU
On Fri 22 Oct 04:43 PDT 2021, Vladimir Lypak wrote:
> This GPU is found on SoCs such as MSM8953(650MHz), SDM450(600MHz),
> SDM632(725MHz).
>
> Signed-off-by: Vladimir Lypak
> ---
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 34 ++
> drivers/gpu/drm/msm/adreno/adreno_device.c | 18
> drivers/gpu/drm/msm/adreno/adreno_gpu.h| 5
> 3 files changed, 45 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 5e2750eb3810..249a0d8bc673 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -441,7 +441,7 @@ void a5xx_set_hwcg(struct msm_gpu *gpu, bool state)
> const struct adreno_five_hwcg_regs *regs;
> unsigned int i, sz;
>
> - if (adreno_is_a508(adreno_gpu)) {
> + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu)) {
> regs = a50x_hwcg;
> sz = ARRAY_SIZE(a50x_hwcg);
> } else if (adreno_is_a509(adreno_gpu) || adreno_is_a512(adreno_gpu)) {
> @@ -485,7 +485,7 @@ static int a5xx_me_init(struct msm_gpu *gpu)
> OUT_RING(ring, 0x);
>
> /* Specify workarounds for various microcode issues */
> - if (adreno_is_a530(adreno_gpu)) {
> + if (adreno_is_a506(adreno_gpu) || adreno_is_a530(adreno_gpu)) {
> /* Workaround for token end syncs
>* Force a WFI after every direct-render 3D mode draw and every
>* 2D mode 3 draw
> @@ -620,8 +620,17 @@ static int a5xx_ucode_init(struct msm_gpu *gpu)
>
> static int a5xx_zap_shader_resume(struct msm_gpu *gpu)
> {
> + struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> int ret;
>
> + /*
> + * Adreno 506,508,512 have CPZ Retention feature and
> + * don't need to resume zap shader
> + */
> + if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) ||
> + adreno_is_a512(adreno_gpu))
> + return 0;
Afaict all other changes in the patch adds a506 support, but this hunk
changes a508 and a512 behavior.
I'm not saying that the change is wrong, but this hunk deserves to be in
it's own patch - so that if there's any impact on those other versions
it can be tracked down to that specific patch.
Thanks,
Bjorn
[Freedreno] [PATCH 2/2] drm/msm/dpu: Remove dynamic allocation from atomic context
From: Rob Clark
We know the upper bound on # of mixers (ie. two), so lets just allocate
this on the stack.
Fixes:
BUG: sleeping function called from invalid context at
include/linux/sched/mm.h:201
in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/0
INFO: lockdep is turned off.
irq event stamp: 43642
hardirqs last enabled at (43641): []
cpuidle_enter_state+0x158/0x25c
hardirqs last disabled at (43642): []
enter_el1_irq_or_nmi+0x10/0x1c
softirqs last enabled at (43620): []
__do_softirq+0x1e4/0x464
softirqs last disabled at (43615): []
__irq_exit_rcu+0x104/0x150
CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW 5.15.0-rc3-debug+
#105
Hardware name: Google Lazor (rev1 - 2) with LTE (DT)
Call trace:
dump_backtrace+0x0/0x18c
show_stack+0x24/0x30
dump_stack_lvl+0xa0/0xd4
dump_stack+0x18/0x34
___might_sleep+0x1e0/0x1f0
__might_sleep+0x78/0x8c
slab_pre_alloc_hook.constprop.0+0x48/0x6c
__kmalloc+0xc8/0x21c
dpu_crtc_vblank_callback+0x158/0x1f8
dpu_encoder_vblank_callback+0x70/0xc4
dpu_encoder_phys_vid_vblank_irq+0x50/0x12c
dpu_core_irq+0x1bc/0x1d0
dpu_irq+0x1c/0x28
msm_irq+0x34/0x40
__handle_irq_event_percpu+0x15c/0x308
handle_irq_event_percpu+0x3c/0x90
handle_irq_event+0x54/0x98
handle_level_irq+0xa0/0xd0
handle_irq_desc+0x2c/0x44
generic_handle_domain_irq+0x28/0x34
dpu_mdss_irq+0x90/0xe8
handle_irq_desc+0x2c/0x44
handle_domain_irq+0x54/0x80
gic_handle_irq+0xd4/0x148
call_on_irq_stack+0x2c/0x54
do_interrupt_handler+0x4c/0x64
el1_interrupt+0x30/0xd0
el1h_64_irq_handler+0x18/0x24
el1h_64_irq+0x78/0x7c
arch_local_irq_enable+0xc/0x14
cpuidle_enter+0x44/0x5c
do_idle+0x248/0x268
cpu_startup_entry+0x30/0x48
rest_init+0x188/0x19c
arch_call_rest_init+0x1c/0x28
start_kernel+0x704/0x744
__primary_switched+0xc0/0xc8
Fixes: 78d9b458cc21 ("drm/msm/dpu: Add CRC support for DPU")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0ae397044310..80c0ae688734 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -182,21 +182,19 @@ static int dpu_crtc_get_crc(struct drm_crtc *crtc)
{
struct dpu_crtc_state *crtc_state;
struct dpu_crtc_mixer *m;
- u32 *crcs;
+ u32 crcs[CRTC_DUAL_MIXERS];
int i = 0;
int rc = 0;
crtc_state = to_dpu_crtc_state(crtc->state);
- crcs = kcalloc(crtc_state->num_mixers, sizeof(*crcs), GFP_KERNEL);
- if (!crcs)
- return -ENOMEM;
+ static_assert(ARRAY_SIZE(crcs) == ARRAY_SIZE(crtc_state->mixers));
/* Skip first 2 frames in case of "uncooked" CRCs */
if (crtc_state->crc_frame_skip_count < 2) {
crtc_state->crc_frame_skip_count++;
- goto cleanup;
+ return 0;
}
for (i = 0; i < crtc_state->num_mixers; ++i) {
@@ -210,16 +208,12 @@ static int dpu_crtc_get_crc(struct drm_crtc *crtc)
if (rc) {
DRM_DEBUG_DRIVER("MISR read failed\n");
- goto cleanup;
+ return rc;
}
}
- rc = drm_crtc_add_crc_entry(crtc, true,
+ return drm_crtc_add_crc_entry(crtc, true,
drm_crtc_accurate_vblank_count(crtc), crcs);
-
-cleanup:
- kfree(crcs);
- return rc;
}
static bool dpu_crtc_get_scanout_position(struct drm_crtc *crtc,
--
2.31.1
[Freedreno] [PATCH 1/2] drm/msm/dpu: Remove impossible NULL check
From: Rob Clark
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index e91568d4f09a..0ae397044310 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -187,11 +187,6 @@ static int dpu_crtc_get_crc(struct drm_crtc *crtc)
int i = 0;
int rc = 0;
- if (!crtc) {
- DPU_ERROR("Invalid crtc\n");
- return -EINVAL;
- }
-
crtc_state = to_dpu_crtc_state(crtc->state);
crcs = kcalloc(crtc_state->num_mixers, sizeof(*crcs), GFP_KERNEL);
--
2.31.1
[Freedreno] [PATCH] drm/msm/a5xx: Add support for Adreno 506 GPU
This GPU is found on SoCs such as MSM8953(650MHz), SDM450(600MHz),
SDM632(725MHz).
Signed-off-by: Vladimir Lypak
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 34 ++
drivers/gpu/drm/msm/adreno/adreno_device.c | 18
drivers/gpu/drm/msm/adreno/adreno_gpu.h| 5
3 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 5e2750eb3810..249a0d8bc673 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -441,7 +441,7 @@ void a5xx_set_hwcg(struct msm_gpu *gpu, bool state)
const struct adreno_five_hwcg_regs *regs;
unsigned int i, sz;
- if (adreno_is_a508(adreno_gpu)) {
+ if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu)) {
regs = a50x_hwcg;
sz = ARRAY_SIZE(a50x_hwcg);
} else if (adreno_is_a509(adreno_gpu) || adreno_is_a512(adreno_gpu)) {
@@ -485,7 +485,7 @@ static int a5xx_me_init(struct msm_gpu *gpu)
OUT_RING(ring, 0x);
/* Specify workarounds for various microcode issues */
- if (adreno_is_a530(adreno_gpu)) {
+ if (adreno_is_a506(adreno_gpu) || adreno_is_a530(adreno_gpu)) {
/* Workaround for token end syncs
* Force a WFI after every direct-render 3D mode draw and every
* 2D mode 3 draw
@@ -620,8 +620,17 @@ static int a5xx_ucode_init(struct msm_gpu *gpu)
static int a5xx_zap_shader_resume(struct msm_gpu *gpu)
{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
int ret;
+ /*
+* Adreno 506,508,512 have CPZ Retention feature and
+* don't need to resume zap shader
+*/
+ if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) ||
+ adreno_is_a512(adreno_gpu))
+ return 0;
+
ret = qcom_scm_set_remote_state(SCM_GPU_ZAP_SHADER_RESUME, GPU_PAS_ID);
if (ret)
DRM_ERROR("%s: zap-shader resume failed: %d\n",
@@ -733,9 +742,10 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
0x0010 + adreno_gpu->gmem - 1);
gpu_write(gpu, REG_A5XX_UCHE_GMEM_RANGE_MAX_HI, 0x);
- if (adreno_is_a508(adreno_gpu) || adreno_is_a510(adreno_gpu)) {
+ if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) ||
+ adreno_is_a510(adreno_gpu)) {
gpu_write(gpu, REG_A5XX_CP_MEQ_THRESHOLDS, 0x20);
- if (adreno_is_a508(adreno_gpu))
+ if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu))
gpu_write(gpu, REG_A5XX_CP_MERCIU_SIZE, 0x400);
else
gpu_write(gpu, REG_A5XX_CP_MERCIU_SIZE, 0x20);
@@ -751,7 +761,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
gpu_write(gpu, REG_A5XX_CP_ROQ_THRESHOLDS_1, 0x40201B16);
}
- if (adreno_is_a508(adreno_gpu))
+ if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu))
gpu_write(gpu, REG_A5XX_PC_DBG_ECO_CNTL,
(0x100 << 11 | 0x100 << 22));
else if (adreno_is_a509(adreno_gpu) || adreno_is_a510(adreno_gpu) ||
@@ -769,8 +779,8 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
* Disable the RB sampler datapath DP2 clock gating optimization
* for 1-SP GPUs, as it is enabled by default.
*/
- if (adreno_is_a508(adreno_gpu) || adreno_is_a509(adreno_gpu) ||
- adreno_is_a512(adreno_gpu))
+ if (adreno_is_a506(adreno_gpu) || adreno_is_a508(adreno_gpu) ||
+ adreno_is_a509(adreno_gpu) || adreno_is_a512(adreno_gpu))
gpu_rmw(gpu, REG_A5XX_RB_DBG_ECO_CNTL, 0, (1 << 9));
/* Disable UCHE global filter as SP can invalidate/flush independently
*/
@@ -895,8 +905,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
if (ret)
return ret;
- if (!(adreno_is_a508(adreno_gpu) || adreno_is_a509(adreno_gpu) ||
- adreno_is_a510(adreno_gpu) || adreno_is_a512(adreno_gpu)))
+ if (adreno_is_a530(adreno_gpu) || adreno_is_a540(adreno_gpu))
a5xx_gpmu_ucode_init(gpu);
ret = a5xx_ucode_init(gpu);
@@ -1338,7 +1347,7 @@ static int a5xx_pm_resume(struct msm_gpu *gpu)
if (ret)
return ret;
- /* Adreno 508, 509, 510, 512 needs manual RBBM sus/res control */
+ /* Adreno 506, 508, 509, 510, 512 needs manual RBBM sus/res control */
if (!(adreno_is_a530(adreno_gpu) || adreno_is_a540(adreno_gpu))) {
/* Halt the sp_input_clk at HM level */
gpu_write(gpu, REG_A5XX_RBBM_CLOCK_CNTL, 0x0055);
@@ -1381,8 +1390,9 @@ static int a5xx_pm_suspend(struct msm_gpu *gpu)
u32 mask = 0xf;
int i, ret;
- /* A508, A510 have 3 XIN ports in VBIF */
- if (adreno_is_a508(adreno_gpu) || adreno_is_a510(adreno_gpu))
+
[Freedreno] [PATCH] drm/msm/a5xx: Eliminate condition on setup of SMMU CP_PROTECT
Only GPU that has larger SMMU region size (0x8000 dwords) is A530.
All other GPUs have 0x4000 SMMU region. However those GPUs work
correctly with larger range protected because there is no known
registers after SMMU region.
This patch needs to be backported to stable releases because A540 GPU
was forgotten to get its branch (that would set up protected region of
0x4000 dwords).
Fixes: b5f103ab98c7 ("drm/msm: gpu: Add A5XX target support")
Signed-off-by: Vladimir Lypak
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 5e2750eb3810..ecf6318a247f 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -851,11 +851,8 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
/* UCHE */
gpu_write(gpu, REG_A5XX_CP_PROTECT(16), ADRENO_PROTECT_RW(0xE80, 16));
- if (adreno_is_a508(adreno_gpu) || adreno_is_a509(adreno_gpu) ||
- adreno_is_a510(adreno_gpu) || adreno_is_a512(adreno_gpu) ||
- adreno_is_a530(adreno_gpu))
- gpu_write(gpu, REG_A5XX_CP_PROTECT(17),
- ADRENO_PROTECT_RW(0x1, 0x8000));
+ /* SMMU */
+ gpu_write(gpu, REG_A5XX_CP_PROTECT(17), ADRENO_PROTECT_RW(0x1,
0x8000));
gpu_write(gpu, REG_A5XX_RBBM_SECVID_TSB_CNTL, 0);
/*
--
2.33.0
Re: [Freedreno] [Intel-gfx] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, Oct 22, 2021 at 03:01:52PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 22, 2021 at 12:25:33PM +0200, Claudio Suarez wrote:
> > On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote:
> > > On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
> > > > drm_get_edid() internally calls to drm_connector_update_edid_property()
> > > > and then drm_add_display_info(), which parses the EDID.
> > > > This happens in the function intel_hdmi_set_edid() and
> > > > intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
> > > >
> > > > Once EDID is parsed, the monitor HDMI support information is available
> > > > through drm_display_info.is_hdmi. Retriving the same information with
> > > > drm_detect_hdmi_monitor() is less efficient. Change to
> > > > drm_display_info.is_hdmi
> > >
> > > I meant we need to examine all call chains that can lead to
> > > .detect() to make sure all of them do in fact update the
> > > display_info beforehand.
> >
> > Well, I studied it carefully and, yes, all call chains that can lead to
> > drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info
> > beforehand. In the case that this doesn't happen, the code is unchanged.
> >
> > Do you want I explain the changes in the code here again ? Or do you want
> > to me change the commit message to be more clear ? In the first case, I can
> > write here a detailed explanation. In the second case I can make a longer
> > commit
> > message.
> >
> > Or both?
>
> I want all those call chains explained in the commit message,
> otherwise I have no easy way to confirm whether the change
> is correct or not.
Hmm. OK, so I had a bit of a dig around and seems that what we do now
.detect()->drm_get_edid()->drm_connector_update_edid_property()->drm_add_display_info()
Now the question is when did that start happening? Looks like it was
commit 4b4df570b41d ("drm: Update edid-derived drm_display_info fields
at edid property set [v2]") that started to call drm_add_display_info()
from drm_connector_update_edid_property(), and then commit 5186421cbfe2
("drm: Introduce epoch counter to drm_connector") started to call
drm_connector_update_edid_property() from drm_get_edid(). Before both
of those commits were in place display_info would still contain
some stale garbage during .detect().
That is the story I think we want in these commit messages since it
a) explains why the old code was directly parsing the edid instead
b) why it's now safe to change this
PS. connector->force handling in drm_get_edid() looks a bit busted
since it doesn't call drm_connector_update_edid_property() at all
in some cases. I think there might be some path that leads there
anywya if/when we change connector->force, but we should fix
drm_get_edid() to do the right thing regarless.
--
Ville Syrjälä
Intel
Re: [Freedreno] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, Oct 22, 2021 at 12:25:33PM +0200, Claudio Suarez wrote: > On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote: > > On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote: > > > drm_get_edid() internally calls to drm_connector_update_edid_property() > > > and then drm_add_display_info(), which parses the EDID. > > > This happens in the function intel_hdmi_set_edid() and > > > intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()). > > > > > > Once EDID is parsed, the monitor HDMI support information is available > > > through drm_display_info.is_hdmi. Retriving the same information with > > > drm_detect_hdmi_monitor() is less efficient. Change to > > > drm_display_info.is_hdmi > > > > I meant we need to examine all call chains that can lead to > > .detect() to make sure all of them do in fact update the > > display_info beforehand. > > Well, I studied it carefully and, yes, all call chains that can lead to > drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info > beforehand. In the case that this doesn't happen, the code is unchanged. > > Do you want I explain the changes in the code here again ? Or do you want > to me change the commit message to be more clear ? In the first case, I can > write here a detailed explanation. In the second case I can make a longer > commit > message. > > Or both? I want all those call chains explained in the commit message, otherwise I have no easy way to confirm whether the change is correct or not. -- Ville Syrjälä Intel
Re: [Freedreno] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote: > On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote: > > drm_get_edid() internally calls to drm_connector_update_edid_property() > > and then drm_add_display_info(), which parses the EDID. > > This happens in the function intel_hdmi_set_edid() and > > intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()). > > > > Once EDID is parsed, the monitor HDMI support information is available > > through drm_display_info.is_hdmi. Retriving the same information with > > drm_detect_hdmi_monitor() is less efficient. Change to > > drm_display_info.is_hdmi > > I meant we need to examine all call chains that can lead to > .detect() to make sure all of them do in fact update the > display_info beforehand. Well, I studied it carefully and, yes, all call chains that can lead to drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info beforehand. In the case that this doesn't happen, the code is unchanged. Do you want I explain the changes in the code here again ? Or do you want to me change the commit message to be more clear ? In the first case, I can write here a detailed explanation. In the second case I can make a longer commit message. Or both? Best Regards, Claudio Suarez.
Re: [Freedreno] [PATCH 11/11] drm/msm/dpu: rip out debugfs support from dpu_plane
Hi,
On Fri, 22 Oct 2021 at 02:53, wrote:
>
> On 2021-09-30 07:00, Dmitry Baryshkov wrote:
> > In preparations of virtualizing the dpu_plane rip out debugfs support
> > from dpu_plane (as it is mostly used to expose plane's pipe registers).
> > Also move disable_danger file to danger/ debugfs subdir where it
> > belongs.
> >
> > Signed-off-by: Dmitry Baryshkov
>
> I am yet to review the second part of the virtual plane series to
> understand why this removal
> is necessary so I might be missing something.
See below
>
> The plane's debugfs holds useful information to check a few things
> rightaway.
>
> So if there is some misconfiguration or corruption in addition to the
> plane state,
> this is good to check.
>
> localhost /sys/kernel/debug/dri/1/plane35 # cat src_blk
> [4000] 03000556 03000556
> [4010] 00414000 0040e000
> [4020] 1600 0080
> [4030] 800236ff 03010002 8001
> [4040] 0030 000c0087 0707
> [4050]
> [4060] 0001
> [4070] 44556677 00112233
> [4080]
> [4090]
> [40a0] 00414000 0040e000
> [40b0]
> [40c0]
> [40d0] 0003f820
> [40e0] 0003e2c4
> [40f0] 000f000f 00010330 02e402e4
> [4100] 03000556
> [4110] 03000556
> [4120] 03000556
> [4130] 000f 000f
> [4140]
>
> So I would like to keep this functionality unless there is some
> compelling reason
> to remove this.
Ok, I'll take a look if I can keep it or rework somehow.
The problem is that if you have the plane is virtual, there is no
strong plane <-> sspp mapping. Consider wide planes, where you'd have
two SSPPs per single plane.
I think it would make sense to move src_blk to global namespace (as
ssppN_src) and then add a file giving plane -> sspp relationship.
>
> BTW, are you going to submit the second half as a new series now that
> most
> of the first one has been reviewed?
>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 123
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 69 -
> > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 171 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 6 +
> > 4 files changed, 69 insertions(+), 300 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index ae48f41821cf..fe33273cdf57 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -101,84 +101,85 @@ static int dpu_debugfs_safe_stats_show(struct
> > seq_file *s, void *v)
> > }
> > DEFINE_SHOW_ATTRIBUTE(dpu_debugfs_safe_stats);
> >
> > -static void dpu_debugfs_danger_init(struct dpu_kms *dpu_kms,
> > - struct dentry *parent)
> > +static ssize_t _dpu_plane_danger_read(struct file *file,
> > + char __user *buff, size_t count, loff_t *ppos)
> > {
> > - struct dentry *entry = debugfs_create_dir("danger", parent);
> > + struct dpu_kms *kms = file->private_data;
> > + int len;
> > + char buf[40];
> >
> > - debugfs_create_file("danger_status", 0600, entry,
> > - dpu_kms, _debugfs_danger_stats_fops);
> > - debugfs_create_file("safe_status", 0600, entry,
> > - dpu_kms, _debugfs_safe_stats_fops);
> > + len = scnprintf(buf, sizeof(buf), "%d\n", !kms->has_danger_ctrl);
> > +
> > + return simple_read_from_buffer(buff, count, ppos, buf, len);
> > }
> >
> > -static int _dpu_debugfs_show_regset32(struct seq_file *s, void *data)
> > +static void _dpu_plane_set_danger_state(struct dpu_kms *kms, bool
> > enable)
> > {
> > - struct dpu_debugfs_regset32 *regset = s->private;
> > - struct dpu_kms *dpu_kms = regset->dpu_kms;
> > - void __iomem *base;
> > - uint32_t i, addr;
> > -
> > - if (!dpu_kms->mmio)
> > - return 0;
> > -
> > - base = dpu_kms->mmio + regset->offset;
> > -
> > - /* insert padding spaces, if needed */
> > - if (regset->offset & 0xF) {
> > - seq_printf(s, "[%x]", regset->offset & ~0xF);
> > - for (i = 0; i < (regset->offset & 0xF); i += 4)
> > - seq_puts(s, " ");
> > - }
> > -
> > - pm_runtime_get_sync(_kms->pdev->dev);
> > -
> > - /* main register output */
> > - for (i = 0; i < regset->blk_len; i += 4) {
> > - addr = regset->offset + i;
> > - if ((addr & 0xF) == 0x0)
> > - seq_printf(s, i ? "\n[%x]" : "[%x]", addr);
> > - seq_printf(s, " %08x", readl_relaxed(base + i));
