Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1470148 > unrolled thread

Re: [PATCH v13 0/4] Mediatek MT8173 CMDQ support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v13 0/4] Mediatek MT8173 CMDQ support Horng-Shyang Liao <hs.liao@mediatek.com> - 2016-08-25 15:40 +0200
    Re: [PATCH v13 0/4] Mediatek MT8173 CMDQ support Jassi Brar <jassisinghbrar@gmail.com> - 2016-08-25 15:50 +0200
      Re: [PATCH v13 0/4] Mediatek MT8173 CMDQ support Horng-Shyang Liao <hs.liao@mediatek.com> - 2016-08-31 10:20 +0200
        Re: [PATCH v13 0/4] Mediatek MT8173 CMDQ support Jassi Brar <jassisinghbrar@gmail.com> - 2016-08-31 10:50 +0200
          Re: [PATCH v13 0/4] Mediatek MT8173 CMDQ support Horng-Shyang Liao <hs.liao@mediatek.com> - 2016-09-02 11:00 +0200

#1470148 — Re: [PATCH v13 0/4] Mediatek MT8173 CMDQ support

Hi Matthias,

On Wed, 2016-08-24 at 13:00 +0200, Matthias Brugger wrote:
> On 24/08/16 05:27, HS Liao wrote:
[...]
> > Changes since v12:
> >  - remove mediatek,gce from device tree
> 
> Why? The binding got accepted by Rob.

We can get cmdq dev from mailbox controller,
so we don't need mediatek,gce anymore.

> >
> > Best regards,
> > HS Liao
> >
> > HS Liao (4):
> >   dt-bindings: soc: Add documentation for the MediaTek GCE unit
> >   CMDQ: Mediatek CMDQ driver
> >   arm64: dts: mt8173: Add GCE node
> >   CMDQ: save more energy in idle
> >
> >  .../devicetree/bindings/soc/mediatek/gce.txt       |  44 +
> >  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  10 +
> >  drivers/soc/mediatek/Kconfig                       |  11 +
> >  drivers/soc/mediatek/Makefile                      |   1 +
> >  drivers/soc/mediatek/mtk-cmdq.c                    | 983 +++++++++++++++++++++
> 
> The driver uses the mailbox framework, so it should live in the 
> drivers/mailbox folder.

As you know, the maximum number of gce threads is 16.
However, we plan to support more clients in the future,
and they may need to use more than 16 gce threads.

For this issue, our plan is to let multiple clients share the same gce
thread; i.e. we will acquire gce thread for client dynamically by
internal policy in cmdq driver.
Unfortunately. mailbox channel has exclusive feature.
Quote from comment of mbox_request_channel().
"The channel is exclusively allocated and can't be used by another
client before the owner calls mbox_free_channel."
Therefore, we plan to remove mailbox framework from cmdq driver in the
future.

For current status, could you tell us your preference?
(1) Move cmdq to mailbox folder and then move it back here when we
submit another patch to support more than 16 gce clients; or
(2) Remove mailbox framework from cmdq now for its future extensibility.

Thanks,
HS

> Please don't forget to add Jassi to the loop, as he is the maintainer of 
> this subsystem and the last instance to decide if/when the driver will 
> get accepted.
> 
> Thanks,
> Matthias
> 

[toc] | [next] | [standalone]

#1470163

On Thu, Aug 25, 2016 at 7:07 PM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> Hi Matthias,
>
> On Wed, 2016-08-24 at 13:00 +0200, Matthias Brugger wrote:
>> On 24/08/16 05:27, HS Liao wrote:
> [...]
>> > Changes since v12:
>> >  - remove mediatek,gce from device tree
>>
>> Why? The binding got accepted by Rob.
>
> We can get cmdq dev from mailbox controller,
> so we don't need mediatek,gce anymore.
>
>> >
>> > Best regards,
>> > HS Liao
>> >
>> > HS Liao (4):
>> >   dt-bindings: soc: Add documentation for the MediaTek GCE unit
>> >   CMDQ: Mediatek CMDQ driver
>> >   arm64: dts: mt8173: Add GCE node
>> >   CMDQ: save more energy in idle
>> >
>> >  .../devicetree/bindings/soc/mediatek/gce.txt       |  44 +
>> >  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  10 +
>> >  drivers/soc/mediatek/Kconfig                       |  11 +
>> >  drivers/soc/mediatek/Makefile                      |   1 +
>> >  drivers/soc/mediatek/mtk-cmdq.c                    | 983 +++++++++++++++++++++
>>
>> The driver uses the mailbox framework, so it should live in the
>> drivers/mailbox folder.
>
> As you know, the maximum number of gce threads is 16.
> However, we plan to support more clients in the future,
> and they may need to use more than 16 gce threads.
>
> For this issue, our plan is to let multiple clients share the same gce
> thread; i.e. we will acquire gce thread for client dynamically by
> internal policy in cmdq driver.
> Unfortunately. mailbox channel has exclusive feature.
> Quote from comment of mbox_request_channel().
> "The channel is exclusively allocated and can't be used by another
> client before the owner calls mbox_free_channel."
> Therefore, we plan to remove mailbox framework from cmdq driver in the
> future.
>
Platforms that need shared access to a channel, implement a 'server'
driver that serialise (which is needed still) the access to common
channel. If you think you don't need mutual exclusion and don't care
about replies, simply share the mailbox handle among different
clients.

