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;
