On 8/9/24 15:42, Hamza Mahfooz wrote:
We don't actually need to request that the compositor does a full
modeset to modify the panel power savings level, we can instead
just make a request to DMUB, to set the new level dynamically.

Cc: Harry Wentland <harry.wentl...@amd.com>
Cc: Leo Li <sunpeng...@amd.com>
Cc: Mario Limonciello <mario.limoncie...@amd.com>
Cc: Sebastian Wick <sebast...@sebastianwick.net>
Signed-off-by: Hamza Mahfooz <hamza.mahf...@amd.com>
---

Thanks, this will solve the side effects that users of GNOME shell were seeing when the attribute was modified.

I tested it on an applicable laptop running 6.11-rc4 and it works correctly. I have one nit below, but feel free to ignore it if you don't agree.

Here's some tags:

Tested-by: Mario Limonciello <mario.limoncie...@amd.com>
Reviewed-by: Mario Limonciello <mario.limoncie...@amd.com>
Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/3578

  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 17 +++++++-
  drivers/gpu/drm/amd/display/dc/core/dc.c      | 39 +++++++++++--------
  drivers/gpu/drm/amd/display/dc/dc.h           |  2 +
  3 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index dd8353283bda..00a8a5959aa9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6819,9 +6819,14 @@ static ssize_t panel_power_savings_store(struct device 
*device,
                                         const char *buf, size_t count)
  {
        struct drm_connector *connector = dev_get_drvdata(device);
+       struct amdgpu_dm_connector *aconn = to_amdgpu_dm_connector(connector);
        struct drm_device *dev = connector->dev;
+       struct amdgpu_device *adev = drm_to_adev(dev);
+       struct dc *dc = adev->dm.dc;
+       struct pipe_ctx *pipe_ctx;
        long val;
        int ret;
+       int i;
ret = kstrtol(buf, 0, &val); @@ -6836,7 +6841,17 @@ static ssize_t panel_power_savings_store(struct device *device,
                ABM_LEVEL_IMMEDIATE_DISABLE;
        drm_modeset_unlock(&dev->mode_config.connection_mutex);
- drm_kms_helper_hotplug_event(dev);
+       mutex_lock(&adev->dm.dc_lock);
+       for (i = 0; i < dc->res_pool->pipe_count; i++) {
+               pipe_ctx = &dc->current_state->res_ctx.pipe_ctx[i];
+
+               if (pipe_ctx->stream &&
+                   pipe_ctx->stream->link == aconn->dc_link) {
+                       dc_set_abm_level(dc, pipe_ctx, val);
+                       break;
+               }
+       }
+       mutex_unlock(&adev->dm.dc_lock);
return count;
  }
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 3ba2acfdae2a..60081cd978b7 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -3319,6 +3319,23 @@ static bool update_planes_and_stream_state(struct dc *dc,
} +void dc_set_abm_level(struct dc *dc, struct pipe_ctx *pipe_ctx, int level)
+{
+       struct timing_generator *tg = pipe_ctx->stream_res.tg;
+       struct abm *abm = pipe_ctx->stream_res.abm;
+
+       if (!abm)
+               return;

AFAICT this is a programmer error if this was to actually happen.
I'd think a WARN_ON() makes sense here.

+
+       if (tg->funcs->is_blanked && !tg->funcs->is_blanked(tg))
+               tg->funcs->wait_for_state(tg, CRTC_STATE_VBLANK);
+
+       if (level == ABM_LEVEL_IMMEDIATE_DISABLE)
+               dc->hwss.set_abm_immediate_disable(pipe_ctx);
+       else
+               abm->funcs->set_abm_level(abm, level);
+}
+
  static void commit_planes_do_stream_update(struct dc *dc,
                struct dc_stream_state *stream,
                struct dc_stream_update *stream_update,
@@ -3447,22 +3464,12 @@ static void commit_planes_do_stream_update(struct dc 
*dc,
                                dc->link_srv->set_dpms_on(dc->current_state, 
pipe_ctx);
                        }
- if (stream_update->abm_level && pipe_ctx->stream_res.abm) {
-                               bool should_program_abm = true;
-
-                               // if otg funcs defined check if blanked before 
programming
-                               if (pipe_ctx->stream_res.tg->funcs->is_blanked)
-                                       if 
(pipe_ctx->stream_res.tg->funcs->is_blanked(pipe_ctx->stream_res.tg))
-                                               should_program_abm = false;
-
-                               if (should_program_abm) {
-                                       if (*stream_update->abm_level == 
ABM_LEVEL_IMMEDIATE_DISABLE) {
-                                               
dc->hwss.set_abm_immediate_disable(pipe_ctx);
-                                       } else {
-                                               
pipe_ctx->stream_res.abm->funcs->set_abm_level(
-                                                       pipe_ctx->stream_res.abm, 
stream->abm_level);
-                                       }
-                               }
+                       if (stream_update->abm_level) {
+                               dc_set_abm_level(dc, pipe_ctx,
+                                                *stream_update->abm_level ==
+                                                ABM_LEVEL_IMMEDIATE_DISABLE ?
+                                                ABM_LEVEL_IMMEDIATE_DISABLE :
+                                                stream->abm_level);
                        }
                }
        }
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index 7873daf72608..134ef00d9668 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -2494,6 +2494,8 @@ void dc_z10_save_init(struct dc *dc);
  bool dc_is_dmub_outbox_supported(struct dc *dc);
  bool dc_enable_dmub_notifications(struct dc *dc);
+void dc_set_abm_level(struct dc *dc, struct pipe_ctx *pipe_ctx, int level);
+
  bool dc_abm_save_restore(
                struct dc *dc,
                struct dc_stream_state *stream,

Reply via email to