linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Vinod Koul <vkoul@kernel.org>, Rob Clark <robdclark@gmail.com>
Cc: linux-arm-msm@vger.kernel.org,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Jonathan Marek <jonathan@marek.ca>,
	Abhinav Kumar <abhinavk@codeaurora.org>,
	Jeffrey Hugo <jeffrey.l.hugo@gmail.com>,
	Sumit Semwal <sumit.semwal@linaro.org>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org
Subject: Re: [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder
Date: Thu, 29 Jul 2021 23:54:19 +0300	[thread overview]
Message-ID: <0227846a-47b1-96e7-f14c-7dc3b4f1ba47@linaro.org> (raw)
In-Reply-To: <20210715065203.709914-9-vkoul@kernel.org>

On 15/07/2021 09:52, Vinod Koul wrote:
> When DSC is enabled in DT, we need to configure the encoder for DSC
> configuration, calculate DSC parameters for the given timing.
> 
> This patch adds that support by adding dpu_encoder_prep_dsc() which is
> invoked when DSC is enabled in DT
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 142 +++++++++++++++++++-
>   1 file changed, 141 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 8d942052db8a..41140b781e66 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -21,12 +21,17 @@
>   #include "dpu_hw_intf.h"
>   #include "dpu_hw_ctl.h"
>   #include "dpu_hw_dspp.h"
> +#include "dpu_hw_dsc.h"
>   #include "dpu_formats.h"
>   #include "dpu_encoder_phys.h"
>   #include "dpu_crtc.h"
>   #include "dpu_trace.h"
>   #include "dpu_core_irq.h"
>   
> +#define DSC_MODE_SPLIT_PANEL		BIT(0)
> +#define DSC_MODE_MULTIPLEX		BIT(1)
> +#define DSC_MODE_VIDEO			BIT(2)

This should go into dpu_hw_dsc.h. Ah. They are already defined there and 
just redefined there. Remove the defines here.

It might be cleaner to add bool flags to struct msm_display_dsc_config 
and then calculate common mode in the dpu_hw_dsc_config().


> +
>   #define DPU_DEBUG_ENC(e, fmt, ...) DPU_DEBUG("enc%d " fmt,\
>   		(e) ? (e)->base.base.id : -1, ##__VA_ARGS__)
>   
> @@ -135,6 +140,7 @@ enum dpu_enc_rc_states {
>    * @cur_slave:		As above but for the slave encoder.
>    * @hw_pp:		Handle to the pingpong blocks used for the display. No.
>    *			pingpong blocks can be different than num_phys_encs.
> + * @hw_dsc		Handle to the DSC blocks used for the display.
>    * @intfs_swapped:	Whether or not the phys_enc interfaces have been swapped
>    *			for partial update right-only cases, such as pingpong
>    *			split where virtual pingpong does not generate IRQs
> @@ -180,6 +186,7 @@ struct dpu_encoder_virt {
>   	struct dpu_encoder_phys *cur_master;
>   	struct dpu_encoder_phys *cur_slave;
>   	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> +	struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
>   
>   	bool intfs_swapped;
>   
> @@ -1008,7 +1015,8 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>   	struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
>   	struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
>   	struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
> -	int num_lm, num_ctl, num_pp;
> +	struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
> +	int num_lm, num_ctl, num_pp, num_dsc;
>   	int i, j;
>   
>   	if (!drm_enc) {
> @@ -1061,11 +1069,16 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
>   	dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>   		drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp,
>   		ARRAY_SIZE(hw_dspp));
> +	num_dsc = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> +		drm_enc->base.id, DPU_HW_BLK_DSC, hw_dsc, ARRAY_SIZE(hw_dsc));
>   
>   	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
>   		dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
>   						: NULL;
>   
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> +		dpu_enc->hw_dsc[i] = i < num_dsc ? to_dpu_hw_dsc(hw_dsc[i]) : NULL;
> +
>   	cstate = to_dpu_crtc_state(drm_crtc->state);
>   
>   	for (i = 0; i < num_lm; i++) {
> @@ -1810,10 +1823,133 @@ static void dpu_encoder_vsync_event_work_handler(struct kthread_work *work)
>   			nsecs_to_jiffies(ktime_to_ns(wakeup_time)));
>   }
>   
> +static void
> +dpu_encoder_dsc_pclk_param_calc(struct msm_display_dsc_config *dsc, u32 width)
> +{
> +	int slice_count, slice_per_intf;
> +	int bytes_in_slice, total_bytes_per_intf;
> +
> +	if (!dsc || !dsc->drm->slice_width || !dsc->drm->slice_count) {
> +		DPU_ERROR("Invalid DSC/slices\n");
> +		return;
> +	}
> +
> +	slice_count = dsc->drm->slice_count;
> +	slice_per_intf = DIV_ROUND_UP(width, dsc->drm->slice_width);
> +
> +	/*
> +	 * If slice_count is greater than slice_per_intf then default to 1.
> +	 * This can happen during partial update.
> +	 */
> +	if (slice_count > slice_per_intf)
> +		slice_count = 1;
> +
> +	bytes_in_slice = DIV_ROUND_UP(dsc->drm->slice_width *
> +				      dsc->drm->bits_per_pixel, 8);
> +	total_bytes_per_intf = bytes_in_slice * slice_per_intf;
> +
> +	dsc->eol_byte_num = total_bytes_per_intf % 3;
> +	dsc->pclk_per_line =  DIV_ROUND_UP(total_bytes_per_intf, 3);
> +	dsc->bytes_in_slice = bytes_in_slice;
> +	dsc->bytes_per_pkt = bytes_in_slice * slice_count;
> +	dsc->pkt_per_line = slice_per_intf / slice_count;
> +}
> +
> +static void
> +dpu_encoder_dsc_initial_line_calc(struct msm_display_dsc_config *dsc,
> +				  u32 enc_ip_width)
> +{
> +	int ssm_delay, total_pixels, soft_slice_per_enc;
> +
> +	soft_slice_per_enc = enc_ip_width / dsc->drm->slice_width;
> +
> +	/*
> +	 * minimum number of initial line pixels is a sum of:
> +	 * 1. sub-stream multiplexer delay (83 groups for 8bpc,
> +	 *    91 for 10 bpc) * 3
> +	 * 2. for two soft slice cases, add extra sub-stream multiplexer * 3
> +	 * 3. the initial xmit delay
> +	 * 4. total pipeline delay through the "lock step" of encoder (47)
> +	 * 5. 6 additional pixels as the output of the rate buffer is
> +	 *    48 bits wide
> +	 */
> +	ssm_delay = ((dsc->drm->bits_per_component < 10) ? 84 : 92);
> +	total_pixels = ssm_delay * 3 + dsc->drm->initial_xmit_delay + 47;
> +	if (soft_slice_per_enc > 1)
> +		total_pixels += (ssm_delay * 3);
> +	dsc->initial_lines = DIV_ROUND_UP(total_pixels, dsc->drm->slice_width);
> +}
> +
> +static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
> +				     struct dpu_hw_pingpong *hw_pp,
> +				     struct msm_display_dsc_config *dsc,
> +				     u32 common_mode)
> +{
> +	if (hw_dsc->ops.dsc_config)
> +		hw_dsc->ops.dsc_config(hw_dsc, dsc, common_mode);
> +
> +	if (hw_dsc->ops.dsc_config_thresh)
> +		hw_dsc->ops.dsc_config_thresh(hw_dsc, dsc);
> +
> +	if (hw_pp->ops.setup_dsc)
> +		hw_pp->ops.setup_dsc(hw_pp);
> +
> +	if (hw_pp->ops.enable_dsc)
> +		hw_pp->ops.enable_dsc(hw_pp);

I think, we do not need to split these operations, I'd suggest having 
just hw_dsc->ops.dsc_config() and hw_pp->ops.enable_dsc(), merging 
dsc_config_thres() and setup_dsc() into respective methods.

> +}
> +
> +static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> +				 struct msm_display_dsc_config *dsc)
> +{
> +	/* coding only for 2LM, 2enc, 1 dsc config */
> +	struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
> +	struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> +	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> +	int this_frame_slices;
> +	int intf_ip_w, enc_ip_w;
> +	int dsc_common_mode;
> +	int pic_width, pic_height;
> +	int i;
> +
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
> +		hw_pp[i] = dpu_enc->hw_pp[i];
> +		hw_dsc[i] = dpu_enc->hw_dsc[i];
> +
> +		if (!hw_pp[i] || !hw_dsc[i]) {
> +			DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
> +			return;
> +		}
> +	}
> +
> +	dsc_common_mode = 0;
> +	pic_width = dsc->drm->pic_width;
> +	pic_height = dsc->drm->pic_height;
> +
> +	dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
> +	if (enc_master->intf_mode == INTF_MODE_VIDEO)
> +		dsc_common_mode |= DSC_MODE_VIDEO;
> +
> +	this_frame_slices = pic_width / dsc->drm->slice_width;
> +	intf_ip_w = this_frame_slices * dsc->drm->slice_width;
> +
> +	dpu_encoder_dsc_pclk_param_calc(dsc, intf_ip_w);
> +
> +	/*
> +	 * dsc merge case: when using 2 encoders for the same stream,
> +	 * no. of slices need to be same on both the encoders.
> +	 */
> +	enc_ip_w = intf_ip_w / 2;
> +	dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
> +
> +	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> +		dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode);
> +}
> +
>   void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
>   {
>   	struct dpu_encoder_virt *dpu_enc;
>   	struct dpu_encoder_phys *phys;
> +	struct msm_drm_private *priv;
>   	bool needs_hw_reset = false;
>   	unsigned int i;
>   
> @@ -1841,6 +1977,10 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
>   			dpu_encoder_helper_hw_reset(dpu_enc->phys_encs[i]);
>   		}
>   	}
> +
> +	priv = drm_enc->dev->dev_private;
> +	if (priv->dsc)
> +		dpu_encoder_prep_dsc(dpu_enc, priv->dsc);

