Re: [PATCH 1/2] drm/amd/display: Switch the custom "max bpc" property to the DRM prop
On 5/22/19 11:11 AM, Nicholas Kazlauskas wrote: > [CAUTION: External Email] > > [Why] > The custom "max bpc" property was added to limit color depth while the > DRM one was still being merged. It's been a few kernel versions since > then and this TODO was still sticking around. > > [How] > Attach the DRM max bpc property to the connector and drop all of our > custom property management. Set the max bpc to 8 by default since > DRM defaults to the max in the range which would be 16 in this case. > > No behavioral changes are intended with this patch, it should just be > a refactor. > > Cc: Leo Li > Cc: Harry Wentland > Signed-off-by: Nicholas Kazlauskas > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 --- > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 -- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 31 --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 - > 4 files changed, 13 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > index 4e944737b708..767ee6991ef4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c > @@ -631,10 +631,6 @@ int amdgpu_display_modeset_create_props(struct > amdgpu_device *adev) > amdgpu_dither_enum_list, sz); > > if (amdgpu_device_has_dc_support(adev)) { > - adev->mode_info.max_bpc_property = > - drm_property_create_range(adev->ddev, 0, "max bpc", > 8, 16); > - if (!adev->mode_info.max_bpc_property) > - return -ENOMEM; > adev->mode_info.abm_level_property = > drm_property_create_range(adev->ddev, 0, > "abm level", 0, 4); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > index c7940af42f76..8bda00ce8816 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > @@ -332,8 +332,6 @@ struct amdgpu_mode_info { > struct drm_property *audio_property; > /* FMT dithering */ > struct drm_property *dither_property; > - /* maximum number of bits per channel for monitor color */ > - struct drm_property *max_bpc_property; > /* Adaptive Backlight Modulation (power feature) */ > struct drm_property *abm_level_property; > /* it is used to allow enablement of freesync mode */ > 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 4a1755bce96c..a7a9e4d81a17 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3040,14 +3040,14 @@ static void update_stream_scaling_settings(const > struct drm_display_mode *mode, > static enum dc_color_depth > convert_color_depth_from_display_info(const struct drm_connector *connector) > { > - struct dm_connector_state *dm_conn_state = > - to_dm_connector_state(connector->state); > - uint32_t bpc = connector->display_info.bpc; > + uint32_t bpc = 8; Actually, there is a behavioral state change here. This should still be: bpc = connector->display_info.bpc; I'll fix up this and drop that extra TODO left over in patch 2 and post a v2. Nicholas Kazlauskas > > - /* TODO: Remove this when there's support for max_bpc in drm */ > - if (dm_conn_state && bpc > dm_conn_state->max_bpc) > - /* Round down to nearest even number. */ > - bpc = dm_conn_state->max_bpc - (dm_conn_state->max_bpc & 1); > + /* TODO: Use passed in state instead of the current state. */ > + if (connector->state) { > + bpc = connector->state->max_bpc; > + /* Round down to the nearest even number. */ > + bpc = bpc - (bpc & 1); > + } > > switch (bpc) { > case 0: > @@ -3689,9 +3689,6 @@ 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 == adev->mode_info.max_bpc_property) { > - dm_new_state->max_bpc = val; > - ret = 0; > } else if (property == adev->mode_info.abm_level_property) { > dm_new_state->abm_level = val; > ret = 0; > @@ -3743,9 +3740,6 @@ 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 == adev->mode_info.max_bpc_property) {
[PATCH 1/2] drm/amd/display: Switch the custom "max bpc" property to the DRM prop
[Why] The custom "max bpc" property was added to limit color depth while the DRM one was still being merged. It's been a few kernel versions since then and this TODO was still sticking around. [How] Attach the DRM max bpc property to the connector and drop all of our custom property management. Set the max bpc to 8 by default since DRM defaults to the max in the range which would be 16 in this case. No behavioral changes are intended with this patch, it should just be a refactor. Cc: Leo Li Cc: Harry Wentland Signed-off-by: Nicholas Kazlauskas --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 --- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 2 -- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 31 --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 - 4 files changed, 13 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 4e944737b708..767ee6991ef4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -631,10 +631,6 @@ int amdgpu_display_modeset_create_props(struct amdgpu_device *adev) amdgpu_dither_enum_list, sz); if (amdgpu_device_has_dc_support(adev)) { - adev->mode_info.max_bpc_property = - drm_property_create_range(adev->ddev, 0, "max bpc", 8, 16); - if (!adev->mode_info.max_bpc_property) - return -ENOMEM; adev->mode_info.abm_level_property = drm_property_create_range(adev->ddev, 0, "abm level", 0, 4); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index c7940af42f76..8bda00ce8816 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -332,8 +332,6 @@ struct amdgpu_mode_info { struct drm_property *audio_property; /* FMT dithering */ struct drm_property *dither_property; - /* maximum number of bits per channel for monitor color */ - struct drm_property *max_bpc_property; /* Adaptive Backlight Modulation (power feature) */ struct drm_property *abm_level_property; /* it is used to allow enablement of freesync mode */ 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 4a1755bce96c..a7a9e4d81a17 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3040,14 +3040,14 @@ static void update_stream_scaling_settings(const struct drm_display_mode *mode, static enum dc_color_depth convert_color_depth_from_display_info(const struct drm_connector *connector) { - struct dm_connector_state *dm_conn_state = - to_dm_connector_state(connector->state); - uint32_t bpc = connector->display_info.bpc; + uint32_t bpc = 8; - /* TODO: Remove this when there's support for max_bpc in drm */ - if (dm_conn_state && bpc > dm_conn_state->max_bpc) - /* Round down to nearest even number. */ - bpc = dm_conn_state->max_bpc - (dm_conn_state->max_bpc & 1); + /* TODO: Use passed in state instead of the current state. */ + if (connector->state) { + bpc = connector->state->max_bpc; + /* Round down to the nearest even number. */ + bpc = bpc - (bpc & 1); + } switch (bpc) { case 0: @@ -3689,9 +3689,6 @@ 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 == adev->mode_info.max_bpc_property) { - dm_new_state->max_bpc = val; - ret = 0; } else if (property == adev->mode_info.abm_level_property) { dm_new_state->abm_level = val; ret = 0; @@ -3743,9 +3740,6 @@ 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 == adev->mode_info.max_bpc_property) { - *val = dm_state->max_bpc; - ret = 0; } else if (property == adev->mode_info.abm_level_property) { *val = dm_state->abm_level; ret = 0; @@ -3808,7 +3802,6 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector *connector) state->underscan_enable = false; state->underscan_hborder = 0; state->underscan_vborder = 0; - state->max_bpc = 8; __drm_atomic_helper_conne