[toc] | [prev] | [next] | [standalone]

#1473165

Hi Jassi,

On Thu, 2016-08-25 at 19:12 +0530, Jassi Brar wrote:
> On Thu, Aug 25, 2016 at 7:07 PM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
> > Hi Matthias,
> >
> > On Wed, 2016-08-24 at 13:00 +0200, Matthias Brugger wrote:
> >> On 24/08/16 05:27, HS Liao wrote:
[...]
> >> > HS Liao (4):
> >> >   dt-bindings: soc: Add documentation for the MediaTek GCE unit
> >> >   CMDQ: Mediatek CMDQ driver
> >> >   arm64: dts: mt8173: Add GCE node
> >> >   CMDQ: save more energy in idle
> >> >
> >> >  .../devicetree/bindings/soc/mediatek/gce.txt       |  44 +
> >> >  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  10 +
> >> >  drivers/soc/mediatek/Kconfig                       |  11 +
> >> >  drivers/soc/mediatek/Makefile                      |   1 +
> >> >  drivers/soc/mediatek/mtk-cmdq.c                    | 983 +++++++++++++++++++++
> >>
> >> The driver uses the mailbox framework, so it should live in the
> >> drivers/mailbox folder.
> >
> > As you know, the maximum number of gce threads is 16.
> > However, we plan to support more clients in the future,
> > and they may need to use more than 16 gce threads.
> >
> > For this issue, our plan is to let multiple clients share the same gce
> > thread; i.e. we will acquire gce thread for client dynamically by
> > internal policy in cmdq driver.
> > Unfortunately. mailbox channel has exclusive feature.
> > Quote from comment of mbox_request_channel().
> > "The channel is exclusively allocated and can't be used by another
> > client before the owner calls mbox_free_channel."
> > Therefore, we plan to remove mailbox framework from cmdq driver in the
> > future.
> >
> Platforms that need shared access to a channel, implement a 'server'
> driver that serialise (which is needed still) the access to common
> channel. If you think you don't need mutual exclusion and don't care
> about replies, simply share the mailbox handle among different
> clients.

Thank you for your kindly reply.
We would like to discuss further with you on this topic.

Our requirement is
(1) cmdq task cannot be split, and
(2) cmdq thread can have multiple cmdq tasks from different clients.

According to your comment "implement a 'server' driver that serialise
the access to common channel", do you mean we should implement cmdq
client (mailbox client) as a server and other clients call the functions
of cmdq client?

clients --> cmdq client (mailbox client) --> cmdq (mailbox controller)

If so, could you please tell us the benefit of using mailbox framework?

Our original plan is to let cmdq driver manage cmdq thread internally.
Cmdq driver can choose a suitable cmdq thread to execute a flushed cmdq
task dynamically, and client doesn't need to know the existence of cmdq
thread.


Could you also please tell us the purpose of putting all mailbox
driver into mailbox folder?
We know that some other drivers also follow this rule, and just want
to know more details.

Thanks,
HS

[toc] | [prev] | [next] | [standalone]

#1473195

On Wed, Aug 31, 2016 at 1:43 PM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:

