All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chun-Kuang Hu <chunkuang.hu@kernel.org>
To: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Chun-Kuang Hu <chunkuang.hu@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	"moderated list:ARM/Mediatek SoC support" 
	<linux-mediatek@lists.infradead.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 0/4] Mediatek DRM driver detect CMDQ execution timeout by vblank IRQ
Date: Sat, 3 Oct 2020 11:22:30 +0800	[thread overview]
Message-ID: <CAAOTY_9bgYK2MDySstc4F2jN0M+Zpmcq2R3G_PdWz2sgt9wp7A@mail.gmail.com> (raw)
In-Reply-To: <CABb+yY1zq0+sqXuSzkkX9+dTaTZgg5HJyQLC3N-yZx35QLLvDQ@mail.gmail.com>

Hi, Jassi:

Jassi Brar <jassisinghbrar@gmail.com> 於 2020年10月3日 週六 上午4:30寫道:
>
> On Sun, Sep 27, 2020 at 6:04 PM Chun-Kuang Hu <chunkuang.hu@kernel.org> wrote:
> >
> > CMDQ helper provide timer to detect execution timeout, but DRM driver
> > could have a better way to detect execution timeout by vblank IRQ.
> > For DRM, CMDQ command should execute in vblank, so if it fail to
> > execute in next 2 vblank, timeout happen. Even though we could
> > calculate time between 2 vblank and use timer to delect, this would
> > make things more complicated.
> >
> > This introduce a series refinement for CMDQ mailbox controller and CMDQ
> > helper. Remove timer handler in helper function because different
> > client have different way to detect timeout. Use standard mailbox
> > callback instead of proprietary one to get the necessary data
> > in callback function. Remove struct cmdq_client to access client
> > instance data by struct mbox_client.
> >
> > Chun-Kuang Hu (4):
> >   soc / drm: mediatek: cmdq: Remove timeout handler in helper function
> >   mailbox / soc / drm: mediatek: Use mailbox rx_callback instead of
> >     cmdq_task_cb
> >   mailbox / soc / drm: mediatek: Remove struct cmdq_client
> >   drm/mediatek: Detect CMDQ execution timeout
> >
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c  |  54 ++++++---
> >  drivers/mailbox/mtk-cmdq-mailbox.c       |  24 ++--
> >  drivers/soc/mediatek/mtk-cmdq-helper.c   | 146 ++---------------------
> >  include/linux/mailbox/mtk-cmdq-mailbox.h |  25 +---
> >  include/linux/soc/mediatek/mtk-cmdq.h    |  54 +--------
> >  5 files changed, 66 insertions(+), 237 deletions(-)
> >
> Please break this into two patchsets - one for mailbox and one for its users.
> Also, CC original author and recent major contributors to mtk-cmdq-mailbox.c
>

Agree with you. But for patch [2/4] ("Use mailbox rx_callback instead
of cmdq_task_cb"), I think it would be a long term process.
I would break it into:

1. mtk-cmdq-mailbox.c: add rx_callback and keep  cmdq_task_cb because
client is using cmdq_task_cb.
2. client: change from cmdq_task_cb to rx_callback.
3. mtk-cmdq-mailbox.c: remove cmdq_task_cb.

The three step has dependency, but the 2nd should move to another
series, so I would go 1st step first.

Regards,
Chun-Kuang.

> Thanks.

WARNING: multiple messages have this Message-ID (diff)
From: Chun-Kuang Hu <chunkuang.hu@kernel.org>
To: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Chun-Kuang Hu <chunkuang.hu@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 0/4] Mediatek DRM driver detect CMDQ execution timeout by vblank IRQ
Date: Sat, 3 Oct 2020 11:22:30 +0800	[thread overview]
Message-ID: <CAAOTY_9bgYK2MDySstc4F2jN0M+Zpmcq2R3G_PdWz2sgt9wp7A@mail.gmail.com> (raw)
In-Reply-To: <CABb+yY1zq0+sqXuSzkkX9+dTaTZgg5HJyQLC3N-yZx35QLLvDQ@mail.gmail.com>

Hi, Jassi:

