| Message ID | 20230329-rfc-msm-dsc-helper-v1-2-f3e479f59b6d@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce MSM-specific DSC helpers | expand |
On 30/03/2023 02:18, Jessica Zhang wrote: > Introduce MSM-specific DSC helper methods, as some calculations are > common between DP and DSC. > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/Makefile | 1 + > drivers/gpu/drm/msm/disp/msm_dsc_helper.c | 74 +++++++++++++++++++++++++++++++ > drivers/gpu/drm/msm/disp/msm_dsc_helper.h | 28 ++++++++++++ > 3 files changed, 103 insertions(+) > > diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile > index 7274c41228ed..897a5b1c88f6 100644 > --- a/drivers/gpu/drm/msm/Makefile > +++ b/drivers/gpu/drm/msm/Makefile > @@ -90,6 +90,7 @@ msm-y += \ > disp/mdp_kms.o \ > disp/msm_disp_snapshot.o \ > disp/msm_disp_snapshot_util.o \ > + disp/msm_dsc_helper.o \ > msm_atomic.o \ > msm_atomic_tracepoints.o \ > msm_debugfs.o \ > diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.c b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c > new file mode 100644 > index 000000000000..ec15c0d829e8 > --- /dev/null > +++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c > @@ -0,0 +1,74 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved > + */ > + > +#include <linux/kernel.h> > +#include <linux/errno.h> > +#include <drm/drm_fixed.h> > + > +#include "msm_drv.h" > +#include "msm_dsc_helper.h" > + > +static int get_comp_ratio(struct drm_dsc_config *dsc, u32 src_bpp) > +{ > + return mult_frac(100, src_bpp, DSC_BPP(*dsc)); > +} > + > +static s64 get_bytes_per_soft_slice(struct drm_dsc_config *dsc, int intf_width, int comp_ratio) > +{ > + s64 comp_ratio_fp, num_bits_fp; > + s64 numerator_fp, denominator_fp; > + > + comp_ratio_fp = drm_fixp_from_fraction(comp_ratio, 100); Please inline comp_ration calculation here. Don't use mult_frac(). > + num_bits_fp = drm_fixp_from_fraction(8, 1); drm_int2fixp > + > + numerator_fp = drm_fixp_from_fraction(dsc->slice_width * dsc->bits_per_component * 3, 1); And here too. > + denominator_fp = drm_fixp_mul(comp_ratio_fp, num_bits_fp); And num_bits_fp can be inlined too. denominator_fp = drm_fixp_from_fraction(src_bpp * 8, DSC_BPP) > + > + return drm_fixp_div(numerator_fp, denominator_fp); dsc->slice_width * bpc * 3 / (8 * src_bpp / DSC_BPP), thus: drm_fixp_from_fraction(dsc->slice_width * bpc * 3 * DSC_BPP, 8 * src_bpp) but I will not insist on this one. > +} > + > +u32 msm_dsc_get_eol_byte_num(struct drm_dsc_config *dsc, int intf_width, u32 src_bpp) > +{ > + u32 bytes_per_ss, extra_eol_bytes, bytes_per_intf; > + s64 bytes_per_ss_fp; > + int slice_per_intf = msm_dsc_get_slice_per_intf(dsc, intf_width); > + int comp_ratio = get_comp_ratio(dsc, src_bpp); > + > + bytes_per_ss_fp = get_bytes_per_soft_slice(dsc, intf_width, comp_ratio); > + bytes_per_ss = drm_fixp2int_ceil(bytes_per_ss_fp); s/_ss/_soft_slice/g > + > + bytes_per_intf = bytes_per_ss * slice_per_intf; > + extra_eol_bytes = bytes_per_intf % 3; > + if (extra_eol_bytes != 0) > + extra_eol_bytes = 3 - extra_eol_bytes; > + > + return extra_eol_bytes; > +} > + > +u32 msm_dsc_get_dce_bytes_per_line(struct drm_dsc_config *dsc, int intf_width) > +{ > + u32 bpp; > + u32 dce_bytes_per_line; > + > + bpp = DSC_BPP(*dsc); Didn't this cause a warning on the unused-but-set variable? > + dce_bytes_per_line = DIV_ROUND_UP(dsc->bits_per_pixel * intf_width, 8); > + > + return dce_bytes_per_line; > +} If you have msm_dsc_get_slice_per_intf() as a static inline, this function can be a static inline too. Nothing more than a single DIV_ROUND_UP. > + > +int msm_dsc_get_pclk_per_line(struct drm_dsc_config *dsc, int intf_width, u32 src_bpp) > +{ > + s64 data_width; > + int comp_ratio = get_comp_ratio(dsc, src_bpp); > + > + if (!dsc->slice_width || (intf_width < dsc->slice_width)) > + return -EINVAL; > + > + data_width = get_bytes_per_soft_slice(dsc, intf_width, comp_ratio); > + data_width = drm_fixp_mul(dsc->slice_count, data_width); > + data_width = drm_fixp_from_fraction(data_width, 3); Reusing a variable is a nice trick, but it can be confusing. Not to mention that the last call should probably be drm_fixp_div() > + > + return drm_fixp2int_ceil(data_width); > +} > diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.h b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h > new file mode 100644 > index 000000000000..308069b2b5a4 > --- /dev/null > +++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h > @@ -0,0 +1,28 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved > + */ > + > +#ifndef MSM_DSC_HELPER_H_ > +#define MSM_DSC_HELPER_H_ > + > +#include <drm/display/drm_dsc_helper.h> > +#include <drm/drm_modes.h> > + > +/* > + * Helper methods for MSM specific DSC calculations that are common between timing engine, > + * DSI, and DP. > + */ > + > +#define MSM_DSC_SLICE_PER_PKT 1 > +#define DSC_BPP(config) ((config).bits_per_pixel >> 4) Oh. Please. If you have used (config)->bits_per_pixel here, you wouldn't have to use clumsy DSC_BPP(*dsc). It might make sense to add: static inline drm_dsc_get_bpp_int(struct drm_dsc_config *dsc) { // most probably WARN_ON_ONCE is enough. WARN_ON(dsc->bits_per_fixel & 0xf); return dsc->bits_per_pixel >> 4; } > + > +static inline int msm_dsc_get_slice_per_intf(struct drm_dsc_config *dsc, int intf_width) > +{ > + return DIV_ROUND_UP(intf_width, dsc->slice_width); > +} > + > +u32 msm_dsc_get_eol_byte_num(struct drm_dsc_config *dsc, int intf_width, u32 src_bpp); > +u32 msm_dsc_get_dce_bytes_per_line(struct drm_dsc_config *dsc, int intf_width); > +int msm_dsc_get_pclk_per_line(struct drm_dsc_config *dsc, int intf_width, u32 src_bpp); > +#endif /* MSM_DSC_HELPER_H_ */ >
On 30/03/2023 02:18, Jessica Zhang wrote: > Introduce MSM-specific DSC helper methods, as some calculations are > common between DP and DSC. > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/Makefile | 1 + > drivers/gpu/drm/msm/disp/msm_dsc_helper.c | 74 +++++++++++++++++++++++++++++++ > drivers/gpu/drm/msm/disp/msm_dsc_helper.h | 28 ++++++++++++ Also: outside of disp, please. > 3 files changed, 103 insertions(+) >
On 3/29/2023 5:40 PM, Dmitry Baryshkov wrote: > On 30/03/2023 02:18, Jessica Zhang wrote: >> Introduce MSM-specific DSC helper methods, as some calculations are >> common between DP and DSC. >> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> drivers/gpu/drm/msm/Makefile | 1 + >> drivers/gpu/drm/msm/disp/msm_dsc_helper.c | 74 >> +++++++++++++++++++++++++++++++ >> drivers/gpu/drm/msm/disp/msm_dsc_helper.h | 28 ++++++++++++ >> 3 files changed, 103 insertions(+) >> >> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile >> index 7274c41228ed..897a5b1c88f6 100644 >> --- a/drivers/gpu/drm/msm/Makefile >> +++ b/drivers/gpu/drm/msm/Makefile >> @@ -90,6 +90,7 @@ msm-y += \ >> disp/mdp_kms.o \ >> disp/msm_disp_snapshot.o \ >> disp/msm_disp_snapshot_util.o \ >> + disp/msm_dsc_helper.o \ >> msm_atomic.o \ >> msm_atomic_tracepoints.o \ >> msm_debugfs.o \ >> diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.c >> b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c >> new file mode 100644 >> index 000000000000..ec15c0d829e8 >> --- /dev/null >> +++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c >> @@ -0,0 +1,74 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights >> reserved >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/errno.h> >> +#include <drm/drm_fixed.h> >> + >> +#include "msm_drv.h" >> +#include "msm_dsc_helper.h" >> + >> +static int get_comp_ratio(struct drm_dsc_config *dsc, u32 src_bpp) >> +{ >> + return mult_frac(100, src_bpp, DSC_BPP(*dsc)); >> +} >> + >> +static s64 get_bytes_per_soft_slice(struct drm_dsc_config *dsc, int >> intf_width, int comp_ratio) >> +{ >> + s64 comp_ratio_fp, num_bits_fp; >> + s64 numerator_fp, denominator_fp; >> + >> + comp_ratio_fp = drm_fixp_from_fraction(comp_ratio, 100); > > Please inline comp_ration calculation here. Don't use mult_frac(). > >> + num_bits_fp = drm_fixp_from_fraction(8, 1); > > drm_int2fixp > >> + >> + numerator_fp = drm_fixp_from_fraction(dsc->slice_width * >> dsc->bits_per_component * 3, 1); > > And here too. > >> + denominator_fp = drm_fixp_mul(comp_ratio_fp, num_bits_fp); > > And num_bits_fp can be inlined too. > denominator_fp = drm_fixp_from_fraction(src_bpp * 8, DSC_BPP) > >> + >> + return drm_fixp_div(numerator_fp, denominator_fp); > > dsc->slice_width * bpc * 3 / (8 * src_bpp / DSC_BPP), thus: > > drm_fixp_from_fraction(dsc->slice_width * bpc * 3 * DSC_BPP, 8 * src_bpp) > > but I will not insist on this one. > >> +} >> + >> +u32 msm_dsc_get_eol_byte_num(struct drm_dsc_config *dsc, int >> intf_width, u32 src_bpp) >> +{ >> + u32 bytes_per_ss, extra_eol_bytes, bytes_per_intf; >> + s64 bytes_per_ss_fp; >> + int slice_per_intf = msm_dsc_get_slice_per_intf(dsc, intf_width); >> + int comp_ratio = get_comp_ratio(dsc, src_bpp); >> + >> + bytes_per_ss_fp = get_bytes_per_soft_slice(dsc, intf_width, >> comp_ratio); >> + bytes_per_ss = drm_fixp2int_ceil(bytes_per_ss_fp); > > s/_ss/_soft_slice/g > >> + >> + bytes_per_intf = bytes_per_ss * slice_per_intf; >> + extra_eol_bytes = bytes_per_intf % 3; >> + if (extra_eol_bytes != 0) >> + extra_eol_bytes = 3 - extra_eol_bytes; >> + >> + return extra_eol_bytes; >> +} >> + >> +u32 msm_dsc_get_dce_bytes_per_line(struct drm_dsc_config *dsc, int >> intf_width) >> +{ >> + u32 bpp; >> + u32 dce_bytes_per_line; >> + >> + bpp = DSC_BPP(*dsc); > > Didn't this cause a warning on the unused-but-set variable? > >> + dce_bytes_per_line = DIV_ROUND_UP(dsc->bits_per_pixel * >> intf_width, 8); >> + >> + return dce_bytes_per_line; >> +} > > If you have msm_dsc_get_slice_per_intf() as a static inline, this > function can be a static inline too. Nothing more than a single > DIV_ROUND_UP. > >> + >> +int msm_dsc_get_pclk_per_line(struct drm_dsc_config *dsc, int >> intf_width, u32 src_bpp) >> +{ >> + s64 data_width; >> + int comp_ratio = get_comp_ratio(dsc, src_bpp); >> + >> + if (!dsc->slice_width || (intf_width < dsc->slice_width)) >> + return -EINVAL; >> + >> + data_width = get_bytes_per_soft_slice(dsc, intf_width, comp_ratio); >> + data_width = drm_fixp_mul(dsc->slice_count, data_width); >> + data_width = drm_fixp_from_fraction(data_width, 3); > > Reusing a variable is a nice trick, but it can be confusing. Not to > mention that the last call should probably be drm_fixp_div() > Hi Dmitry, Acked (for all the comments here). Planning to move the last divide by 3 out of this method (as the value that uncompressed pclk is divided by depends on DSI/DP and if widebus is enabled), so I'll merge the get_bytes_per_soft_slice call with the 2nd line. Thanks, Jessica Zhang >> + >> + return drm_fixp2int_ceil(data_width); >> +} >> diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.h >> b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h >> new file mode 100644 >> index 000000000000..308069b2b5a4 >> --- /dev/null >> +++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h >> @@ -0,0 +1,28 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights >> reserved >> + */ >> + >> +#ifndef MSM_DSC_HELPER_H_ >> +#define MSM_DSC_HELPER_H_ >> + >> +#include <drm/display/drm_dsc_helper.h> >> +#include <drm/drm_modes.h> >> + >> +/* >> + * Helper methods for MSM specific DSC calculations that are common >> between timing engine, >> + * DSI, and DP. >> + */ >> + >> +#define MSM_DSC_SLICE_PER_PKT 1 >> +#define DSC_BPP(config) ((config).bits_per_pixel >> 4) > > Oh. Please. If you have used (config)->bits_per_pixel here, you wouldn't > have to use clumsy DSC_BPP(*dsc). It might make sense to add: > > static inline drm_dsc_get_bpp_int(struct drm_dsc_config *dsc) > { > // most probably WARN_ON_ONCE is enough. > WARN_ON(dsc->bits_per_fixel & 0xf); > > return dsc->bits_per_pixel >> 4; > } > >> + >> +static inline int msm_dsc_get_slice_per_intf(struct drm_dsc_config >> *dsc, int intf_width) >> +{ >> + return DIV_ROUND_UP(intf_width, dsc->slice_width); >> +} >> + >> +u32 msm_dsc_get_eol_byte_num(struct drm_dsc_config *dsc, int >> intf_width, u32 src_bpp); >> +u32 msm_dsc_get_dce_bytes_per_line(struct drm_dsc_config *dsc, int >> intf_width); >> +int msm_dsc_get_pclk_per_line(struct drm_dsc_config *dsc, int >> intf_width, u32 src_bpp); >> +#endif /* MSM_DSC_HELPER_H_ */ >> > > -- > With best wishes > Dmitry >
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 7274c41228ed..897a5b1c88f6 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -90,6 +90,7 @@ msm-y += \ disp/mdp_kms.o \ disp/msm_disp_snapshot.o \ disp/msm_disp_snapshot_util.o \ + disp/msm_dsc_helper.o \ msm_atomic.o \ msm_atomic_tracepoints.o \ msm_debugfs.o \ diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.c b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c new file mode 100644 index 000000000000..ec15c0d829e8 --- /dev/null +++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.c @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved + */ + +#include <linux/kernel.h> +#include <linux/errno.h> +#include <drm/drm_fixed.h> + +#include "msm_drv.h" +#include "msm_dsc_helper.h" + +static int get_comp_ratio(struct drm_dsc_config *dsc, u32 src_bpp) +{ + return mult_frac(100, src_bpp, DSC_BPP(*dsc)); +} + +static s64 get_bytes_per_soft_slice(struct drm_dsc_config *dsc, int intf_width, int comp_ratio) +{ + s64 comp_ratio_fp, num_bits_fp; + s64 numerator_fp, denominator_fp; + + comp_ratio_fp = drm_fixp_from_fraction(comp_ratio, 100); + num_bits_fp = drm_fixp_from_fraction(8, 1); + + numerator_fp = drm_fixp_from_fraction(dsc->slice_width * dsc->bits_per_component * 3, 1); + denominator_fp = drm_fixp_mul(comp_ratio_fp, num_bits_fp); + + return drm_fixp_div(numerator_fp, denominator_fp); +} + +u32 msm_dsc_get_eol_byte_num(struct drm_dsc_config *dsc, int intf_width, u32 src_bpp) +{ + u32 bytes_per_ss, extra_eol_bytes, bytes_per_intf; + s64 bytes_per_ss_fp; + int slice_per_intf = msm_dsc_get_slice_per_intf(dsc, intf_width); + int comp_ratio = get_comp_ratio(dsc, src_bpp); + + bytes_per_ss_fp = get_bytes_per_soft_slice(dsc, intf_width, comp_ratio); + bytes_per_ss = drm_fixp2int_ceil(bytes_per_ss_fp); + + bytes_per_intf = bytes_per_ss * slice_per_intf; + extra_eol_bytes = bytes_per_intf % 3; + if (extra_eol_bytes != 0) + extra_eol_bytes = 3 - extra_eol_bytes; + + return extra_eol_bytes; +} + +u32 msm_dsc_get_dce_bytes_per_line(struct drm_dsc_config *dsc, int intf_width) +{ + u32 bpp; + u32 dce_bytes_per_line; + + bpp = DSC_BPP(*dsc); + dce_bytes_per_line = DIV_ROUND_UP(dsc->bits_per_pixel * intf_width, 8); + + return dce_bytes_per_line; +} + +int msm_dsc_get_pclk_per_line(struct drm_dsc_config *dsc, int intf_width, u32 src_bpp) +{ + s64 data_width; + int comp_ratio = get_comp_ratio(dsc, src_bpp); + + if (!dsc->slice_width || (intf_width < dsc->slice_width)) + return -EINVAL; + + data_width = get_bytes_per_soft_slice(dsc, intf_width, comp_ratio); + data_width = drm_fixp_mul(dsc->slice_count, data_width); + data_width = drm_fixp_from_fraction(data_width, 3); + + return drm_fixp2int_ceil(data_width); +} diff --git a/drivers/gpu/drm/msm/disp/msm_dsc_helper.h b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h new file mode 100644 index 000000000000..308069b2b5a4 --- /dev/null +++ b/drivers/gpu/drm/msm/disp/msm_dsc_helper.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved + */ + +#ifndef MSM_DSC_HELPER_H_ +#define MSM_DSC_HELPER_H_ + +#include <drm/display/drm_dsc_helper.h> +#include <drm/drm_modes.h> + +/* + * Helper methods for MSM specific DSC calculations that are common between timing engine, + * DSI, and DP. + */ + +#define MSM_DSC_SLICE_PER_PKT 1 +#define DSC_BPP(config) ((config).bits_per_pixel >> 4) + +static inline int msm_dsc_get_slice_per_intf(struct drm_dsc_config *dsc, int intf_width) +{ + return DIV_ROUND_UP(intf_width, dsc->slice_width); +} + +u32 msm_dsc_get_eol_byte_num(struct drm_dsc_config *dsc, int intf_width, u32 src_bpp); +u32 msm_dsc_get_dce_bytes_per_line(struct drm_dsc_config *dsc, int intf_width); +int msm_dsc_get_pclk_per_line(struct drm_dsc_config *dsc, int intf_width, u32 src_bpp); +#endif /* MSM_DSC_HELPER_H_ */
Introduce MSM-specific DSC helper methods, as some calculations are common between DP and DSC. Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> --- drivers/gpu/drm/msm/Makefile | 1 + drivers/gpu/drm/msm/disp/msm_dsc_helper.c | 74 +++++++++++++++++++++++++++++++ drivers/gpu/drm/msm/disp/msm_dsc_helper.h | 28 ++++++++++++ 3 files changed, 103 insertions(+)