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

Groups > linux.kernel > #1312171 > unrolled thread

Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288

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 0/5] clk: rockchip: add full support for HDMI clock on rk3288 Tomeu Vizoso <tomeu@tomeuvizoso.net> - 2016-01-19 13:10 +0100
    Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288 Doug Anderson <dianders@chromium.org> - 2016-01-20 18:00 +0100
      Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288 Tomeu Vizoso <tomeu@tomeuvizoso.net> - 2016-01-21 10:10 +0100
        Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288 Doug Anderson <dianders@chromium.org> - 2016-01-21 21:20 +0100
          Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288 Tomeu Vizoso <tomeu@tomeuvizoso.net> - 2016-01-22 15:10 +0100
            Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288 Doug Anderson <dianders@chromium.org> - 2016-01-22 18:10 +0100
              Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288 Tomeu Vizoso <tomeu@tomeuvizoso.net> - 2016-01-26 09:30 +0100
                Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288 Doug Anderson <dianders@chromium.org> - 2016-01-26 17:40 +0100
                  Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288 Tomeu Vizoso <tomeu@tomeuvizoso.net> - 2016-01-27 11:30 +0100
                    Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288 Doug Anderson <dianders@chromium.org> - 2016-01-27 17:50 +0100

#1312171 — Re: [PATCH 0/5] clk: rockchip: add full support for HDMI clock on rk3288

On 13 November 2014 at 23:59, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Thu, Nov 13, 2014 at 12:52 AM, Kever Yang <kever.yang@rock-chips.com> wrote:
>> Hi Heiko,
>>
>> On 11/07/2014 05:06 AM, Heiko Stübner wrote:
>>>
>>> Hi Kever,
>>>
>>> Am Dienstag, 4. November 2014, 15:52:34 schrieb Kever Yang:
>>>>
>>>> we are going to make a clock usage solution for rk3288:
>>>> 1. CPLL and GPLL always not change after assign init;
>>>> 2. NPLL default as 500MHz, may used for most scene;
>>>> 3. NPLL may be changed by VOP(HDMI) clock for some special
>>>>     frequency requirement.
>>>>
>>>>      I test it with rk3288 evb on top of Heiko's clk-for-next
>>>
>>> In general I'm not really sure if allowing one component to arbitarily
>>> change
>>> a shared clock wouldn't result in trouble.
>>>
>>> At the moment only dclk_vop0 is included in your series, while the hdmi
>>> controller can connect to both vop0 and vop1.
>>> And as Doug mentioned the gpu also has the npll as one possible source.
>>
>> I think the problem GPU HANGs with 480MHz clock from usbphy has
>> been fixed with my patch to gerrit:
>> https://chromium-review.googlesource.com/#/c/229554/
>>>
>>>
>>> Looking through the clock-tree there are a lot more components possibly
>>> using
>>> (or wanting to use) the npll: of course the VOPs, the edp, hdmi, isp,
>>> hevc,
>>> gpu, tsp uart0 and gmac. So I'm slightly uncomfortable with somehow
>>> reserving
>>> the npll for VOP0 alone.
>>
>> It's true that I customized the usage of npll for VOP0 when we need some
>> very special frequency, but it doesn't means other modules can't use the
>> npll, it will always decided by clock core for module clocks that which
>> parent
>> is the best.
>
> We will just need to be very careful.  As I've mentioned in the past
> we'll need to think about what happens to other clocks that happen to
> be parented by NPLL whenever we change it.
>
> So if we do this:
>
> 1. NPLL happens to be 500MHz.
> 2. We set GPU to be 500MHz and it picks NPLL.
> 3. We change NPLL to a different speed (like 600MHz).
>
> ...I believe in this scenario the GPU would start running at 600MHz
> immediately.  We'd need to add special code to watch out for this and
> pick an alternate clock for the GPU (like the USB 480) before the NPLL
> change.  If NPLL later changes back to 500MHz and the GPU still wanted
> 500MHz, we'd have to decide what to do.
>
> The summary is: it's pretty complicated

Hello,

I have heard that this is still an issue in RK3288 support in mainline
so have given a look and wonder if you have considered having the
consumers of a shared clock (which could be a child clock) use
PRE_RATE_CHANGE notifications to adapt to changes in a parent's rate,
or to abort them if that's not possible.

Alternatively, a consumer can set min and max constraints if it
doesn't want for others to mess with its provider, and let the other
consumer deal with failed rate changes (probably by choosing another
provider).

Have I missed any problems with that?

Thanks,

Tomeu

[toc] | [next] | [standalone]

#1313370

Tomeu,