Jassi Brar <jassisinghbrar@gmail.com> 於 2020年10月3日 週六 上午4:30寫道:
>
> On Sun, Sep 27, 2020 at 6:04 PM Chun-Kuang Hu <chunkuang.hu@kernel.org> wrote:
> >
> > CMDQ helper provide timer to detect execution timeout, but DRM driver
> > could have a better way to detect execution timeout by vblank IRQ.
> > For DRM, CMDQ command should execute in vblank, so if it fail to
> > execute in next 2 vblank, timeout happen. Even though we could
> > calculate time between 2 vblank and use timer to delect, this would
> > make things more complicated.
> >
> > This introduce a series refinement for CMDQ mailbox controller and CMDQ
> > helper. Remove timer handler in helper function because different
> > client have different way to detect timeout. Use standard mailbox
> > callback instead of proprietary one to get the necessary data
> > in callback function. Remove struct cmdq_client to access client
> > instance data by struct mbox_client.
> >
> > Chun-Kuang Hu (4):
> >   soc / drm: mediatek: cmdq: Remove timeout handler in helper function
> >   mailbox / soc / drm: mediatek: Use mailbox rx_callback instead of
> >     cmdq_task_cb
> >   mailbox / soc / drm: mediatek: Remove struct cmdq_client
> >   drm/mediatek: Detect CMDQ execution timeout
> >
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c  |  54 ++++++---
> >  drivers/mailbox/mtk-cmdq-mailbox.c       |  24 ++--
> >  drivers/soc/mediatek/mtk-cmdq-helper.c   | 146 ++---------------------
> >  include/linux/mailbox/mtk-cmdq-mailbox.h |  25 +---
> >  include/linux/soc/mediatek/mtk-cmdq.h    |  54 +--------
> >  5 files changed, 66 insertions(+), 237 deletions(-)
> >
> Please break this into two patchsets - one for mailbox and one for its users.
> Also, CC original author and recent major contributors to mtk-cmdq-mailbox.c
>

Agree with you. But for patch [2/4] ("Use mailbox rx_callback instead
of cmdq_task_cb"), I think it would be a long term process.
I would break it into:

1. mtk-cmdq-mailbox.c: add rx_callback and keep  cmdq_task_cb because
client is using cmdq_task_cb.
2. client: change from cmdq_task_cb to rx_callback.
3. mtk-cmdq-mailbox.c: remove cmdq_task_cb.

The three step has dependency, but the 2nd should move to another
series, so I would go 1st step first.

Regards,
Chun-Kuang.

> Thanks.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
From: Chun-Kuang Hu <chunkuang.hu@kernel.org>
To: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Chun-Kuang Hu <chunkuang.hu@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 0/4] Mediatek DRM driver detect CMDQ execution timeout by vblank IRQ
Date: Sat, 3 Oct 2020 11:22:30 +0800	[thread overview]
Message-ID: <CAAOTY_9bgYK2MDySstc4F2jN0M+Zpmcq2R3G_PdWz2sgt9wp7A@mail.gmail.com> (raw)
In-Reply-To: <CABb+yY1zq0+sqXuSzkkX9+dTaTZgg5HJyQLC3N-yZx35QLLvDQ@mail.gmail.com>

Hi, Jassi:

Jassi Brar <jassisinghbrar@gmail.com> 於 2020年10月3日 週六 上午4:30寫道:
>
> On Sun, Sep 27, 2020 at 6:04 PM Chun-Kuang Hu <chunkuang.hu@kernel.org> wrote:
> >
> > CMDQ helper provide timer to detect execution timeout, but DRM driver
> > could have a better way to detect execution timeout by vblank IRQ.
> > For DRM, CMDQ command should execute in vblank, so if it fail to
> > execute in next 2 vblank, timeout happen. Even though we could
> > calculate time between 2 vblank and use timer to delect, this would
> > make things more complicated.
> >
> > This introduce a series refinement for CMDQ mailbox controller and CMDQ
> > helper. Remove timer handler in helper function because different
> > client have different way to detect timeout. Use standard mailbox
> > callback instead of proprietary one to get the necessary data
> > in callback function. Remove struct cmdq_client to access client
> > instance data by struct mbox_client.
> >
> > Chun-Kuang Hu (4):
> >   soc / drm: mediatek: cmdq: Remove timeout handler in helper function
> >   mailbox / soc / drm: mediatek: Use mailbox rx_callback instead of
> >     cmdq_task_cb
> >   mailbox / soc / drm: mediatek: Remove struct cmdq_client
> >   drm/mediatek: Detect CMDQ execution timeout
> >
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c  |  54 ++++++---
> >  drivers/mailbox/mtk-cmdq-mailbox.c       |  24 ++--
> >  drivers/soc/mediatek/mtk-cmdq-helper.c   | 146 ++---------------------
> >  include/linux/mailbox/mtk-cmdq-mailbox.h |  25 +---
> >  include/linux/soc/mediatek/mtk-cmdq.h    |  54 +--------
> >  5 files changed, 66 insertions(+), 237 deletions(-)
> >
> Please break this into two patchsets - one for mailbox and one for its users.
> Also, CC original author and recent major contributors to mtk-cmdq-mailbox.c
>

