[PATCH v3 2/2] drm/amd: Add power_saving_policy drm property to eDP connectors

Mario Limonciello mario.limonciello at amd.com
Wed Jun 19 04:08:50 UTC 2024


On 6/18/2024 17:36, Leo Li wrote:
> 
> 
> On 2024-06-05 22:04, Mario Limonciello wrote:
>> When the `power_saving_policy` property is set to bit mask
>> "Require color accuracy" ABM should be disabled immediately and
>> any requests by sysfs to update will return an -EBUSY error.
>>
>> When the `power_saving_policy` property is set to bit mask
>> "Require low latency" PSR should be disabled.
>>
>> When the property is restored to an empty bit mask the previous
>> value of ABM and pSR will be restored.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> Reviewed-by: Leo Li <sunpeng.li at amd.com>

Thanks!  I don't have permissions, so can you (or someone else) please 
apply to drm-misc-next for me?

After it's merged I'll rebase and work on the feedback for the new IGT 
tests.

> 
> Thanks!
> 
>> ---
>> v2->v3:
>>   * Use `disallow_edp_enter_psr` instead
>>   * Drop case in dm_update_crtc_state()
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  4 ++
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +++++++++++++++++--
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +
>>   3 files changed, 51 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> index 3ecc7ef95172..cfb5220cf182 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> @@ -1350,6 +1350,10 @@ int amdgpu_display_modeset_create_props(struct 
>> amdgpu_device *adev)
>>                        "dither",
>>                        amdgpu_dither_enum_list, sz);
>> +    if (adev->dc_enabled)
>> +        drm_mode_create_power_saving_policy_property(adev_to_drm(adev),
>> +                                 DRM_MODE_POWER_SAVING_POLICY_ALL);
>> +
>>       return 0;
>>   }
>> 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 f1d67c6f4b98..5fd7210b2479 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -6436,6 +6436,13 @@ int 
>> amdgpu_dm_connector_atomic_set_property(struct drm_connector *connector,
>>       } else if (property == adev->mode_info.underscan_property) {
>>           dm_new_state->underscan_enable = val;
>>           ret = 0;
>> +    } else if (property == dev->mode_config.power_saving_policy) {
>> +        dm_new_state->abm_forbidden = val & 
>> DRM_MODE_REQUIRE_COLOR_ACCURACY;
>> +        dm_new_state->abm_level = (dm_new_state->abm_forbidden || 
>> !amdgpu_dm_abm_level) ?
>> +                        ABM_LEVEL_IMMEDIATE_DISABLE :
>> +                        amdgpu_dm_abm_level;
>> +        dm_new_state->psr_forbidden = val & 
>> DRM_MODE_REQUIRE_LOW_LATENCY;
>> +        ret = 0;
>>       }
>>       return ret;
>> @@ -6478,6 +6485,13 @@ int 
>> amdgpu_dm_connector_atomic_get_property(struct drm_connector *connector,
>>       } else if (property == adev->mode_info.underscan_property) {
>>           *val = dm_state->underscan_enable;
>>           ret = 0;
>> +    } else if (property == dev->mode_config.power_saving_policy) {
>> +        *val = 0;
>> +        if (dm_state->psr_forbidden)
>> +            *val |= DRM_MODE_REQUIRE_LOW_LATENCY;
>> +        if (dm_state->abm_forbidden)
>> +            *val |= DRM_MODE_REQUIRE_COLOR_ACCURACY;
>> +        ret = 0;
>>       }
>>       return ret;
>> @@ -6504,9 +6518,12 @@ static ssize_t panel_power_savings_show(struct 
>> device *device,
>>       u8 val;
>>       drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>> -    val = to_dm_connector_state(connector->state)->abm_level ==
>> -        ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
>> -        to_dm_connector_state(connector->state)->abm_level;
>> +    if (to_dm_connector_state(connector->state)->abm_forbidden)
>> +        val = 0;
>> +    else
>> +        val = to_dm_connector_state(connector->state)->abm_level ==
>> +            ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
>> +            to_dm_connector_state(connector->state)->abm_level;
>>       drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>       return sysfs_emit(buf, "%u\n", val);
>> @@ -6530,10 +6547,16 @@ static ssize_t 
>> panel_power_savings_store(struct device *device,
>>           return -EINVAL;
>>       drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>> -    to_dm_connector_state(connector->state)->abm_level = val ?:
>> -        ABM_LEVEL_IMMEDIATE_DISABLE;
>> +    if (to_dm_connector_state(connector->state)->abm_forbidden)
>> +        ret = -EBUSY;
>> +    else
>> +        to_dm_connector_state(connector->state)->abm_level = val ?:
>> +            ABM_LEVEL_IMMEDIATE_DISABLE;
>>       drm_modeset_unlock(&dev->mode_config.connection_mutex);
>> +    if (ret)
>> +        return ret;
>> +
>>       drm_kms_helper_hotplug_event(dev);
>>       return count;
>> @@ -7704,6 +7727,13 @@ void amdgpu_dm_connector_init_helper(struct 
>> amdgpu_display_manager *dm,
>>       aconnector->base.state->max_bpc = 16;
>>       aconnector->base.state->max_requested_bpc = 
>> aconnector->base.state->max_bpc;
>> +    if (connector_type == DRM_MODE_CONNECTOR_eDP &&
>> +        (dc_is_dmcu_initialized(adev->dm.dc) ||
>> +         adev->dm.dc->ctx->dmub_srv)) {
>> +        drm_object_attach_property(&aconnector->base.base,
>> +                dm->ddev->mode_config.power_saving_policy, 0);
>> +    }
>> +
>>       if (connector_type == DRM_MODE_CONNECTOR_HDMIA) {
>>           /* Content Type is currently only implemented for HDMI. */
>>           drm_connector_attach_content_type_property(&aconnector->base);
>> @@ -9307,6 +9337,7 @@ static void amdgpu_dm_atomic_commit_tail(struct 
>> drm_atomic_state *state)
>>       for_each_oldnew_connector_in_state(state, connector, 
>> old_con_state, new_con_state, i) {
>>           struct dm_connector_state *dm_new_con_state = 
>> to_dm_connector_state(new_con_state);
>>           struct dm_connector_state *dm_old_con_state = 
>> to_dm_connector_state(old_con_state);
>> +        struct amdgpu_dm_connector *aconnector = 
>> to_amdgpu_dm_connector(connector);
>>           struct amdgpu_crtc *acrtc = 
>> to_amdgpu_crtc(dm_new_con_state->base.crtc);
>>           struct dc_surface_update *dummy_updates;
>>           struct dc_stream_update stream_update;
>> @@ -9360,6 +9391,15 @@ static void amdgpu_dm_atomic_commit_tail(struct 
>> drm_atomic_state *state)
>>               stream_update.hdr_static_metadata = &hdr_packet;
>>           }
>> +        aconnector->disallow_edp_enter_psr = 
>> dm_new_con_state->psr_forbidden;
>> +
>> +        /* immediately disable PSR if disallowed */
>> +        if (aconnector->disallow_edp_enter_psr) {
>> +            mutex_lock(&dm->dc_lock);
>> +            amdgpu_dm_psr_disable(dm_new_crtc_state->stream);
>> +            mutex_unlock(&dm->dc_lock);
>> +        }
>> +
>>           status = dc_stream_get_status(dm_new_crtc_state->stream);
>>           if (WARN_ON(!status))
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>> index 09519b7abf67..b246e82f5b0d 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>> @@ -873,6 +873,8 @@ struct dm_connector_state {
>>       bool underscan_enable;
>>       bool freesync_capable;
>>       bool update_hdcp;
>> +    bool abm_forbidden;
>> +    bool psr_forbidden;
>>       uint8_t abm_level;
>>       int vcpi_slots;
>>       uint64_t pbn;



More information about the dri-devel mailing list