On Tue, Jan 19, 2016 at 4:02 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
> On 13 November 2014 at 23:59, Doug Anderson <dianders@chromium.org> wrote:
>> Hi,
>>
>> On Thu, Nov 13, 2014 at 12:52 AM, Kever Yang <kever.yang@rock-chips.com> wrote:
>>> Hi Heiko,
>>>
>>> On 11/07/2014 05:06 AM, Heiko Stübner wrote:
>>>>
>>>> Hi Kever,
>>>>
>>>> Am Dienstag, 4. November 2014, 15:52:34 schrieb Kever Yang:
>>>>>
>>>>> we are going to make a clock usage solution for rk3288:
>>>>> 1. CPLL and GPLL always not change after assign init;
>>>>> 2. NPLL default as 500MHz, may used for most scene;
>>>>> 3. NPLL may be changed by VOP(HDMI) clock for some special
>>>>>     frequency requirement.
>>>>>
>>>>>      I test it with rk3288 evb on top of Heiko's clk-for-next
>>>>
>>>> In general I'm not really sure if allowing one component to arbitarily
>>>> change
>>>> a shared clock wouldn't result in trouble.
>>>>
>>>> At the moment only dclk_vop0 is included in your series, while the hdmi
>>>> controller can connect to both vop0 and vop1.
>>>> And as Doug mentioned the gpu also has the npll as one possible source.
>>>
>>> I think the problem GPU HANGs with 480MHz clock from usbphy has
>>> been fixed with my patch to gerrit:
>>> https://chromium-review.googlesource.com/#/c/229554/
>>>>
>>>>
>>>> Looking through the clock-tree there are a lot more components possibly
>>>> using
>>>> (or wanting to use) the npll: of course the VOPs, the edp, hdmi, isp,
>>>> hevc,
>>>> gpu, tsp uart0 and gmac. So I'm slightly uncomfortable with somehow
>>>> reserving
>>>> the npll for VOP0 alone.
>>>
>>> It's true that I customized the usage of npll for VOP0 when we need some
>>> very special frequency, but it doesn't means other modules can't use the
>>> npll, it will always decided by clock core for module clocks that which
>>> parent
>>> is the best.
>>
>> We will just need to be very careful.  As I've mentioned in the past
>> we'll need to think about what happens to other clocks that happen to
>> be parented by NPLL whenever we change it.
>>
>> So if we do this:
>>
>> 1. NPLL happens to be 500MHz.
>> 2. We set GPU to be 500MHz and it picks NPLL.
>> 3. We change NPLL to a different speed (like 600MHz).
>>
>> ...I believe in this scenario the GPU would start running at 600MHz
>> immediately.  We'd need to add special code to watch out for this and
>> pick an alternate clock for the GPU (like the USB 480) before the NPLL
>> change.  If NPLL later changes back to 500MHz and the GPU still wanted
>> 500MHz, we'd have to decide what to do.
>>
>> The summary is: it's pretty complicated
>
> Hello,
>
> I have heard that this is still an issue in RK3288 support in mainline
> so have given a look and wonder if you have considered having the
> consumers of a shared clock (which could be a child clock) use
> PRE_RATE_CHANGE notifications to adapt to changes in a parent's rate,
> or to abort them if that's not possible.
>
> Alternatively, a consumer can set min and max constraints if it
> doesn't want for others to mess with its provider, and let the other
> consumer deal with failed rate changes (probably by choosing another
> provider).
>
> Have I missed any problems with that?

I don't think it's really that simple, but I certainly could be wrong.

Summary of the system:

* There are two VOPs (video processors) on rk3288: the big one and the
little one.

* The big VOP can handle higher resolutions than the little one but
uses more power.

* Both VOPs two can be sourced from CPLL, GPLL, or NPLL.

* Both VOPs can be setup to process video for either HDMI, eDP, LVDS, ...

* NPLL is the "new" PLL and is kinda like an extra PLL that's not used
for any of the core peripherals.  It's intended to be one that's
changed dynamically based on system needs.

* GPLL appears intended (in current designs) to run at 594 MHz on
rk3288 using a special "low jitter" setting.  This is intended to be
the source clock for HDMI when running the very common 148.5 and 74.25
rates.  It also it intended to be the source clock for various other
peripherals like i2c, eMMC, SD, etc.  This also runs the GPU when the
GPU is running at ~600MHz or ~300MHz.

* CPLL appears intended (in current designs) to run at 400MHz.  There
are some peripherals that run off this plus the GPU at 400MHz, 200MHz,
or 100MHz.


Let's imagine that we're booting up on a laptop where the builtin
panel is hooked up via eDP.  Let's say that the panel really wants
76.42 MHz but that will work just fine with 74.25 MHz too.  If this is
a system that has no other graphics needs, we'd want eDP to take NPLL
and grab 76.42 MHz.  Make the best rate, right?

...but what about if it's a system with an HDMI port?  The eDP panel
is built in and we know it works with 74.25 MHz, but the user could
plug into all sorts of HDMI devices and they may require some very
special PLL rates to get there.  So if we happen to take NPLL for eDP
we'll need to know to switch away from it once something is plugged
into HDMI.  We don't want to block the HDMI request since we know we
can run eDP at 74.25 off GPLL.  Presumably we could see the HDMI
request and then try to remux the eDP on the fly.  ...but what decides
the HDMI is more important to eDP?  You might want different rules if
the eDP port is somehow exposed and the user might plug different
panels into it...


