lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YV0zHet/25Zx9ld5@matsya>
Date:   Wed, 6 Oct 2021 10:54:45 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     abhinavk@...eaurora.org
Cc:     Rob Clark <robdclark@...il.com>,
        Jonathan Marek <jonathan@...ek.ca>,
        Jeffrey Hugo <jeffrey.l.hugo@...il.com>,
        David Airlie <airlied@...ux.ie>, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        dri-devel@...ts.freedesktop.org, Daniel Vetter <daniel@...ll.ch>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        freedreno@...ts.freedesktop.org,
        Sumit Semwal <sumit.semwal@...aro.org>
Subject: Re: [Freedreno] [PATCH 01/11] drm/msm/dsi: add support for dsc data

Hi Abhinav,

On 02-08-21, 15:55, abhinavk@...eaurora.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