On 2/26/2024 7:50 PM, Jani Nikula wrote:
On Mon, 26 Feb 2024, "Nautiyal, Ankit K" <ankit.k.nautiyal@xxxxxxxxx> wrote:
On 2/22/2024 11:27 AM, Golani, Mitulkumar Ajitkumar wrote:
-----Original Message-----
From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
Bhanuprakash Modem
Sent: Wednesday, February 21, 2024 4:42 PM
To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Modem, Bhanuprakash <bhanuprakash.modem@xxxxxxxxx>
Subject: [PATCH] drm/i915/display/debugfs: New entry "DRRS capable" to
i915_drrs_status
If the connected panel supports both DRRS & PSR, driver gives preference to
PSR ("DRRS enabled: no"). Even though the hardware supports DRRS, IGT
treats ("DRRS enabled: yes") as not capable.
Introduce a new entry "DRRS capable" to debugfs i915_drrs_status, so that
IGT will read the DRRS capability as "DRRS capable: yes".
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@xxxxxxxxx>
---
drivers/gpu/drm/i915/display/intel_drrs.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c
b/drivers/gpu/drm/i915/display/intel_drrs.c
index 6282ec0fc9b4..169ef38ff188 100644
--- a/drivers/gpu/drm/i915/display/intel_drrs.c
+++ b/drivers/gpu/drm/i915/display/intel_drrs.c
@@ -299,6 +299,7 @@ void intel_drrs_crtc_init(struct intel_crtc *crtc) static
int intel_drrs_debugfs_status_show(struct seq_file *m, void *unused) {
struct intel_crtc *crtc = m->private;
+ struct drm_i915_private *i915 = to_i915(crtc->base.dev);
const struct intel_crtc_state *crtc_state;
int ret;
@@ -310,6 +311,11 @@ static int intel_drrs_debugfs_status_show(struct
seq_file *m, void *unused)
mutex_lock(&crtc->drrs.mutex);
+ seq_printf(m, "DRRS capable: %s\n",
+ str_yes_no(crtc_state->has_drrs ||
+ HAS_DOUBLE_BUFFERED_M_N(i915) ||
+ intel_cpu_transcoder_has_m2_n2(i915,
+crtc_state->cpu_transcoder)));
Why would "capability" look at ->has_drrs?
Why didn't anyone question the duplication of the conditions of what
"drrs capable" means?
And what *does* "drrs capable" mean here anyway? That the platform is
capable? But what if the display isn't capable?
As I understand, for the display there is another debugfs for connector
i915_drrs_type which can be none, seamless, static.
Here drrs capable means whether the platform is capable, which depends
on cpu_transcoder_has_drrs() which in turn depends on
HAS_DOUBLE_BUFFERED_M_N and intel_cpu_transcoder_has_m2_n2.
Regards,
Ankit
BR,
Jani.
+
Adding DRRS capable property to debugfs.
Change LGTM
Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani@xxxxxxxxx>
Thanks for the patch and review. Pushed to drm-intel-next.
Regards,
Ankit
seq_printf(m, "DRRS enabled: %s\n",
str_yes_no(crtc_state->has_drrs));
--
2.43.0