On 01/17, Joshua Ashton wrote:
> Implements the 'content type' property for HDMI connectors.
> Verified by checking the avi infoframe on a connected TV.
>
> This also simplifies a lot of the code in that area as well, there were
> a lot of temp variables doing very little and unnecessary logic
> that was quite confusing.
>
> It is not necessary to check for support in the EDID before sending a
> 'content type' value in the avi infoframe also.
>
> Signed-off-by: Joshua Ashton <jos...@froggi.es>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 +++++++
> .../gpu/drm/amd/display/dc/core/dc_resource.c | 69 ++++++-------------
> drivers/gpu/drm/amd/display/dc/dc_stream.h | 1 +
> 3 files changed, 46 insertions(+), 48 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 9547037857b6..999965fe3de9 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5216,6 +5216,24 @@ get_output_color_space(const struct dc_crtc_timing
> *dc_crtc_timing)
> return color_space;
> }
>
> +static enum display_content_type
> +get_output_content_type(const struct drm_connector_state *connector_state)
> +{
> + switch (connector_state->content_type) {
> + default:
> + case DRM_MODE_CONTENT_TYPE_NO_DATA:
> + return DISPLAY_CONTENT_TYPE_NO_DATA;
> + case DRM_MODE_CONTENT_TYPE_GRAPHICS:
> + return DISPLAY_CONTENT_TYPE_GRAPHICS;
> + case DRM_MODE_CONTENT_TYPE_PHOTO:
> + return DISPLAY_CONTENT_TYPE_PHOTO;
> + case DRM_MODE_CONTENT_TYPE_CINEMA:
> + return DISPLAY_CONTENT_TYPE_CINEMA;
> + case DRM_MODE_CONTENT_TYPE_GAME:
> + return DISPLAY_CONTENT_TYPE_GAME;
> + }
> +}
> +
> static bool adjust_colour_depth_from_display_info(
> struct dc_crtc_timing *timing_out,
> const struct drm_display_info *info)
> @@ -5349,6 +5367,7 @@ static void
> fill_stream_properties_from_drm_display_mode(
> }
>
> stream->output_color_space = get_output_color_space(timing_out);
> + stream->content_type = get_output_content_type(connector_state);
> }
>
> static void fill_audio_info(struct audio_info *audio_info,
> @@ -7123,6 +7142,11 @@ void amdgpu_dm_connector_init_helper(struct
> amdgpu_display_manager *dm,
> adev->mode_info.abm_level_property, 0);
> }
>
> + if (connector_type == DRM_MODE_CONNECTOR_HDMIA) {
> + /* Content Type is currently only implemented for HDMI. */
> + drm_connector_attach_content_type_property(&aconnector->base);
> + }
> +
> if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> connector_type == DRM_MODE_CONNECTOR_eDP) {
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index a5b5f8592c1b..39ceccdb6586 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -2944,14 +2944,9 @@ static void set_avi_info_frame(
> uint32_t pixel_encoding = 0;
> enum scanning_type scan_type = SCANNING_TYPE_NODATA;
> enum dc_aspect_ratio aspect = ASPECT_RATIO_NO_DATA;
> - bool itc = false;
> - uint8_t itc_value = 0;
> - uint8_t cn0_cn1 = 0;
> - unsigned int cn0_cn1_value = 0;
> uint8_t *check_sum = NULL;
> uint8_t byte_index = 0;
> union hdmi_info_packet hdmi_info;
> - union display_content_support support = {0};
> unsigned int vic = pipe_ctx->stream->timing.vic;
> unsigned int rid = pipe_ctx->stream->timing.rid;
> unsigned int fr_ind = pipe_ctx->stream->timing.fr_index;
> @@ -3055,49 +3050,27 @@ static void set_avi_info_frame(
> /* Active Format Aspect ratio - same as Picture Aspect Ratio. */
> hdmi_info.bits.R0_R3 = ACTIVE_FORMAT_ASPECT_RATIO_SAME_AS_PICTURE;
>
> - /* TODO: un-hardcode cn0_cn1 and itc */
> -
> - cn0_cn1 = 0;
> - cn0_cn1_value = 0;
> -
> - itc = true;
> - itc_value = 1;
> -
> - support = stream->content_support;
> -
> - if (itc) {
> - if (!support.bits.valid_content_type) {
> - cn0_cn1_value = 0;
> - } else {
> - if (cn0_cn1 == DISPLAY_CONTENT_TYPE_GRAPHICS) {
> - if (support.bits.graphics_content == 1) {
> - cn0_cn1_value = 0;
> - }
> - } else if (cn0_cn1 == DISPLAY_CONTENT_TYPE_PHOTO) {
> - if (support.bits.photo_content == 1) {
> - cn0_cn1_value = 1;
> - } else {
> - cn0_cn1_value = 0;
> - itc_value = 0;
> - }
> - } else if (cn0_cn1 == DISPLAY_CONTENT_TYPE_CINEMA) {
> - if (support.bits.cinema_content == 1) {
> - cn0_cn1_value = 2;
> - } else {
> - cn0_cn1_value = 0;
> - itc_value = 0;
> - }
> - } else if (cn0_cn1 == DISPLAY_CONTENT_TYPE_GAME) {
> - if (support.bits.game_content == 1) {
> - cn0_cn1_value = 3;
> - } else {
> - cn0_cn1_value = 0;
> - itc_value = 0;
> - }
> - }
> - }
> - hdmi_info.bits.CN0_CN1 = cn0_cn1_value;
> - hdmi_info.bits.ITC = itc_value;
> + switch (stream->content_type) {
> + case DISPLAY_CONTENT_TYPE_NO_DATA:
> + hdmi_info.bits.CN0_CN1 = 0;
> + hdmi_info.bits.ITC = 0;
Hmm.. why is ITC value equal zero here ^, instead of the same hardcoded
`itc_value = 1`? Does it come from a DRM default value?
Other than that, changes seem fine to me and it's nice to see the code
wired to the DRM and actually used.
CC'ing other AMD DC folks since I don't know if these changes affect
other platforms. Can you guys verify it?
> + break;
> + case DISPLAY_CONTENT_TYPE_GRAPHICS:
> + hdmi_info.bits.CN0_CN1 = 0;
> + hdmi_info.bits.ITC = 1;
> + break;
> + case DISPLAY_CONTENT_TYPE_PHOTO:
> + hdmi_info.bits.CN0_CN1 = 1;
> + hdmi_info.bits.ITC = 1;
> + break;
> + case DISPLAY_CONTENT_TYPE_CINEMA:
> + hdmi_info.bits.CN0_CN1 = 2;
> + hdmi_info.bits.ITC = 1;
> + break;
> + case DISPLAY_CONTENT_TYPE_GAME:
> + hdmi_info.bits.CN0_CN1 = 3;
> + hdmi_info.bits.ITC = 1;
> + break;
> }
>
> if (stream->qs_bit == 1) {
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> index ef33d7d8a2bf..51dc30706e43 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> @@ -205,6 +205,7 @@ struct dc_stream_state {
> struct dc_csc_transform csc_color_matrix;
>
> enum dc_color_space output_color_space;
> + enum display_content_type content_type;
> enum dc_dither_option dither_option;
>
> enum view_3d_format view_format;
> --
> 2.39.0
>
signature.asc
Description: PGP signature
