| Message ID | 20230406134615.1422509-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/dsc: fix drm_edp_dsc_sink_output_bpp() DPCD high byte usage | expand |
LGTM. Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> 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
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
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
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(-)