Re: [PATCH v2] drm/amd/display: Calculate output_color_space after pixel encoding adjustment
Applied with the fixes tag. Thanks! Alex On Mon, Jan 16, 2023 at 9:41 AM Melissa Wen wrote: > > On 01/16, Melissa Wen wrote: > > On 01/10, Joshua Ashton wrote: > > > Code in get_output_color_space depends on knowing the pixel encoding to > > > determine whether to pick between eg. COLOR_SPACE_SRGB or > > > COLOR_SPACE_YCBCR709 for transparent RGB -> YCbCr 4:4:4 in the driver. > > > > > > v2: Fixed patch being accidentally based on a personal feature branch, > > > oops! > > > > > > Signed-off-by: Joshua Ashton > > > --- > > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 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 b4d60eedbcbf..9da71ee8fcc4 100644 > > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > > @@ -5341,8 +5341,6 @@ static void > > > fill_stream_properties_from_drm_display_mode( > > > > > > timing_out->aspect_ratio = get_aspect_ratio(mode_in); > > > > > > - stream->output_color_space = get_output_color_space(timing_out); > > > - > > > stream->out_transfer_func->type = TF_TYPE_PREDEFINED; > > > stream->out_transfer_func->tf = TRANSFER_FUNCTION_SRGB; > > > if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A) { > > > @@ -5353,6 +5351,8 @@ static void > > > fill_stream_properties_from_drm_display_mode( > > > adjust_colour_depth_from_display_info(timing_out, > > > info); > > > } > > > } > > > + > > > + stream->output_color_space = get_output_color_space(timing_out); > > > > LGTM. > > > > I see that we were ignoring the updated value of pixel_encoding in the > > previous point. Nice catch! > > > > Reviewed-by: Melissa Wen > > BTW, I think it deserves a fixes tag to: > > Fixes: ea117312ea9f ("drm/amd/display: Reduce HDMI pixel encoding if max > clock is exceeded") > > > > > > } > > > > > > static void fill_audio_info(struct audio_info *audio_info, > > > -- > > > 2.39.0 > > > > >
Re: [PATCH v2] drm/amd/display: Calculate output_color_space after pixel encoding adjustment
On 01/16, Melissa Wen wrote: > On 01/10, Joshua Ashton wrote: > > Code in get_output_color_space depends on knowing the pixel encoding to > > determine whether to pick between eg. COLOR_SPACE_SRGB or > > COLOR_SPACE_YCBCR709 for transparent RGB -> YCbCr 4:4:4 in the driver. > > > > v2: Fixed patch being accidentally based on a personal feature branch, oops! > > > > Signed-off-by: Joshua Ashton > > --- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 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 b4d60eedbcbf..9da71ee8fcc4 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > @@ -5341,8 +5341,6 @@ static void > > fill_stream_properties_from_drm_display_mode( > > > > timing_out->aspect_ratio = get_aspect_ratio(mode_in); > > > > - stream->output_color_space = get_output_color_space(timing_out); > > - > > stream->out_transfer_func->type = TF_TYPE_PREDEFINED; > > stream->out_transfer_func->tf = TRANSFER_FUNCTION_SRGB; > > if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A) { > > @@ -5353,6 +5351,8 @@ static void > > fill_stream_properties_from_drm_display_mode( > > adjust_colour_depth_from_display_info(timing_out, info); > > } > > } > > + > > + stream->output_color_space = get_output_color_space(timing_out); > > LGTM. > > I see that we were ignoring the updated value of pixel_encoding in the > previous point. Nice catch! > > Reviewed-by: Melissa Wen BTW, I think it deserves a fixes tag to: Fixes: ea117312ea9f ("drm/amd/display: Reduce HDMI pixel encoding if max clock is exceeded") > > > } > > > > static void fill_audio_info(struct audio_info *audio_info, > > -- > > 2.39.0 > > signature.asc Description: PGP signature
Re: [PATCH v2] drm/amd/display: Calculate output_color_space after pixel encoding adjustment
On 01/10, Joshua Ashton wrote: > Code in get_output_color_space depends on knowing the pixel encoding to > determine whether to pick between eg. COLOR_SPACE_SRGB or > COLOR_SPACE_YCBCR709 for transparent RGB -> YCbCr 4:4:4 in the driver. > > v2: Fixed patch being accidentally based on a personal feature branch, oops! > > Signed-off-by: Joshua Ashton > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 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 b4d60eedbcbf..9da71ee8fcc4 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -5341,8 +5341,6 @@ static void > fill_stream_properties_from_drm_display_mode( > > timing_out->aspect_ratio = get_aspect_ratio(mode_in); > > - stream->output_color_space = get_output_color_space(timing_out); > - > stream->out_transfer_func->type = TF_TYPE_PREDEFINED; > stream->out_transfer_func->tf = TRANSFER_FUNCTION_SRGB; > if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A) { > @@ -5353,6 +5351,8 @@ static void > fill_stream_properties_from_drm_display_mode( > adjust_colour_depth_from_display_info(timing_out, info); > } > } > + > + stream->output_color_space = get_output_color_space(timing_out); LGTM. I see that we were ignoring the updated value of pixel_encoding in the previous point. Nice catch! Reviewed-by: Melissa Wen > } > > static void fill_audio_info(struct audio_info *audio_info, > -- > 2.39.0 > signature.asc Description: PGP signature
[PATCH v2] drm/amd/display: Calculate output_color_space after pixel encoding adjustment
Code in get_output_color_space depends on knowing the pixel encoding to determine whether to pick between eg. COLOR_SPACE_SRGB or COLOR_SPACE_YCBCR709 for transparent RGB -> YCbCr 4:4:4 in the driver. v2: Fixed patch being accidentally based on a personal feature branch, oops! Signed-off-by: Joshua Ashton --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 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 b4d60eedbcbf..9da71ee8fcc4 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -5341,8 +5341,6 @@ static void fill_stream_properties_from_drm_display_mode( timing_out->aspect_ratio = get_aspect_ratio(mode_in); - stream->output_color_space = get_output_color_space(timing_out); - stream->out_transfer_func->type = TF_TYPE_PREDEFINED; stream->out_transfer_func->tf = TRANSFER_FUNCTION_SRGB; if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A) { @@ -5353,6 +5351,8 @@ static void fill_stream_properties_from_drm_display_mode( adjust_colour_depth_from_display_info(timing_out, info); } } + + stream->output_color_space = get_output_color_space(timing_out); } static void fill_audio_info(struct audio_info *audio_info, -- 2.39.0