linux-kernel - Re: [PATCH v5 10/13] soc: mediatek: cmdq: export finalize function

Open Source and information security mailing list archives

Message-ID: <e487573a-2252-cd52-3a3d-c271f67fcb9a@gmail.com>
Date:   Mon, 25 May 2020 10:38:11 +0200
From:   Matthias Brugger <matthias.bgg@...il.com>
To:     Chun-Kuang Hu <chunkuang.hu@...nel.org>
Cc:     Dennis YC Hsieh <dennis-yc.hsieh@...iatek.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Jassi Brar <jassisinghbrar@...il.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>, devicetree@...r.kernel.org,
        wsd_upstream@...iatek.com,
        linux-kernel <linux-kernel@...r.kernel.org>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        HS Liao <hs.liao@...iatek.com>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>,
        Houlong Wei <houlong.wei@...iatek.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v5 10/13] soc: mediatek: cmdq: export finalize function

On 25/05/2020 02:23, Chun-Kuang Hu wrote:
> Hi, Matthias:
> 
> Matthias Brugger <matthias.bgg@...il.com> 於 2020年5月17日 週日 上午2:22寫道：
>>
>>
>>
>> On 08/03/2020 11:52, Dennis YC Hsieh wrote:
>>> Export finalize function to client which helps append eoc and jump
>>> command to pkt. Let client decide call finalize or not.
>>>
>>> Signed-off-by: Dennis YC Hsieh <dennis-yc.hsieh@...iatek.com>
>>> Reviewed-by: CK Hu <ck.hu@...iatek.com>
>>> ---
>>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 1 +
>>>  drivers/soc/mediatek/mtk-cmdq-helper.c  | 7 ++-----
>>>  include/linux/soc/mediatek/mtk-cmdq.h   | 8 ++++++++
>>>  3 files changed, 11 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>>> index 0dfcd1787e65..7daaabc26eb1 100644
>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>>> @@ -490,6 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
>>>               cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
>>>               cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
>>>               mtk_crtc_ddp_config(crtc, cmdq_handle);
>>> +             cmdq_pkt_finalize(cmdq_handle);
>>>               cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
>>>       }
>>>  #endif
>>
>> This should be a independent patch.
>> Other then that patch looks good.
> 
> Apply only drm part or only cmdq helpr part, it would be abnormal.

Right it would break DRM driver (if only applied to cmdq) or compilation if only
applied to DRM.

> Shall we seperate this patch?

After thinking twice, I think we can leave it as it is. If you provide your
Acked-by I can take it thorugh my tree, if that's OK for you.

Regards,
Matthias

> Or seperate it but make sure these two patches be in the same tree?
> 
> Regards,
> Chun-Kuang.
> 
>>
>>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> index a9ebbabb7439..59bc1164b411 100644
>>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
>>> @@ -372,7 +372,7 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
>>>  }
>>>  EXPORT_SYMBOL(cmdq_pkt_assign);
>>>
>>> -static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>>> +int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>>>  {
>>>       struct cmdq_instruction inst = { {0} };
>>>       int err;
>>> @@ -392,6 +392,7 @@ static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
>>>
>>>       return err;
>>>  }
>>> +EXPORT_SYMBOL(cmdq_pkt_finalize);
>>>
>>>  static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data)
>>>  {
>>> @@ -426,10 +427,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, cmdq_async_flush_cb cb,
>>>       unsigned long flags = 0;
>>>       struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
>>>
>>> -     err = cmdq_pkt_finalize(pkt);
>>> -     if (err < 0)
>>> -             return err;
>>> -
>>>       pkt->cb.cb = cb;
>>>       pkt->cb.data = data;
>>>       pkt->async_cb.cb = cmdq_pkt_flush_async_cb;
>>> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
>>> index fec292aac83c..99e77155f967 100644
>>> --- a/include/linux/soc/mediatek/mtk-cmdq.h
>>> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
>>> @@ -213,6 +213,14 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
>>>   */
>>>  int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value);
>>>
>>> +/**
>>> + * cmdq_pkt_finalize() - Append EOC and jump command to pkt.
>>> + * @pkt:     the CMDQ packet
>>> + *
>>> + * Return: 0 for success; else the error code is returned
>>> + */
>>> +int cmdq_pkt_finalize(struct cmdq_pkt *pkt);
>>> +
>>>  /**
>>>   * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
>>>   *                          packet and call back at the end of done packet
>>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@...ts.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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