linux-kernel - Re: [PATCH 4/5] drm/msm/dpu1: Account for DSC's bits_per

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]

Date:   Tue, 4 Oct 2022 10:03:07 -0700
From:   Abhinav Kumar <quic_abhinavk@...cinc.com>
To:     Marijn Suijten <marijn.suijten@...ainline.org>,
        <phone-devel@...r.kernel.org>, Rob Clark <robdclark@...il.com>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Vinod Koul <vkoul@...nel.org>
CC:     <freedreno@...ts.freedesktop.org>,
        Douglas Anderson <dianders@...omium.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Jami Kettunen <jami.kettunen@...ainline.org>,
        Vladimir Lypak <vladimir.lypak@...il.com>,
        <linux-arm-msm@...r.kernel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        <dri-devel@...ts.freedesktop.org>,
        Javier Martinez Canillas <javierm@...hat.com>,
        David Airlie <airlied@...ux.ie>,
        Martin Botka <martin.botka@...ainline.org>,
        <~postmarketos/upstreaming@...ts.sr.ht>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>,
        Alex Deucher <alexander.deucher@....com>,
        Sean Paul <sean@...rly.run>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/5] drm/msm/dpu1: Account for DSC's bits_per_pixel having
 4 fractional bits



On 10/1/2022 12:08 PM, Marijn Suijten wrote:
> According to the comment this DPU register contains the bits per pixel
> as a 6.4 fractional value, conveniently matching the contents of
> bits_per_pixel in struct drm_dsc_config which also uses 4 fractional
> bits.  However, the downstream source this implementation was
> copy-pasted from has its bpp field stored _without_ fractional part.
> 
> This makes the entire convoluted math obsolete as it is impossible to
> pull those 4 fractional bits out of thin air, by somehow trying to reuse
> the lowest 2 bits of a non-fractional bpp (lsb = bpp % 4??).
> 
> The rest of the code merely attempts to keep the integer part a multiple
> of 4, which is rendered useless thanks to data |= dsc->bits_per_pixel <<
> 12; already filling up those bits anyway (but not on downstream).
> 
> Fixes: c110cfd1753e ("drm/msm/disp/dpu1: Add support for DSC")
> Signed-off-by: Marijn Suijten <marijn.suijten@...ainline.org>

Many of this bugs are because the downstream code from which this 
implementation was derived wasnt the latest perhaps?

Earlier, downstream had its own DSC struct maybe leading to this 
redundant math but now we have migrated over to use the upstream struct 
drm_dsc_config.

That being said, this patch LGTM
Reviewed-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 11 ++---------
>   1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> index f2ddcfb6f7ee..3662df698dae 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
> @@ -42,7 +42,7 @@ static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
>   			      u32 initial_lines)
>   {
>   	struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
> -	u32 data, lsb, bpp;
> +	u32 data;
>   	u32 slice_last_group_size;
>   	u32 det_thresh_flatness;
>   	bool is_cmd_mode = !(mode & DSC_MODE_VIDEO);
> @@ -56,14 +56,7 @@ static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
>   	data = (initial_lines << 20);
>   	data |= ((slice_last_group_size - 1) << 18);
>   	/* bpp is 6.4 format, 4 LSBs bits are for fractional part */
> -	data |= dsc->bits_per_pixel << 12;
> -	lsb = dsc->bits_per_pixel % 4;
> -	bpp = dsc->bits_per_pixel / 4;
> -	bpp *= 4;
> -	bpp <<= 4;
> -	bpp |= lsb;
> -
> -	data |= bpp << 8;
> +	data |= (dsc->bits_per_pixel << 8);
>   	data |= (dsc->block_pred_enable << 7);
>   	data |= (dsc->line_buf_depth << 3);
>   	data |= (dsc->simple_422 << 2);