Re: [PATCH v3 2/3] drm/msm/dp: revise timing engine programming to support compression (DSC)

[Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>]

On 04/02/2022 21:36, Kuogee Hsieh wrote:
> Divides horizontal width by 3 at timing engine of interface. There are
> major part of  compression (DSC) programming have to be done at DSC
> controller which is not covered by this patch.
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 22 ++++++++++++++++++----
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h |  3 +++
>   2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index 35d4aaa..ee7ca34 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -128,7 +128,7 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
>   	 * video timing. It is recommended to enable it for all cases, except
>   	 * if compression is enabled in 1 pixel per clock mode
>   	 */
> -	if (p->wide_bus_en)
> +	if (!p->compression_en || p->wide_bus_en)
>   		intf_cfg2 |= BIT(4);

So, what exactly is BIT(4)? Is it WIDE_BUS_ENABLE of some kind? Then why 
do you disable it for compression_en?

Please, add defines for all the respective BIT(n) values.

>   
>   	if (p->wide_bus_en)
> @@ -150,10 +150,16 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
>   	 */
>   	data_width = p->width;
>   
> -	if (!dp_intf && p->wide_bus_en)
> +	if (p->compression_en) {
> +		data_width = DIV_ROUND_UP(p->dce_bytes_per_line, 3);
> +
> +		if (p->wide_bus_en)
> +			data_width >>= 1;
> +	} else if (!dp_intf && p->wide_bus_en) {
>   		data_width = p->width >> 1;
> -	else
> +	} else {
>   		data_width = p->width;
> +	}
>   
>   	hsync_data_start_x = hsync_start_x;
>   	hsync_data_end_x =  hsync_start_x + data_width - 1;
> @@ -178,8 +184,16 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx,
>   
>   	active_hctl = (active_h_end << 16) | active_h_start;
>   
> -	if (dp_intf)
> +	if (dp_intf) {
>   		display_hctl = active_hctl;
> +		if (p->compression_en) {
> +			active_data_hctl = (hsync_start_x +
> +					p->extra_dto_cycles) << 16;
> +			active_data_hctl += hsync_start_x;
> +
> +			display_data_hctl = active_data_hctl;
> +		}
> +	}
>   
>   	den_polarity = 0;
>   	if (ctx->cap->type == INTF_HDMI) {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> index e4a518a..8fc71ce 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h
> @@ -32,6 +32,9 @@ struct intf_timing_params {
>   	u32 hsync_skew;
>   
>   	bool wide_bus_en;
> +	bool compression_en;
> +	u32 extra_dto_cycles;   /* for DP only */
> +	u32 dce_bytes_per_line;
>   };
>   
>   struct intf_prog_fetch {


--
With best wishes
Dmitry