-Doug

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

#1314017

On 20 January 2016 at 17:50, Doug Anderson <dianders@chromium.org> wrote:
> Tomeu,
>
> On Tue, Jan 19, 2016 at 4:02 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>> On 13 November 2014 at 23:59, Doug Anderson <dianders@chromium.org> wrote:
>>> Hi,
>>>
>>> On Thu, Nov 13, 2014 at 12:52 AM, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>> Hi Heiko,
>>>>
>>>> On 11/07/2014 05:06 AM, Heiko Stübner wrote:
>>>>>
>>>>> Hi Kever,
>>>>>
>>>>> Am Dienstag, 4. November 2014, 15:52:34 schrieb Kever Yang:
>>>>>>
>>>>>> we are going to make a clock usage solution for rk3288:
>>>>>> 1. CPLL and GPLL always not change after assign init;
>>>>>> 2. NPLL default as 500MHz, may used for most scene;
>>>>>> 3. NPLL may be changed by VOP(HDMI) clock for some special
>>>>>>     frequency requirement.
>>>>>>
>>>>>>      I test it with rk3288 evb on top of Heiko's clk-for-next
>>>>>
>>>>> In general I'm not really sure if allowing one component to arbitarily
>>>>> change
>>>>> a shared clock wouldn't result in trouble.
>>>>>
>>>>> At the moment only dclk_vop0 is included in your series, while the hdmi
>>>>> controller can connect to both vop0 and vop1.
>>>>> And as Doug mentioned the gpu also has the npll as one possible source.
>>>>
>>>> I think the problem GPU HANGs with 480MHz clock from usbphy has
>>>> been fixed with my patch to gerrit:
>>>> https://chromium-review.googlesource.com/#/c/229554/
>>>>>
>>>>>
>>>>> Looking through the clock-tree there are a lot more components possibly
>>>>> using
>>>>> (or wanting to use) the npll: of course the VOPs, the edp, hdmi, isp,
>>>>> hevc,
>>>>> gpu, tsp uart0 and gmac. So I'm slightly uncomfortable with somehow
>>>>> reserving
>>>>> the npll for VOP0 alone.
>>>>
>>>> It's true that I customized the usage of npll for VOP0 when we need some
>>>> very special frequency, but it doesn't means other modules can't use the
>>>> npll, it will always decided by clock core for module clocks that which
>>>> parent
>>>> is the best.
>>>
>>> We will just need to be very careful.  As I've mentioned in the past
>>> we'll need to think about what happens to other clocks that happen to
>>> be parented by NPLL whenever we change it.
>>>
>>> So if we do this:
>>>
>>> 1. NPLL happens to be 500MHz.
>>> 2. We set GPU to be 500MHz and it picks NPLL.
>>> 3. We change NPLL to a different speed (like 600MHz).
>>>
>>> ...I believe in this scenario the GPU would start running at 600MHz
>>> immediately.  We'd need to add special code to watch out for this and
>>> pick an alternate clock for the GPU (like the USB 480) before the NPLL
>>> change.  If NPLL later changes back to 500MHz and the GPU still wanted
>>> 500MHz, we'd have to decide what to do.
>>>
>>> The summary is: it's pretty complicated
>>
>> Hello,
>>
>> I have heard that this is still an issue in RK3288 support in mainline
>> so have given a look and wonder if you have considered having the
>> consumers of a shared clock (which could be a child clock) use
>> PRE_RATE_CHANGE notifications to adapt to changes in a parent's rate,
>> or to abort them if that's not possible.
>>
>> Alternatively, a consumer can set min and max constraints if it
>> doesn't want for others to mess with its provider, and let the other
>> consumer deal with failed rate changes (probably by choosing another
>> provider).
>>
>> Have I missed any problems with that?
>
> I don't think it's really that simple, but I certainly could be wrong.
>
> Summary of the system:
>
> * There are two VOPs (video processors) on rk3288: the big one and the
> little one.
>
> * The big VOP can handle higher resolutions than the little one but
> uses more power.
>
> * Both VOPs two can be sourced from CPLL, GPLL, or NPLL.
>
> * Both VOPs can be setup to process video for either HDMI, eDP, LVDS, ...
>
> * NPLL is the "new" PLL and is kinda like an extra PLL that's not used
> for any of the core peripherals.  It's intended to be one that's
> changed dynamically based on system needs.
>
> * GPLL appears intended (in current designs) to run at 594 MHz on
> rk3288 using a special "low jitter" setting.  This is intended to be
> the source clock for HDMI when running the very common 148.5 and 74.25
> rates.  It also it intended to be the source clock for various other
> peripherals like i2c, eMMC, SD, etc.  This also runs the GPU when the
> GPU is running at ~600MHz or ~300MHz.
>
> * CPLL appears intended (in current designs) to run at 400MHz.  There
> are some peripherals that run off this plus the GPU at 400MHz, 200MHz,
> or 100MHz.
>
>
> Let's imagine that we're booting up on a laptop where the builtin
> panel is hooked up via eDP.  Let's say that the panel really wants
> 76.42 MHz but that will work just fine with 74.25 MHz too.  If this is
> a system that has no other graphics needs, we'd want eDP to take NPLL
> and grab 76.42 MHz.  Make the best rate, right?
>
> ...but what about if it's a system with an HDMI port?  The eDP panel
> is built in and we know it works with 74.25 MHz, but the user could
> plug into all sorts of HDMI devices and they may require some very
> special PLL rates to get there.  So if we happen to take NPLL for eDP
> we'll need to know to switch away from it once something is plugged
> into HDMI.  We don't want to block the HDMI request since we know we
> can run eDP at 74.25 off GPLL.  Presumably we could see the HDMI
> request and then try to remux the eDP on the fly.  ...but what decides
> the HDMI is more important to eDP?  You might want different rules if
> the eDP port is somehow exposed and the user might plug different
> panels into it...

