linux-kernel - Re: [PATCH v4 4/7] drm/msm/dpu: add PINGPONG

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] [day] [month] [year] [list]

Date:   Thu, 4 May 2023 01:36:53 +0200
From:   Marijn Suijten <marijn.suijten@...ainline.org>
To:     Kuogee Hsieh <quic_khsieh@...cinc.com>
Cc:     dri-devel@...ts.freedesktop.org, robdclark@...il.com,
        sean@...rly.run, swboyd@...omium.org, dianders@...omium.org,
        vkoul@...nel.org, daniel@...ll.ch, airlied@...il.com,
        agross@...nel.org, dmitry.baryshkov@...aro.org,
        andersson@...nel.org, quic_abhinavk@...cinc.com,
        quic_sbillaka@...cinc.com, freedreno@...ts.freedesktop.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 4/7] drm/msm/dpu: add PINGPONG_NONE to disconnect DSC
 from PINGPONG

On 2023-05-03 13:10:36, Kuogee Hsieh wrote:
> During DSC setup, the crossbar mux need to be programmed to engage
> DSC to specified PINGPONG. Hence during tear down, the crossbar mux
> need to be reset to disengage DSC from PINGPONG. 0X0F is written to
> reset crossbar mux. It is not relevant to hw_pp->idx.  This patch add
> PINGPONG_NONE to serve as disable to reset crossbar mux.
> 
> Changes in v4:
> -- more details to commit text

As requested in v3, this doesn't adequately explain that all you're
doing is **removing `bool enable`** so that this function becomes
simpler to call in the disable scenario without coming up with a random
dpu_pingpong value that's irrelevant when enable=false.  How about the
following wording:

    drm/msm/dpu: Introduce PINGPONG_NONE to disconnect DSC from PINGPONG

    Disabling the crossbar mux between DSC and PINGPONG currently
    requires a bogus enum dpu_pingpong value to be passed when calling
    dsc_bind_pingpong_blk() with enable=false, even though the register
    value written is independent of the current PINGPONG block.  Replace
    that `bool enable` parameter with a new PINGPONG_NONE dpu_pingpong
    flag that triggers the write of the "special" 0xF "crossbar
    disabled" value to the register instead.

And don't forget to fix the log statement below.

<snip>

>  	DRM_DEBUG_KMS("%s dsc:%d %s pp:%d\n",
> -			enable ? "Binding" : "Unbinding",
> +			pp ? "Binding" : "Unbinding",
>  			hw_dsc->idx - DSC_0,
> -			enable ? "to" : "from",
> +			pp ? "to" : "from",
>  			pp - PINGPONG_0);

This wasn't adjusted, see v3 review.

- Marijn