[Freedreno] [PATCH v5 2/8] drm/msm: Add MSM-specific DSC helper methods

Wed Apr 12 21:49:37 UTC 2023

On 4/12/2023 12:16 PM, Dmitry Baryshkov wrote:
> On 12/04/2023 22:09, Jessica Zhang wrote:
>> Introduce MSM-specific DSC helper methods, as some calculations are
>> common between DP and DSC.
>>
>> Changes in v2:
>> - Moved files up to msm/ directory
>> - Dropped get_comp_ratio() helper
>> - Used drm_int2fixp() to convert to integers to fp
>> - Style changes to improve readability
>> - Dropped unused bpp variable in msm_dsc_get_dce_bytes_per_line()
>> - Changed msm_dsc_get_slice_per_intf() to a static inline method
>> - Dropped last division step of msm_dsc_get_pclk_per_line() and changed
>>    method name accordingly
>> - Changed DSC_BPP macro to drm_dsc_get_bpp_int() helper method
>> - Fixed some math issues caused by passing in incorrect types to
>>    drm_fixed methods in get_bytes_per_soft_slice()
>>
>> Changes in v3:
>> - Dropped src_bpp parameter from all methods -- src_bpp can be
>>    calculated as dsc->bits_per_component * 3
>> - Dropped intf_width parameter from get_bytes_per_soft_slice()
>> - Moved dsc->bits_per_component to numerator calculation in
>>    get_bytes_per_soft_slice()
>> - Renamed msm_dsc_get_uncompressed_pclk_per_line to
>>    *_get_uncompressed_pclk_per_intf()
>> - Removed dsc->slice_width check from
>>    msm_dsc_get_uncompressed_pclk_per_intf()
>> - Made get_bytes_per_soft_slice() a public method (this will be called
>>    later to help calculate DP pclk params)
>> - Added documentation in comments
>> - Moved extra_eol_bytes math out of msm_dsc_get_eol_byte_num() and
>>    renamed msm_dsc_get_eol_byte_num to *_get_bytes_per_intf.
>>
>> Changes in v4:
>> - Changed msm_dsc_get_uncompressed_pclk_per_intf to
>>    msm_dsc_get_pclk_per_intf
>>
>> Changes in v5:
>> - Added extra line at end of msm_dsc_helper.h
>> - Simplified msm_dsc_get_bytes_per_soft_slice() math
>> - Simplified and inlined msm_dsc_get_pclk_per_intf() math
>> - Removed unused headers
>>
>> Signed-off-by: Jessica Zhang <quic_jesszhan at quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/Makefile         |  1 +
>>   drivers/gpu/drm/msm/msm_dsc_helper.c | 25 ++++++++++++
>>   drivers/gpu/drm/msm/msm_dsc_helper.h | 75 
>> ++++++++++++++++++++++++++++++++++++
>>   3 files changed, 101 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
>> index 7274c41228ed..b814fc80e2d5 100644
>> --- a/drivers/gpu/drm/msm/Makefile
>> +++ b/drivers/gpu/drm/msm/Makefile
>> @@ -94,6 +94,7 @@ msm-y += \
>>       msm_atomic_tracepoints.o \
>>       msm_debugfs.o \
>>       msm_drv.o \
>> +    msm_dsc_helper.o \
>>       msm_fb.o \
>>       msm_fence.o \
>>       msm_gem.o \
>> diff --git a/drivers/gpu/drm/msm/msm_dsc_helper.c 
>> b/drivers/gpu/drm/msm/msm_dsc_helper.c
>> new file mode 100644
>> index 000000000000..a34614b8e131
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/msm_dsc_helper.c
>> @@ -0,0 +1,25 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights 
>> reserved
>> + */
>> +
>> +#include <drm/drm_fixed.h>
>> +
>> +#include "msm_dsc_helper.h"
>> +
>> +s64 msm_dsc_get_bytes_per_soft_slice(struct drm_dsc_config *dsc)
>> +{
>> +    return drm_fixp_from_fraction(dsc->slice_width * 
>> msm_dsc_get_bpp_int(dsc), 8);
>> +}
>> +
>> +u32 msm_dsc_get_bytes_per_intf(struct drm_dsc_config *dsc, int 
>> intf_width)
>> +{
>> +    u32 bytes_per_soft_slice;
>> +    s64 bytes_per_soft_slice_fp;
>> +    int slice_per_intf = msm_dsc_get_slice_per_intf(dsc, intf_width);
>> +
>> +    bytes_per_soft_slice_fp = msm_dsc_get_bytes_per_soft_slice(dsc);
>> +    bytes_per_soft_slice = drm_fixp2int_ceil(bytes_per_soft_slice_fp);
>> +
>> +    return bytes_per_soft_slice * slice_per_intf;
>> +}
>> diff --git a/drivers/gpu/drm/msm/msm_dsc_helper.h 
>> b/drivers/gpu/drm/msm/msm_dsc_helper.h
>> new file mode 100644
>> index 000000000000..34238153ffdf
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/msm_dsc_helper.h
>> @@ -0,0 +1,75 @@
>> +/* 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 <linux/kernel.h>
>> +#include <linux/bug.h>
>> +#include <drm/display/drm_dsc_helper.h>
>> +
>> +/*
>> + * Helper methods for MSM specific DSC calculations that are common 
>> between timing engine,
>> + * DSI, and DP.
>> + */
>> +
>> +/**
>> + * msm_dsc_get_bpp_int - get bits per pixel integer value
>> + * @dsc: Pointer to drm dsc config struct
>> + */
>> +static inline int msm_dsc_get_bpp_int(struct drm_dsc_config *dsc)
>> +{
>> +    WARN_ON_ONCE(dsc->bits_per_pixel & 0xf);
>> +    return dsc->bits_per_pixel >> 4;
>> +}
>> +
>> +/**
>> + * msm_dsc_get_slice_per_intf - get number of slices per interface
>> + * @dsc: Pointer to drm dsc config struct
>> + * @intf_width: interface width
>> + */
>> +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);
>> +}
>> +
>> +/**
>> + * msm_dsc_get_dce_bytes_per_line - get bytes per line to help 
>> calculate data width
>> + *    when configuring the timing engine
>> + * @dsc: Pointer to drm dsc config struct
>> + * @intf_width: interface width
>> + */
>> +static inline u32 msm_dsc_get_dce_bytes_per_line(struct 
>> drm_dsc_config *dsc, int intf_width)
>> +{
>> +    return DIV_ROUND_UP(msm_dsc_get_bpp_int(dsc) * intf_width, 8);
>> +}
>> +
>> +/**
>> + * msm_dsc_get_pclk_per_intf - Calculate pclk per interface.
>> + * @dsc: Pointer to drm dsc config struct
>> + *
>> + * Note: This value will then be passed along to DSI and DP for some 
>> more
>> + * calculations. This is because DSI and DP divide the pclk_per_intf 
>> value
>> + * by different values depending on if widebus is enabled.
>> + */
>> +static inline int msm_dsc_get_pclk_per_intf(struct drm_dsc_config *dsc)
>> +{
>> +    return DIV_ROUND_UP(dsc->slice_width * dsc->slice_count * 
>> msm_dsc_get_bpp_int(dsc), 8);
>> +}
>> +
>> +/**
>> + * msm_dsc_get_bytes_per_soft_slice - get size of each soft slice for 
>> dsc
>> + * @dsc: Pointer to drm dsc config struct
> 
> Nit:
> Returns value in fixed point s31.32 format.

Hi Dmitry,

Acked, I'll add docs on the return value for all the helpers.

> 
> Even better would be to make this an internal-only static function. When 
> DSI or DP driver requires it, we can make it public again.

I'd prefer to keep the method public since we're planning to submit DP 
driver DSC v1.2 patches (after this series has been finalized) that will 
make use of this method. Keeping this function public would mean that 
the DP changes won't have to touch the msm_dsc_helper files at all.

Thanks,

Jessica Zhang

> 
>> + */
>> +s64 msm_dsc_get_bytes_per_soft_slice(struct drm_dsc_config *dsc);
>> +
>> +/**
>> + * msm_dsc_get_bytes_per_intf - get total bytes per interface
>> + * @dsc: Pointer to drm dsc config struct
>> + * @intf_width: interface width
>> + */
>> +u32 msm_dsc_get_bytes_per_intf(struct drm_dsc_config *dsc, int 
>> intf_width);
>> +
>> +#endif /* MSM_DSC_HELPER_H_ */
>>
> 
> -- 
> With best wishes
> Dmitry
> 