So we have a mechanism for detecting a conflict in the clock
hierarchy, and a mechanism to solve it, but we are missing a way for
userspace to communicate policy regarding which clocks should be given
priority when solving such a conflict?

Regards,

Tomeu

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

#1314487

Hi,

On Thu, Jan 21, 2016 at 1:03 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
> So we have a mechanism for detecting a conflict in the clock
> hierarchy, and a mechanism to solve it, but we are missing a way for
> userspace to communicate policy regarding which clocks should be given
> priority when solving such a conflict?

Hrmmm, I guess it could be userspace that makes the decision.  It does
seem a little odd to force it to userspace in all cases, though.  For
a particular laptop that is designed with a specific panel connected
up eDP it seems less than ideal to push this into userspace.  If the
kernel could just work in the expected sane way (or at least work that
way by default) it would be ideal.

If the kernel doesn't try to do anything sane by default then you're
creating a requirement for everyone's userspace to somehow figure this
out.  Do you expect there to be UI here, or that this would be
something that would be figured out by the Linux distribution?
Certainly exposing UI on something like a laptop with a builtin panel
wouldn't make any sense to me, but it might make sense if you had an
eval board with different display connectors on it.  If there's no UI,
would the Linux distribution need to somehow identify which board we
were on and then have a big lookup table about how to configure
things?

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

#1315007

On 21 January 2016 at 21:11, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Thu, Jan 21, 2016 at 1:03 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>> So we have a mechanism for detecting a conflict in the clock
>> hierarchy, and a mechanism to solve it, but we are missing a way for
>> userspace to communicate policy regarding which clocks should be given
>> priority when solving such a conflict?
>
> Hrmmm, I guess it could be userspace that makes the decision.  It does
> seem a little odd to force it to userspace in all cases, though.  For
> a particular laptop that is designed with a specific panel connected
> up eDP it seems less than ideal to push this into userspace.  If the
> kernel could just work in the expected sane way (or at least work that
> way by default) it would be ideal.

Ah, I was wrongly assuming that the kernel didn't have enough
information to make an informed decision in this case, sorry.

Guess the per-user rate limits don't help here because the consumer
with higher priority could work with frequencies other than the ideal.

And we cannot have a consumer listening for PRE_RATE_CHANGE and
aborting unwanted changes or rerouting the ancestors of the clocks of
other consumers because that would be a massive violation of
separation of concerns.

If we were to rearrange the clock topology from within the CCF, then
consumers need to have a way to communicate to the core that they are
more important than other consumers. clk_set_important(clk, true)
could be enough in this case, but would be insufficient in more
complex cases where more than two clocks could use the same PLL.

> If the kernel doesn't try to do anything sane by default then you're
> creating a requirement for everyone's userspace to somehow figure this
> out.  Do you expect there to be UI here, or that this would be
> something that would be figured out by the Linux distribution?
> Certainly exposing UI on something like a laptop with a builtin panel
> wouldn't make any sense to me, but it might make sense if you had an
> eval board with different display connectors on it.  If there's no UI,
> would the Linux distribution need to somehow identify which board we
> were on and then have a big lookup table about how to configure
> things?

If we don't actually need input from userspace for this use case, I
wouldn't go this way right now, because it seems to me like it could
be a really big timesink for little gain.

Once someone comes with a situation in which feedback from userspace
is really needed, that person can propose such an interface ;)

Regards,

Tomeu

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

#1315141

Tomeu,

