linux-kernel - Re: [PATCH 1/2] drm/mediatek: Support CMDQ interface in ddp component

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:   Fri, 6 Sep 2019 06:49:59 +0800
From:   Nicolas Boichat <drinkcat@...omium.org>
To:     Bibby Hsieh <bibby.hsieh@...iatek.com>
Cc:     David Airlie <airlied@...ux.ie>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        dri-devel@...ts.freedesktop.org,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        YT Shen <yt.shen@...iatek.com>,
        Thierry Reding <thierry.reding@...il.com>,
        CK Hu <ck.hu@...iatek.com>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        Tomasz Figa <tfiga@...omium.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Yongqiang Niu <yongqiang.niu@...iatek.com>,
        Guenter Roeck <groeck@...omium.org>
Subject: Re: [PATCH 1/2] drm/mediatek: Support CMDQ interface in ddp component

On Fri, Aug 30, 2019 at 3:41 PM Bibby Hsieh <bibby.hsieh@...iatek.com> wrote:
>
> The CMDQ (Command Queue) in MT8183 is used to help
> update all relevant display controller registers
> with critical time limation.
> This patch add cmdq interface in ddp_comp interface,
> let all ddp_comp interface can support cpu/cmdq function
> at the same time.
>
> Signed-off-by: YT Shen <yt.shen@...iatek.com>
> Signed-off-by: CK Hu <ck.hu@...iatek.com>
> Signed-off-by: Philipp Zabel <p.zabel@...gutronix.de>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@...iatek.com>
> Signed-off-by: Yongqiang Niu <yongqiang.niu@...iatek.com>
> --- [snip]
>  static void mtk_gamma_set(struct mtk_ddp_comp *comp,
> -                         struct drm_crtc_state *state)
> +                         struct drm_crtc_state *state,
> +                         struct cmdq_pkt *cmdq_pkt)
>  {
> -       unsigned int i, reg;
> +       unsigned int i;
>         struct drm_color_lut *lut;
>         void __iomem *lut_base;
>         u32 word;
>
>         if (state->gamma_lut) {
> -               reg = readl(comp->regs + DISP_GAMMA_CFG);
> -               reg = reg | GAMMA_LUT_EN;
> -               writel(reg, comp->regs + DISP_GAMMA_CFG);
> +               mtk_ddp_write_mask(cmdq_pkt, GAMMA_LUT_EN, comp,
> +                                  DISP_GAMMA_CFG, GAMMA_LUT_EN);
>                 lut_base = comp->regs + DISP_GAMMA_LUT;
>                 lut = (struct drm_color_lut *)state->gamma_lut->data;
>                 for (i = 0; i < MTK_LUT_SIZE; i++) {
>                         word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
>                                 (((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
>                                 ((lut[i].blue >> 6) & LUT_10BIT_MASK);
> -                       writel(word, (lut_base + i * 4));
> +                       mtk_ddp_write(cmdq_pkt, word, comp,
> +                                     (unsigned int)(lut_base + i * 4));

Guenter pointed out that this looks quite wrong. We should have:
unsigned int lut_base;
lut_base = DISP_GAMMA_LUT;
mtk_ddp_write(cmdq_pkt, word, comp, lut_base + i * 4);

Or more simply:
mtk_ddp_write(cmdq_pkt, word, comp, DISP_GAMMA_LUT + i * 4);

>                 }
>         }