Not quite. This makes dsc config global, while we can have several 
encoders enabled at once (think of DSI + DP). So the dsc should be a 
per-encoder setting rather than global.

>   }
>   
>   void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
> 


-- 
With best wishes
Dmitry

  parent reply	other threads:[~2021-07-29 20:54 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15  6:51 [PATCH 00/11] drm/msm: Add Display Stream Compression Support Vinod Koul
2021-07-15  6:51 ` [PATCH 01/11] drm/msm/dsi: add support for dsc data Vinod Koul
2021-08-02 22:55   ` [Freedreno] " abhinavk
2021-10-06  5:24     ` Vinod Koul
2021-07-15  6:51 ` [PATCH 02/11] drm/msm/disp/dpu1: Add support for DSC Vinod Koul
2021-07-19  8:28   ` kernel test robot
2021-08-02 23:03   ` [Freedreno] " abhinavk
2021-10-06  5:36     ` Vinod Koul
2021-07-15  6:51 ` [PATCH 03/11] drm/msm/disp/dpu1: Add support for DSC in pingpong block Vinod Koul
2021-08-02 23:07   ` [Freedreno] " abhinavk
2021-07-15  6:51 ` [PATCH 04/11] drm/msm/disp/dpu1: Add DSC support in RM Vinod Koul
2021-07-29 20:23   ` Dmitry Baryshkov
2021-10-06 10:26     ` Vinod Koul
2021-08-02 23:24   ` [Freedreno] " abhinavk
2021-10-06 10:27     ` Vinod Koul
2021-07-15  6:51 ` [PATCH 05/11] drm/msm/disp/dpu1: Add DSC for SDM845 to hw_catalog Vinod Koul
2021-07-29 20:25   ` Dmitry Baryshkov
2021-10-06 10:50     ` Vinod Koul
2021-08-02 23:29   ` [Freedreno] " abhinavk
2021-10-06 10:52     ` Vinod Koul
2021-07-15  6:51 ` [PATCH 06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl Vinod Koul
2021-07-29 22:15   ` Dmitry Baryshkov
2021-10-06 12:21     ` Vinod Koul
2021-08-03  0:00   ` [Freedreno] " abhinavk
2021-10-06 12:21     ` Vinod Koul
2021-07-15  6:51 ` [PATCH 07/11] drm/msm/disp/dpu1: Don't use DSC with mode_3d Vinod Koul
2021-08-03  0:24   ` [Freedreno] " abhinavk
2021-10-06 12:22     ` Vinod Koul
2021-07-15  6:52 ` [PATCH 08/11] drm/msm/disp/dpu1: Add support for DSC in encoder Vinod Koul
2021-07-19  8:54   ` kernel test robot
2021-07-29 20:54   ` Dmitry Baryshkov [this message]
2021-10-06 12:43     ` Vinod Koul
2021-08-03  0:57   ` [Freedreno] " abhinavk
2021-10-06 12:47     ` Vinod Koul
2021-07-15  6:52 ` [PATCH 09/11] drm/msm/disp/dpu1: Add support for DSC in topology Vinod Koul
2021-08-03  1:05   ` [Freedreno] " abhinavk
2021-07-15  6:52 ` [PATCH 10/11] drm/msm/dsi: Add support for DSC configuration Vinod Koul
2021-07-19  9:26   ` kernel test robot
2021-07-29 22:10   ` Dmitry Baryshkov
2021-08-03  1:16   ` [Freedreno] " abhinavk
2021-07-15  6:52 ` [PATCH 11/11] drm/msm/dsi: Pass DSC params to drm_panel Vinod Koul
2021-08-03  1:22   ` [Freedreno] " abhinavk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0227846a-47b1-96e7-f14c-7dc3b4f1ba47@linaro.org \
    --to=dmitry.baryshkov@linaro.org \
    --cc=abhinavk@codeaurora.org \
    --cc=airlied@linux.ie \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=jeffrey.l.hugo@gmail.com \
    --cc=jonathan@marek.ca \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robdclark@gmail.com \
    --cc=sumit.semwal@linaro.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).