On Fri, Jan 22, 2016 at 6:00 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
> On 21 January 2016 at 21:11, Doug Anderson <dianders@chromium.org> wrote:
>> Hi,
>>
>> On Thu, Jan 21, 2016 at 1:03 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>>> So we have a mechanism for detecting a conflict in the clock
>>> hierarchy, and a mechanism to solve it, but we are missing a way for
>>> userspace to communicate policy regarding which clocks should be given
>>> priority when solving such a conflict?
>>
>> Hrmmm, I guess it could be userspace that makes the decision.  It does
>> seem a little odd to force it to userspace in all cases, though.  For
>> a particular laptop that is designed with a specific panel connected
>> up eDP it seems less than ideal to push this into userspace.  If the
>> kernel could just work in the expected sane way (or at least work that
>> way by default) it would be ideal.
>
> Ah, I was wrongly assuming that the kernel didn't have enough
> information to make an informed decision in this case, sorry.
>
> Guess the per-user rate limits don't help here because the consumer
> with higher priority could work with frequencies other than the ideal.
>
> And we cannot have a consumer listening for PRE_RATE_CHANGE and
> aborting unwanted changes or rerouting the ancestors of the clocks of
> other consumers because that would be a massive violation of
> separation of concerns.
>
> If we were to rearrange the clock topology from within the CCF, then
> consumers need to have a way to communicate to the core that they are
> more important than other consumers. clk_set_important(clk, true)
> could be enough in this case, but would be insufficient in more
> complex cases where more than two clocks could use the same PLL.

With something like the above I'd still expect some complexity
depending on the probe order.  If a less important device grabs the
clock first, it might be forced to re-think its clocks later.  That
might be disconcerting.

OK, so I was just involved in a change recently that made me realize
that maybe our original problems were tied to the fact that our
builtin panels were trying to specify a clock that was impossible to
achieve with CPLL / GPLL.  It was a known problem that the request
would be denied and the CCF would just pick the closest rate it could.
Probably the right thing is to solve _that_ problem first.  If using
simple panel you could do a change like
<https://chromium-review.googlesource.com/#/c/323211/> (though
presumably you'd have to handle people using the same panel in other
laptops).  You might also be able to do funny things to fixup the mode
like dbehr tried to do in
<https://chromium-review.googlesource.com/#/c/270017/>.  By doing this
and making sure that

With that being said, how about this as a solution:

1. Figure some way (presumably some type of device tree property?) to
make sure that the builtin panel chose a clock that was achievable by
dividing CPLL / GPLL on rk3288.  If you have eDP but no builtin panel
you wouldn't specify this and you'd just let the system pick whatever
it wanted.

2. We use "assigned-clocks" to (by default) make NPLL something silly
and unattractive.  This will keep the common clock framework from
picking it to use as the source if the clock could be made just as
well from CPLL / GPLL (or find some other way to make NPLL a lower
priority muxing option).

3. We use the "dibs" rule in that the first display driver to claim
NPLL blocks anyone else from changing things.


In the above on a laptop with a builtin panel and an HDMI port you'd
get the panel using CPLL / GPLL and HDMI getting CPLL, GPLL or NPLL
(and it would be able to change NPLL if needed).


In the above on a dev board with a lot of ports then the first device
probed would get the most flexibility.  Future devices would be
limited in their choices.

-Doug

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

#1317622

On 22 January 2016 at 18:07, Doug Anderson <dianders@chromium.org> wrote:
> Tomeu,
>
> On Fri, Jan 22, 2016 at 6:00 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>> On 21 January 2016 at 21:11, Doug Anderson <dianders@chromium.org> wrote:
>>> Hi,
>>>
>>> On Thu, Jan 21, 2016 at 1:03 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>>>> So we have a mechanism for detecting a conflict in the clock
>>>> hierarchy, and a mechanism to solve it, but we are missing a way for
>>>> userspace to communicate policy regarding which clocks should be given
>>>> priority when solving such a conflict?
>>>
>>> Hrmmm, I guess it could be userspace that makes the decision.  It does
>>> seem a little odd to force it to userspace in all cases, though.  For
>>> a particular laptop that is designed with a specific panel connected
>>> up eDP it seems less than ideal to push this into userspace.  If the
>>> kernel could just work in the expected sane way (or at least work that
>>> way by default) it would be ideal.
>>
>> Ah, I was wrongly assuming that the kernel didn't have enough
>> information to make an informed decision in this case, sorry.
>>
>> Guess the per-user rate limits don't help here because the consumer
>> with higher priority could work with frequencies other than the ideal.
>>
>> And we cannot have a consumer listening for PRE_RATE_CHANGE and
>> aborting unwanted changes or rerouting the ancestors of the clocks of
>> other consumers because that would be a massive violation of
>> separation of concerns.
>>
>> If we were to rearrange the clock topology from within the CCF, then
>> consumers need to have a way to communicate to the core that they are
>> more important than other consumers. clk_set_important(clk, true)
>> could be enough in this case, but would be insufficient in more
>> complex cases where more than two clocks could use the same PLL.
>
> With something like the above I'd still expect some complexity
> depending on the probe order.  If a less important device grabs the
> clock first, it might be forced to re-think its clocks later.  That
> might be disconcerting.

How much disconcerting do you think this could be? Hopefully those
devices should probe quite close to each other, right?