Agree with you. But for patch [2/4] ("Use mailbox rx_callback instead
of cmdq_task_cb"), I think it would be a long term process.
I would break it into:

1. mtk-cmdq-mailbox.c: add rx_callback and keep  cmdq_task_cb because
client is using cmdq_task_cb.
2. client: change from cmdq_task_cb to rx_callback.
3. mtk-cmdq-mailbox.c: remove cmdq_task_cb.

The three step has dependency, but the 2nd should move to another
series, so I would go 1st step first.

Regards,
Chun-Kuang.

> Thanks.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Chun-Kuang Hu <chunkuang.hu@kernel.org>
To: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Chun-Kuang Hu <chunkuang.hu@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 0/4] Mediatek DRM driver detect CMDQ execution timeout by vblank IRQ
Date: Sat, 3 Oct 2020 11:22:30 +0800	[thread overview]
Message-ID: <CAAOTY_9bgYK2MDySstc4F2jN0M+Zpmcq2R3G_PdWz2sgt9wp7A@mail.gmail.com> (raw)
In-Reply-To: <CABb+yY1zq0+sqXuSzkkX9+dTaTZgg5HJyQLC3N-yZx35QLLvDQ@mail.gmail.com>

Hi, Jassi:

Jassi Brar <jassisinghbrar@gmail.com> 於 2020年10月3日 週六 上午4:30寫道:
>
> On Sun, Sep 27, 2020 at 6:04 PM Chun-Kuang Hu <chunkuang.hu@kernel.org> wrote:
> >
> > CMDQ helper provide timer to detect execution timeout, but DRM driver
> > could have a better way to detect execution timeout by vblank IRQ.
> > For DRM, CMDQ command should execute in vblank, so if it fail to
> > execute in next 2 vblank, timeout happen. Even though we could
> > calculate time between 2 vblank and use timer to delect, this would
> > make things more complicated.
> >
> > This introduce a series refinement for CMDQ mailbox controller and CMDQ
> > helper. Remove timer handler in helper function because different
> > client have different way to detect timeout. Use standard mailbox
> > callback instead of proprietary one to get the necessary data
> > in callback function. Remove struct cmdq_client to access client
> > instance data by struct mbox_client.
> >
> > Chun-Kuang Hu (4):
> >   soc / drm: mediatek: cmdq: Remove timeout handler in helper function
> >   mailbox / soc / drm: mediatek: Use mailbox rx_callback instead of
> >     cmdq_task_cb
> >   mailbox / soc / drm: mediatek: Remove struct cmdq_client
> >   drm/mediatek: Detect CMDQ execution timeout
> >
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c  |  54 ++++++---
> >  drivers/mailbox/mtk-cmdq-mailbox.c       |  24 ++--
> >  drivers/soc/mediatek/mtk-cmdq-helper.c   | 146 ++---------------------
> >  include/linux/mailbox/mtk-cmdq-mailbox.h |  25 +---
> >  include/linux/soc/mediatek/mtk-cmdq.h    |  54 +--------
> >  5 files changed, 66 insertions(+), 237 deletions(-)
> >
> Please break this into two patchsets - one for mailbox and one for its users.
> Also, CC original author and recent major contributors to mtk-cmdq-mailbox.c
>

Agree with you. But for patch [2/4] ("Use mailbox rx_callback instead
of cmdq_task_cb"), I think it would be a long term process.
I would break it into:

1. mtk-cmdq-mailbox.c: add rx_callback and keep  cmdq_task_cb because
client is using cmdq_task_cb.
2. client: change from cmdq_task_cb to rx_callback.
3. mtk-cmdq-mailbox.c: remove cmdq_task_cb.

The three step has dependency, but the 2nd should move to another
series, so I would go 1st step first.

Regards,
Chun-Kuang.

> Thanks.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2020-10-03  3:22 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-27 23:04 [PATCH 0/4] Mediatek DRM driver detect CMDQ execution timeout by vblank IRQ Chun-Kuang Hu
2020-09-27 23:04 ` Chun-Kuang Hu
2020-09-27 23:04 ` Chun-Kuang Hu
2020-09-27 23:04 ` Chun-Kuang Hu
2020-09-27 23:04 ` [PATCH 1/4] soc / drm: mediatek: cmdq: Remove timeout handler in helper function Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04 ` [PATCH 2/4] mailbox / soc / drm: mediatek: Use mailbox rx_callback instead of cmdq_task_cb Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04 ` [PATCH 3/4] mailbox / soc / drm: mediatek: Remove struct cmdq_client Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04 ` [PATCH 4/4] drm/mediatek: Detect CMDQ execution timeout Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-09-27 23:04   ` Chun-Kuang Hu
2020-10-02 20:29 ` [PATCH 0/4] Mediatek DRM driver detect CMDQ execution timeout by vblank IRQ Jassi Brar
2020-10-02 20:29   ` Jassi Brar
2020-10-02 20:29   ` Jassi Brar
2020-10-02 20:29   ` Jassi Brar
2020-10-03  3:22   ` Chun-Kuang Hu [this message]
2020-10-03  3:22     ` Chun-Kuang Hu
2020-10-03  3:22     ` Chun-Kuang Hu
2020-10-03  3:22     ` Chun-Kuang Hu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAAOTY_9bgYK2MDySstc4F2jN0M+Zpmcq2R3G_PdWz2sgt9wp7A@mail.gmail.com \
    --to=chunkuang.hu@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jassisinghbrar@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.