>> >> The driver uses the mailbox framework, so it should live in the
>> >> drivers/mailbox folder.
>> >
>> > As you know, the maximum number of gce threads is 16.
>> > However, we plan to support more clients in the future,
>> > and they may need to use more than 16 gce threads.
>> >
>> > For this issue, our plan is to let multiple clients share the same gce
>> > thread; i.e. we will acquire gce thread for client dynamically by
>> > internal policy in cmdq driver.
>> > Unfortunately. mailbox channel has exclusive feature.
>> > Quote from comment of mbox_request_channel().
>> > "The channel is exclusively allocated and can't be used by another
>> > client before the owner calls mbox_free_channel."
>> > Therefore, we plan to remove mailbox framework from cmdq driver in the
>> > future.
>> >
>> Platforms that need shared access to a channel, implement a 'server'
>> driver that serialise (which is needed still) the access to common
>> channel. If you think you don't need mutual exclusion and don't care
>> about replies, simply share the mailbox handle among different
>> clients.
>
> Thank you for your kindly reply.
> We would like to discuss further with you on this topic.
>
> Our requirement is
> (1) cmdq task cannot be split, and
> (2) cmdq thread can have multiple cmdq tasks from different clients.
>
> According to your comment "implement a 'server' driver that serialise
> the access to common channel", do you mean we should implement cmdq
> client (mailbox client) as a server and other clients call the functions
> of cmdq client?
>
> clients --> cmdq client (mailbox client) --> cmdq (mailbox controller)
>
> If so, could you please tell us the benefit of using mailbox framework?
>
You don't have to reinvent 80% of the wheel and reuse the mailbox.c
core that supports many features and is tested on many platforms. Your
implementation is going to be quite similar, only you clump all the
code in one file and you use different terminology.

You said "we will acquire gce thread for client dynamically by
internal policy in cmdq driver"
On mailbox api, this maps to simply sharing the channel/thread handle,
protected by a lock, among clients on some basis (like FCFS or
whatever you internal policy is). So your server driver could be very
thin. And all your clients could follow the mailbox api (which is good
from the point of reusability/portability).

> Our original plan is to let cmdq driver manage cmdq thread internally.
> Cmdq driver can choose a suitable cmdq thread to execute a flushed cmdq
> task dynamically, and client doesn't need to know the existence of cmdq
> thread.
>
>
> Could you also please tell us the purpose of putting all mailbox
> driver into mailbox folder?
> We know that some other drivers also follow this rule, and just want
> to know more details.
>
Any driver that implements the Mailbox API should live in
drivers/mailbox/.  And why you should implement mailbox api, is
explained above.

[toc] | [prev] | [next] | [standalone]

#1474963

Hi Jassi,

On Wed, 2016-08-31 at 14:15 +0530, Jassi Brar wrote:
> On Wed, Aug 31, 2016 at 1:43 PM, Horng-Shyang Liao <hs.liao@mediatek.com> wrote:
[...]
> >> Platforms that need shared access to a channel, implement a 'server'
> >> driver that serialise (which is needed still) the access to common
> >> channel. If you think you don't need mutual exclusion and don't care
> >> about replies, simply share the mailbox handle among different
> >> clients.
> >
> > Thank you for your kindly reply.
> > We would like to discuss further with you on this topic.
> >
> > Our requirement is
> > (1) cmdq task cannot be split, and
> > (2) cmdq thread can have multiple cmdq tasks from different clients.
> >
> > According to your comment "implement a 'server' driver that serialise
> > the access to common channel", do you mean we should implement cmdq
> > client (mailbox client) as a server and other clients call the functions
> > of cmdq client?
> >
> > clients --> cmdq client (mailbox client) --> cmdq (mailbox controller)
> >
> > If so, could you please tell us the benefit of using mailbox framework?
> >
> You don't have to reinvent 80% of the wheel and reuse the mailbox.c
> core that supports many features and is tested on many platforms. Your
> implementation is going to be quite similar, only you clump all the
> code in one file and you use different terminology.
> 
> You said "we will acquire gce thread for client dynamically by
> internal policy in cmdq driver"
> On mailbox api, this maps to simply sharing the channel/thread handle,
> protected by a lock, among clients on some basis (like FCFS or
> whatever you internal policy is). So your server driver could be very
> thin. And all your clients could follow the mailbox api (which is good
> from the point of reusability/portability).
> 
> > Our original plan is to let cmdq driver manage cmdq thread internally.
> > Cmdq driver can choose a suitable cmdq thread to execute a flushed cmdq
> > task dynamically, and client doesn't need to know the existence of cmdq
> > thread.
> >
> >
> > Could you also please tell us the purpose of putting all mailbox
> > driver into mailbox folder?
> > We know that some other drivers also follow this rule, and just want
> > to know more details.
> >
> Any driver that implements the Mailbox API should live in
> drivers/mailbox/.  And why you should implement mailbox api, is
> explained above.

Thank you for your explanation.
I will move cmdq driver to mailbox folder in the next version.

Thanks,
HS

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web