> OK, so I was just involved in a change recently that made me realize
> that maybe our original problems were tied to the fact that our
> builtin panels were trying to specify a clock that was impossible to
> achieve with CPLL / GPLL.  It was a known problem that the request
> would be denied and the CCF would just pick the closest rate it could.
> Probably the right thing is to solve _that_ problem first.  If using
> simple panel you could do a change like
> <https://chromium-review.googlesource.com/#/c/323211/> (though
> presumably you'd have to handle people using the same panel in other
> laptops).  You might also be able to do funny things to fixup the mode
> like dbehr tried to do in
> <https://chromium-review.googlesource.com/#/c/270017/>.  By doing this
> and making sure that

Are we missing something here?

> With that being said, how about this as a solution:
>
> 1. Figure some way (presumably some type of device tree property?) to
> make sure that the builtin panel chose a clock that was achievable by
> dividing CPLL / GPLL on rk3288.  If you have eDP but no builtin panel
> you wouldn't specify this and you'd just let the system pick whatever
> it wanted.
>
> 2. We use "assigned-clocks" to (by default) make NPLL something silly
> and unattractive.  This will keep the common clock framework from
> picking it to use as the source if the clock could be made just as
> well from CPLL / GPLL (or find some other way to make NPLL a lower
> priority muxing option).
>
> 3. We use the "dibs" rule in that the first display driver to claim
> NPLL blocks anyone else from changing things.
>
>
> In the above on a laptop with a builtin panel and an HDMI port you'd
> get the panel using CPLL / GPLL and HDMI getting CPLL, GPLL or NPLL
> (and it would be able to change NPLL if needed).
>
>
> In the above on a dev board with a lot of ports then the first device
> probed would get the most flexibility.  Future devices would be
> limited in their choices.

Not sure of this, sounds a bit ad-hoc and may be avoiding to address
limitations in the CCF that need to be solved anyway? So far it still
looks to me like the CCF needs to get better at allocating limited
resources.

Mike, Stephen, what do you think?

Thanks,

Tomeu

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

#1318130

Tomeu,

On Tue, Jan 26, 2016 at 12:28 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
> On 22 January 2016 at 18:07, Doug Anderson <dianders@chromium.org> wrote:
>> Tomeu,
>>
>> On Fri, Jan 22, 2016 at 6:00 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>>> On 21 January 2016 at 21:11, Doug Anderson <dianders@chromium.org> wrote:
>>>> Hi,
>>>>
>>>> On Thu, Jan 21, 2016 at 1:03 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>>>>> So we have a mechanism for detecting a conflict in the clock
>>>>> hierarchy, and a mechanism to solve it, but we are missing a way for
>>>>> userspace to communicate policy regarding which clocks should be given
>>>>> priority when solving such a conflict?
>>>>
>>>> Hrmmm, I guess it could be userspace that makes the decision.  It does
>>>> seem a little odd to force it to userspace in all cases, though.  For
>>>> a particular laptop that is designed with a specific panel connected
>>>> up eDP it seems less than ideal to push this into userspace.  If the
>>>> kernel could just work in the expected sane way (or at least work that
>>>> way by default) it would be ideal.
>>>
>>> Ah, I was wrongly assuming that the kernel didn't have enough
>>> information to make an informed decision in this case, sorry.
>>>
>>> Guess the per-user rate limits don't help here because the consumer
>>> with higher priority could work with frequencies other than the ideal.
>>>
>>> And we cannot have a consumer listening for PRE_RATE_CHANGE and
>>> aborting unwanted changes or rerouting the ancestors of the clocks of
>>> other consumers because that would be a massive violation of
>>> separation of concerns.
>>>
>>> If we were to rearrange the clock topology from within the CCF, then
>>> consumers need to have a way to communicate to the core that they are
>>> more important than other consumers. clk_set_important(clk, true)
>>> could be enough in this case, but would be insufficient in more
>>> complex cases where more than two clocks could use the same PLL.
>>
>> With something like the above I'd still expect some complexity
>> depending on the probe order.  If a less important device grabs the
>> clock first, it might be forced to re-think its clocks later.  That
>> might be disconcerting.
>
> How much disconcerting do you think this could be? Hopefully those
> devices should probe quite close to each other, right?

Probe: probably, though with defers it could be several seconds.

...but remember that display interfaces tend to be hotplug.  That
might mean that the HDMI interface won't try to set the clock until
much, much later.


>> OK, so I was just involved in a change recently that made me realize
>> that maybe our original problems were tied to the fact that our
>> builtin panels were trying to specify a clock that was impossible to
>> achieve with CPLL / GPLL.  It was a known problem that the request
>> would be denied and the CCF would just pick the closest rate it could.
>> Probably the right thing is to solve _that_ problem first.  If using
>> simple panel you could do a change like
>> <https://chromium-review.googlesource.com/#/c/323211/> (though
>> presumably you'd have to handle people using the same panel in other
>> laptops).  You might also be able to do funny things to fixup the mode
>> like dbehr tried to do in
>> <https://chromium-review.googlesource.com/#/c/270017/>.  By doing this
>> and making sure that
>
> Are we missing something here?

Eh?

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

#1318834

