Re: [Intel-gfx] [PATCH 1/2] drm/dsc: fix drm_edp_dsc_sink_output_bpp() DPCD high byte usage

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 20 Apr 2023, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
> LGTM.
>
> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

Thanks for the reviews, pushed these a week+ ago.

BR,
Jani.


>
> On 4/6/2023 7:16 PM, Jani Nikula wrote:
>> The operator precedence between << and & is wrong, leading to the high
>> byte being completely ignored. For example, with the 6.4 format, 32
>> becomes 0 and 24 becomes 8. Fix it, and remove the slightly confusing
>> and unnecessary DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT macro while at it.
>>
>> Fixes: 0575650077ea ("drm/dp: DRM DP helper/macros to get DP sink DSC parameters")
>> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>> Cc: Manasi Navare <navaremanasi@google.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> Cc: <stable@vger.kernel.org> # v5.0+
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   include/drm/display/drm_dp.h        | 1 -
>>   include/drm/display/drm_dp_helper.h | 5 ++---
>>   2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
>> index 358db4a9f167..89d5a700b04d 100644
>> --- a/include/drm/display/drm_dp.h
>> +++ b/include/drm/display/drm_dp.h
>> @@ -286,7 +286,6 @@
>>   
>>   #define DP_DSC_MAX_BITS_PER_PIXEL_HI        0x068   /* eDP 1.4 */
>>   # define DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK  (0x3 << 0)
>> -# define DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT 8
>>   # define DP_DSC_MAX_BPP_DELTA_VERSION_MASK  0x06
>>   # define DP_DSC_MAX_BPP_DELTA_AVAILABILITY  0x08
>>   
>> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
>> index 533d3ee7fe05..86f24a759268 100644
>> --- a/include/drm/display/drm_dp_helper.h
>> +++ b/include/drm/display/drm_dp_helper.h
>> @@ -181,9 +181,8 @@ static inline u16
>>   drm_edp_dsc_sink_output_bpp(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>>   {
>>   	return dsc_dpcd[DP_DSC_MAX_BITS_PER_PIXEL_LOW - DP_DSC_SUPPORT] |
>> -		(dsc_dpcd[DP_DSC_MAX_BITS_PER_PIXEL_HI - DP_DSC_SUPPORT] &
>> -		 DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK <<
>> -		 DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT);
>> +		((dsc_dpcd[DP_DSC_MAX_BITS_PER_PIXEL_HI - DP_DSC_SUPPORT] &
>> +		  DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK) << 8);
>>   }
>>   
>>   static inline u32

-- 
Jani Nikula, Intel Open Source Graphics Center