linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vinod Koul <vkoul@kernel.org>
To: abhinavk@codeaurora.org
Cc: Rob Clark <robdclark@gmail.com>,
	Jonathan Marek <jonathan@marek.ca>,
	Jeffrey Hugo <jeffrey.l.hugo@gmail.com>,
	David Airlie <airlied@linux.ie>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	dri-devel@lists.freedesktop.org, Daniel Vetter <daniel@ffwll.ch>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	freedreno@lists.freedesktop.org,
	Sumit Semwal <sumit.semwal@linaro.org>
Subject: Re: [Freedreno] [PATCH 01/11] drm/msm/dsi: add support for dsc data
Date: Wed, 6 Oct 2021 10:54:45 +0530	[thread overview]
Message-ID: <YV0zHet/25Zx9ld5@matsya> (raw)
In-Reply-To: <c411e4d60efd3029b2dc6b0d899ea8a9@codeaurora.org>

Hi Abhinav,

On 02-08-21, 15:55, abhinavk@codeaurora.org wrote:

> > +static int dsi_populate_dsc_params(struct msm_display_dsc_config *dsc)
> > +{
> > +	int mux_words_size;
> > +	int groups_per_line, groups_total;
> > +	int min_rate_buffer_size;
> > +	int hrd_delay;
> > +	int pre_num_extra_mux_bits, num_extra_mux_bits;
> > +	int slice_bits;
> > +	int target_bpp_x16;
> > +	int data;
> > +	int final_value, final_scale;
> > +	int i;
> > +
> > +	dsc->drm->rc_model_size = 8192;
> > +	dsc->drm->first_line_bpg_offset = 12;
> > +	dsc->drm->rc_edge_factor = 6;
> > +	dsc->drm->rc_tgt_offset_high = 3;
> > +	dsc->drm->rc_tgt_offset_low = 3;
> > +	dsc->drm->simple_422 = 0;
> > +	dsc->drm->convert_rgb = 1;
> > +	dsc->drm->vbr_enable = 0;
> > +
> > +	/* handle only bpp = bpc = 8 */
> > +	for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++)
> > +		dsc->drm->rc_buf_thresh[i] = dsi_dsc_rc_buf_thresh[i];
> > +
> > +	for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> > +		dsc->drm->rc_range_params[i].range_min_qp = min_qp[i];
> > +		dsc->drm->rc_range_params[i].range_max_qp = max_qp[i];
> > +		dsc->drm->rc_range_params[i].range_bpg_offset = bpg_offset[i];
> > +	}
> > +
> > +	dsc->drm->initial_offset = 6144; /* Not bpp 12 */
> > +	if (dsc->drm->bits_per_pixel != 8)
> > +		dsc->drm->initial_offset = 2048;	/* bpp = 12 */
> > +
> > +	mux_words_size = 48;		/* bpc == 8/10 */
> > +	if (dsc->drm->bits_per_component == 12)
> > +		mux_words_size = 64;
> > +
> > +	dsc->drm->initial_xmit_delay = 512;
> > +	dsc->drm->initial_scale_value = 32;
> > +	dsc->drm->first_line_bpg_offset = 12;
> > +	dsc->drm->line_buf_depth = dsc->drm->bits_per_component + 1;
> > +
> > +	/* bpc 8 */
> > +	dsc->drm->flatness_min_qp = 3;
> > +	dsc->drm->flatness_max_qp = 12;
> > +	dsc->det_thresh_flatness = 7 + 2 * (dsc->drm->bits_per_component - 8);
> > +	dsc->drm->rc_quant_incr_limit0 = 11;
> > +	dsc->drm->rc_quant_incr_limit1 = 11;
> > +	dsc->drm->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;
> > +
> > +	/* FIXME: need to call drm_dsc_compute_rc_parameters() so that rest of
> > +	 * params are calculated
> > +	 */
> > +	dsc->slice_last_group_size = 3 - (dsc->drm->slice_width % 3);
> > +	groups_per_line = DIV_ROUND_UP(dsc->drm->slice_width, 3);
> > +	dsc->drm->slice_chunk_size = dsc->drm->slice_width *
> > dsc->drm->bits_per_pixel / 8;
> > +	if ((dsc->drm->slice_width * dsc->drm->bits_per_pixel) % 8)
> > +		dsc->drm->slice_chunk_size++;
> > +
> > +	/* rbs-min */
> > +	min_rate_buffer_size =  dsc->drm->rc_model_size -
> > dsc->drm->initial_offset +
> > +				dsc->drm->initial_xmit_delay * dsc->drm->bits_per_pixel +
> > +				groups_per_line * dsc->drm->first_line_bpg_offset;
> > +
> > +	hrd_delay = DIV_ROUND_UP(min_rate_buffer_size,
> > dsc->drm->bits_per_pixel);
> > +
> > +	dsc->drm->initial_dec_delay = hrd_delay -
> > dsc->drm->initial_xmit_delay;
> > +
> > +	dsc->drm->initial_scale_value = 8 * dsc->drm->rc_model_size /
> > +				       (dsc->drm->rc_model_size - dsc->drm->initial_offset);
> > +
> > +	slice_bits = 8 * dsc->drm->slice_chunk_size * dsc->drm->slice_height;
> > +
> > +	groups_total = groups_per_line * dsc->drm->slice_height;
> > +
> > +	data = dsc->drm->first_line_bpg_offset * 2048;
> > +
> > +	dsc->drm->nfl_bpg_offset = DIV_ROUND_UP(data, (dsc->drm->slice_height
> > - 1));
> > +
> > +	pre_num_extra_mux_bits = 3 * (mux_words_size + (4 *
> > dsc->drm->bits_per_component + 4) - 2);
> > +
> > +	num_extra_mux_bits = pre_num_extra_mux_bits - (mux_words_size -
> > +			     ((slice_bits - pre_num_extra_mux_bits) % mux_words_size));
> > +
> > +	data = 2048 * (dsc->drm->rc_model_size - dsc->drm->initial_offset +
> > num_extra_mux_bits);
> > +	dsc->drm->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
> > +
> > +	/* bpp * 16 + 0.5 */
> > +	data = dsc->drm->bits_per_pixel * 16;
> > +	data *= 2;
> > +	data++;
> > +	data /= 2;
> > +	target_bpp_x16 = data;
> > +
> > +	data = (dsc->drm->initial_xmit_delay * target_bpp_x16) / 16;
> > +	final_value =  dsc->drm->rc_model_size - data + num_extra_mux_bits;
> As we discussed, can you please check why there is an additional + 8 and /16
> in the upstream final_offset calculation?
> If we can eliminate or root-cause the difference in the calculations, either
> this patch can be substantially reduced or
> we will atleast know for future reference what was the delta and can leave a
> comment.

I am checking this as well, I think there is something more, so will
continue to debug on that.

Meanwhile I propose we continue this and then switch once we have
concluded.

> > +/* DSC config */
> > +struct msm_display_dsc_config {
> > +	struct drm_dsc_config *drm;
> > +	u8 scr_rev;
> Can scr_rev also move into drm_dsc_config? SCR itself is not QC specific.
> Its just telling there was a change request
> for that DSC revision.
> In QC side, we only use this scr_rev to have some different tables. This can
> even be true for other vendors.
> So moving this to drm_dsc_config will only help.

So I checked and looks like this is not used here in the code, so will
drop it for now. Once we add support for this, we can add this back in
drm_dsc_config

-- 
~Vinod

  reply	other threads:[~2021-10-06  5:24 UTC|newest]

Thread overview: 41+ 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 [this message]
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
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-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=YV0zHet/25Zx9ld5@matsya \
    --to=vkoul@kernel.org \
    --cc=abhinavk@codeaurora.org \
    --cc=airlied@linux.ie \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel@ffwll.ch \
    --cc=dmitry.baryshkov@linaro.org \
    --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 \
    /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).