On 26 January 2016 at 17:32, Doug Anderson <dianders@chromium.org> wrote:
> Tomeu,
>
> On Tue, Jan 26, 2016 at 12:28 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>> On 22 January 2016 at 18:07, Doug Anderson <dianders@chromium.org> wrote:
>>> Tomeu,
>>>
>>> On Fri, Jan 22, 2016 at 6:00 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>>>> On 21 January 2016 at 21:11, Doug Anderson <dianders@chromium.org> wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, Jan 21, 2016 at 1:03 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>>>>>> So we have a mechanism for detecting a conflict in the clock
>>>>>> hierarchy, and a mechanism to solve it, but we are missing a way for
>>>>>> userspace to communicate policy regarding which clocks should be given
>>>>>> priority when solving such a conflict?
>>>>>
>>>>> Hrmmm, I guess it could be userspace that makes the decision.  It does
>>>>> seem a little odd to force it to userspace in all cases, though.  For
>>>>> a particular laptop that is designed with a specific panel connected
>>>>> up eDP it seems less than ideal to push this into userspace.  If the
>>>>> kernel could just work in the expected sane way (or at least work that
>>>>> way by default) it would be ideal.
>>>>
>>>> Ah, I was wrongly assuming that the kernel didn't have enough
>>>> information to make an informed decision in this case, sorry.
>>>>
>>>> Guess the per-user rate limits don't help here because the consumer
>>>> with higher priority could work with frequencies other than the ideal.
>>>>
>>>> And we cannot have a consumer listening for PRE_RATE_CHANGE and
>>>> aborting unwanted changes or rerouting the ancestors of the clocks of
>>>> other consumers because that would be a massive violation of
>>>> separation of concerns.
>>>>
>>>> If we were to rearrange the clock topology from within the CCF, then
>>>> consumers need to have a way to communicate to the core that they are
>>>> more important than other consumers. clk_set_important(clk, true)
>>>> could be enough in this case, but would be insufficient in more
>>>> complex cases where more than two clocks could use the same PLL.
>>>
>>> With something like the above I'd still expect some complexity
>>> depending on the probe order.  If a less important device grabs the
>>> clock first, it might be forced to re-think its clocks later.  That
>>> might be disconcerting.
>>
>> How much disconcerting do you think this could be? Hopefully those
>> devices should probe quite close to each other, right?
>
> Probe: probably, though with defers it could be several seconds.
>
> ...but remember that display interfaces tend to be hotplug.  That
> might mean that the HDMI interface won't try to set the clock until
> much, much later.

I'm still having trouble grasping what's the impact to users. Is it
that if HDMI gets the contended clock first and the internal panel
device only probes after a second or so, then the user may notice a
change in frequency in the HDMI screen when the panel lights up?

Though in that particular case I'm not sure if the impact is that big
for the user, wonder if such rearranging of the clock hierarchy can
cause bigger problems in other situations.

The on-demand probing series could help here because a downstream that
cared about these issues could just rearrange the contents of the DT
(maybe with a script as part of the build process) so that the panel
is always probed first, but well, that one collided with an iceberg.

>>> OK, so I was just involved in a change recently that made me realize
>>> that maybe our original problems were tied to the fact that our
>>> builtin panels were trying to specify a clock that was impossible to
>>> achieve with CPLL / GPLL.  It was a known problem that the request
>>> would be denied and the CCF would just pick the closest rate it could.
>>> Probably the right thing is to solve _that_ problem first.  If using
>>> simple panel you could do a change like
>>> <https://chromium-review.googlesource.com/#/c/323211/> (though
>>> presumably you'd have to handle people using the same panel in other
>>> laptops).  You might also be able to do funny things to fixup the mode
>>> like dbehr tried to do in
>>> <https://chromium-review.googlesource.com/#/c/270017/>.  By doing this
>>> and making sure that
>>
>> Are we missing something here?
>
> Eh?

The sentence above seemed to have been cut in the middle and I was
wondering if there's something relevant I'm missing because of it.

Thanks,

Tomeu

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

#1319147

Tomeu,

On Wed, Jan 27, 2016 at 2:20 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
> On 26 January 2016 at 17:32, Doug Anderson <dianders@chromium.org> wrote:
>> Tomeu,
>>
>> On Tue, Jan 26, 2016 at 12:28 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>>> On 22 January 2016 at 18:07, Doug Anderson <dianders@chromium.org> wrote:
>>>> Tomeu,
>>>>
>>>> On Fri, Jan 22, 2016 at 6:00 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>>>>> On 21 January 2016 at 21:11, Doug Anderson <dianders@chromium.org> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, Jan 21, 2016 at 1:03 AM, Tomeu Vizoso <tomeu@tomeuvizoso.net> wrote:
>>>>>>> So we have a mechanism for detecting a conflict in the clock
>>>>>>> hierarchy, and a mechanism to solve it, but we are missing a way for
>>>>>>> userspace to communicate policy regarding which clocks should be given
>>>>>>> priority when solving such a conflict?
>>>>>>
>>>>>> Hrmmm, I guess it could be userspace that makes the decision.  It does
>>>>>> seem a little odd to force it to userspace in all cases, though.  For
>>>>>> a particular laptop that is designed with a specific panel connected
>>>>>> up eDP it seems less than ideal to push this into userspace.  If the
>>>>>> kernel could just work in the expected sane way (or at least work that
>>>>>> way by default) it would be ideal.
>>>>>
>>>>> Ah, I was wrongly assuming that the kernel didn't have enough
>>>>> information to make an informed decision in this case, sorry.
>>>>>
>>>>> Guess the per-user rate limits don't help here because the consumer
>>>>> with higher priority could work with frequencies other than the ideal.
>>>>>
>>>>> And we cannot have a consumer listening for PRE_RATE_CHANGE and
>>>>> aborting unwanted changes or rerouting the ancestors of the clocks of
>>>>> other consumers because that would be a massive violation of
>>>>> separation of concerns.
>>>>>
>>>>> If we were to rearrange the clock topology from within the CCF, then
>>>>> consumers need to have a way to communicate to the core that they are
>>>>> more important than other consumers. clk_set_important(clk, true)
>>>>> could be enough in this case, but would be insufficient in more
>>>>> complex cases where more than two clocks could use the same PLL.
>>>>
>>>> With something like the above I'd still expect some complexity
>>>> depending on the probe order.  If a less important device grabs the
>>>> clock first, it might be forced to re-think its clocks later.  That
>>>> might be disconcerting.
>>>
>>> How much disconcerting do you think this could be? Hopefully those
>>> devices should probe quite close to each other, right?
>>
>> Probe: probably, though with defers it could be several seconds.
>>
>> ...but remember that display interfaces tend to be hotplug.  That
>> might mean that the HDMI interface won't try to set the clock until
>> much, much later.
>
> I'm still having trouble grasping what's the impact to users. Is it
> that if HDMI gets the contended clock first and the internal panel
> device only probes after a second or so, then the user may notice a
> change in frequency in the HDMI screen when the panel lights up?
>
> Though in that particular case I'm not sure if the impact is that big
> for the user, wonder if such rearranging of the clock hierarchy can
> cause bigger problems in other situations.
>
> The on-demand probing series could help here because a downstream that
> cared about these issues could just rearrange the contents of the DT
> (maybe with a script as part of the build process) so that the panel
> is always probed first, but well, that one collided with an iceberg.

Problem I'm imagining is this one:

1. EDP (used for builtin panel) is setup by userspace.  It requests a
pixel clock that's not quite achievable from CPLL / GPLL.  Let's say
it requests 67 MHz.  GPLL can make 66 MHz (594 / 9) and CPLL can make
66.67 (400 / 6).  ...but NPLL is sitting right there, so let's make
things exact and set NPLL to 1.608 GHz (67 MHz * 24).  Now we get
exactly 67 MHz.

2. An hour after bootup, the user plugs in HDMI.  The user wants to
use a 78.8 MHz pixel clock.

Option #1: deny HDMI because CPLL / GPLL are fixed and NPLL is in use
by eDP.  Not great for HDMI support.

Option #2: switch EDP away from NPLL to CPLL.  This presumably will
require disabling / re-enabling eDP and isn't super amazing, but could
be done if need be.  Since user space might care about the eDP refresh
rate (so it can properly schedule video frames) and this will tweak
the refresh rate by a bit, this might be cleanest as a full "unplug"
of eDP and re-plug of eDP.

Option #3: Somehow ahead of time know that on this particular device
that eDP shouldn't use NPLL.

--

The whole situation would presumably be different if eDP wasn't used
for the builtin panel.  Technically eDP can be hotplugged (I think).
If we're on a board where eDP could be hotplugged to different
monitors then all the rules might be different.


>>>> OK, so I was just involved in a change recently that made me realize
>>>> that maybe our original problems were tied to the fact that our
>>>> builtin panels were trying to specify a clock that was impossible to
>>>> achieve with CPLL / GPLL.  It was a known problem that the request
>>>> would be denied and the CCF would just pick the closest rate it could.
>>>> Probably the right thing is to solve _that_ problem first.  If using
>>>> simple panel you could do a change like
>>>> <https://chromium-review.googlesource.com/#/c/323211/> (though
>>>> presumably you'd have to handle people using the same panel in other
>>>> laptops).  You might also be able to do funny things to fixup the mode
>>>> like dbehr tried to do in
>>>> <https://chromium-review.googlesource.com/#/c/270017/>.  By doing this
>>>> and making sure that
>>>
>>> Are we missing something here?
>>
>> Eh?
>
> The sentence above seemed to have been cut in the middle and I was
> wondering if there's something relevant I'm missing because of it.

On the current systems eDP will actually request 72.5 MHz, which is
definitely not achievable from CPLL / GPLL.  You can try redoing the
exercise above where eDP requests 72.5 MHz if you want, but suffice to
say that the eDP refresh rate will change more drastically if it goes
from 72.5 MHz to 66.67 MHz.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web