Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Hi Stefan, On Thu, Sep 15, 2022 at 01:30:02PM +0200, Stefan Wahren wrote: > Am 15.09.22 um 09:54 schrieb Maxime Ripard: > > On Wed, Sep 14, 2022 at 08:26:55PM +0200, Stefan Wahren wrote: > > > Am 14.09.22 um 20:14 schrieb Stephen Boyd: > > > > Quoting Stefan Wahren (2022-09-14 11:09:04) > > > > > Am 14.09.22 um 20:05 schrieb Stephen Boyd: > > > > > > Quoting Stefan Wahren (2022-09-14 10:45:48) > > > > > > > Am 14.09.22 um 17:50 schrieb Stephen Boyd: > > > > > > > > Furthermore, I wonder if even that part needs to be > > > > > > > > implemented. Why > > > > > > > > not make a direct call to rpi_firmware_property() and get the > > > > > > > > max rate? > > > > > > > > All of that can live in the drm driver. Making it a generic API > > > > > > > > that > > > > > > > > takes a 'struct clk' means that it looks like any clk can be > > > > > > > > passed, > > > > > > > > when that isn't true. It would be better to restrict it to the > > > > > > > > one use > > > > > > > > case so that the scope of the problem doesn't grow. I > > > > > > > > understand that it > > > > > > > > duplicates a few lines of code, but that looks like a fair > > > > > > > > tradeoff vs. > > > > > > > > exposing an API that can be used for other clks in the future. > > > > > > > it would be nice to keep all the Rpi specific stuff out of the DRM > > > > > > > driver, since there more users of it. > > > > > > Instead of 'all' did you mean 'any'? > > > > > yes > > > > Why? > > > This firmware is written specific for the Raspberry Pi and not stable from > > > interface point of view. So i'm afraid that the DRM driver is only usable > > > for the Raspberry Pi at the end with all these board specific > > > dependencies. > > I'm open for suggestions there, but is there any other bcm2711 device > > that we support upstream? > > I meant the driver as a whole. According to the vc4 binding there are three > compatibles bcm2835-vc4, cygnus-vc4 and bcm2711-vc5. Unfortunately i don't > have access to any of these Cygnus boards, so i cannot do any regression > tests or provide more information to your question. I don't have access to these boards either, and none of them are upstream, so I'm not sure what we can do to improve their support by then. > > If not, I'm not sure what the big deal is at this point. Chances are the > > DRM driver won't work as is on a different board. > > > > Plus, such a board wouldn't be using config.txt at all, so this whole > > dance to find what was enabled or not wouldn't be used at all. > > My concern is that we reach some point that we need to say this kernel > version requires this firmware version. In the Raspberry Pi OS world this is > not a problem, but not all distributions has this specific knowledge. The recent mess with the Intel GPU firmware (https://lore.kernel.org/dri-devel/CAPM=9txdca1VnRpp-oNLXpBc2UWq3=ceeim5+gw4n9tarir...@mail.gmail.com/) makes it fairly clear that such a situation should be considered a regression and fixed. So it might be a situation that the downstream tree will end up in, but it's not something we will allow to happen upstream. > > > Emma invested a lot of time to make this open source and now it looks that > > > like that more and more functionality moves back to firmware. > > What functionality has been moved back to firmware? > > This wasn't a offense against your great work. Just a slight warning that > some functions of clock or power management moved back into firmware. We > should watch out, but maybe i emote here. Yeah, I guess we'll want to consider it on a case per case basis but it's not like we merged fkms either :) Maxime signature.asc Description: PGP signature
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Hi Maxime, Am 15.09.22 um 09:54 schrieb Maxime Ripard: On Wed, Sep 14, 2022 at 08:26:55PM +0200, Stefan Wahren wrote: Am 14.09.22 um 20:14 schrieb Stephen Boyd: Quoting Stefan Wahren (2022-09-14 11:09:04) Am 14.09.22 um 20:05 schrieb Stephen Boyd: Quoting Stefan Wahren (2022-09-14 10:45:48) Am 14.09.22 um 17:50 schrieb Stephen Boyd: Furthermore, I wonder if even that part needs to be implemented. Why not make a direct call to rpi_firmware_property() and get the max rate? All of that can live in the drm driver. Making it a generic API that takes a 'struct clk' means that it looks like any clk can be passed, when that isn't true. It would be better to restrict it to the one use case so that the scope of the problem doesn't grow. I understand that it duplicates a few lines of code, but that looks like a fair tradeoff vs. exposing an API that can be used for other clks in the future. it would be nice to keep all the Rpi specific stuff out of the DRM driver, since there more users of it. Instead of 'all' did you mean 'any'? yes Why? This firmware is written specific for the Raspberry Pi and not stable from interface point of view. So i'm afraid that the DRM driver is only usable for the Raspberry Pi at the end with all these board specific dependencies. I'm open for suggestions there, but is there any other bcm2711 device that we support upstream? I meant the driver as a whole. According to the vc4 binding there are three compatibles bcm2835-vc4, cygnus-vc4 and bcm2711-vc5. Unfortunately i don't have access to any of these Cygnus boards, so i cannot do any regression tests or provide more information to your question. If not, I'm not sure what the big deal is at this point. Chances are the DRM driver won't work as is on a different board. Plus, such a board wouldn't be using config.txt at all, so this whole dance to find what was enabled or not wouldn't be used at all. My concern is that we reach some point that we need to say this kernel version requires this firmware version. In the Raspberry Pi OS world this is not a problem, but not all distributions has this specific knowledge. Emma invested a lot of time to make this open source and now it looks that like that more and more functionality moves back to firmware. What functionality has been moved back to firmware? This wasn't a offense against your great work. Just a slight warning that some functions of clock or power management moved back into firmware. We should watch out, but maybe i emote here. Maxime
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Hi, On Wed, Sep 14, 2022 at 11:20:59AM -0700, Stephen Boyd wrote: > Quoting Stefan Wahren (2022-09-14 11:09:04) > > Am 14.09.22 um 20:05 schrieb Stephen Boyd: > > > Quoting Stefan Wahren (2022-09-14 10:45:48) > > >> Am 14.09.22 um 17:50 schrieb Stephen Boyd: > > >>> Furthermore, I wonder if even that part needs to be implemented. Why > > >>> not make a direct call to rpi_firmware_property() and get the max rate? > > >>> All of that can live in the drm driver. Making it a generic API that > > >>> takes a 'struct clk' means that it looks like any clk can be passed, > > >>> when that isn't true. It would be better to restrict it to the one use > > >>> case so that the scope of the problem doesn't grow. I understand that it > > >>> duplicates a few lines of code, but that looks like a fair tradeoff vs. > > >>> exposing an API that can be used for other clks in the future. > > >> it would be nice to keep all the Rpi specific stuff out of the DRM > > >> driver, since there more users of it. > > > Instead of 'all' did you mean 'any'? > > yes > > Another idea is to populate an OPP table in the rpi firmware driver for > this platform device with the adjusted max frequency. That would be an > SoC/firmware agnostic interface that expresses the constraints. I'm > almost certain we talked about this before. Yeah, that rings a bell too. I found the thread: https://lore.kernel.org/linux-clk/20220629092900.inpjgl7st33dwcak@houat/ Admittedly, I don't know the OPP stuff that well, but I was always under the impression that it was to express the operating range of a device. We're not quite in this case here, since we want to get the range of one of the clock that feeds the device but might or might not affect the frequency of the device itself. I'm ok with your proposal to create a custom function in the firmware driver though, and I don't believe it would be an obstacle to any board we might upstream in the future that wouldn't have use the RPi firmware, so I'll work on that. Thanks! Maxime signature.asc Description: PGP signature
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
On Wed, Sep 14, 2022 at 08:26:55PM +0200, Stefan Wahren wrote: > Am 14.09.22 um 20:14 schrieb Stephen Boyd: > > Quoting Stefan Wahren (2022-09-14 11:09:04) > > > Am 14.09.22 um 20:05 schrieb Stephen Boyd: > > > > Quoting Stefan Wahren (2022-09-14 10:45:48) > > > > > Am 14.09.22 um 17:50 schrieb Stephen Boyd: > > > > > > Furthermore, I wonder if even that part needs to be implemented. > > > > > > Why > > > > > > not make a direct call to rpi_firmware_property() and get the max > > > > > > rate? > > > > > > All of that can live in the drm driver. Making it a generic API that > > > > > > takes a 'struct clk' means that it looks like any clk can be passed, > > > > > > when that isn't true. It would be better to restrict it to the one > > > > > > use > > > > > > case so that the scope of the problem doesn't grow. I understand > > > > > > that it > > > > > > duplicates a few lines of code, but that looks like a fair tradeoff > > > > > > vs. > > > > > > exposing an API that can be used for other clks in the future. > > > > > it would be nice to keep all the Rpi specific stuff out of the DRM > > > > > driver, since there more users of it. > > > > Instead of 'all' did you mean 'any'? > > > yes > > Why? > This firmware is written specific for the Raspberry Pi and not stable from > interface point of view. So i'm afraid that the DRM driver is only usable > for the Raspberry Pi at the end with all these board specific dependencies. I'm open for suggestions there, but is there any other bcm2711 device that we support upstream? If not, I'm not sure what the big deal is at this point. Chances are the DRM driver won't work as is on a different board. Plus, such a board wouldn't be using config.txt at all, so this whole dance to find what was enabled or not wouldn't be used at all. > Emma invested a lot of time to make this open source and now it looks that > like that more and more functionality moves back to firmware. What functionality has been moved back to firmware? Maxime signature.asc Description: PGP signature
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Hi Stephen, Am 14.09.22 um 20:20 schrieb Stephen Boyd: Quoting Stefan Wahren (2022-09-14 11:09:04) Am 14.09.22 um 20:05 schrieb Stephen Boyd: Quoting Stefan Wahren (2022-09-14 10:45:48) Am 14.09.22 um 17:50 schrieb Stephen Boyd: Furthermore, I wonder if even that part needs to be implemented. Why not make a direct call to rpi_firmware_property() and get the max rate? All of that can live in the drm driver. Making it a generic API that takes a 'struct clk' means that it looks like any clk can be passed, when that isn't true. It would be better to restrict it to the one use case so that the scope of the problem doesn't grow. I understand that it duplicates a few lines of code, but that looks like a fair tradeoff vs. exposing an API that can be used for other clks in the future. it would be nice to keep all the Rpi specific stuff out of the DRM driver, since there more users of it. Instead of 'all' did you mean 'any'? yes Another idea is to populate an OPP table in the rpi firmware driver for this platform device with the adjusted max frequency. That would be an SoC/firmware agnostic interface that expresses the constraints. Do you mean in the source code of this driver or in the DT? I'm almost certain we talked about this before. I'm not sure about the context. Do you mean the CPU frequency handling? I remember it was a hard decision. In the end it was little benefit but a lot of disadvantages (hard to maintain all theses OPP tables for all Raspberry Pi boards, doesn't work with already deployed DT). So yes, i'm part of the problem i mentioned before ;-)
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Am 14.09.22 um 20:14 schrieb Stephen Boyd: Quoting Stefan Wahren (2022-09-14 11:09:04) Am 14.09.22 um 20:05 schrieb Stephen Boyd: Quoting Stefan Wahren (2022-09-14 10:45:48) Am 14.09.22 um 17:50 schrieb Stephen Boyd: Furthermore, I wonder if even that part needs to be implemented. Why not make a direct call to rpi_firmware_property() and get the max rate? All of that can live in the drm driver. Making it a generic API that takes a 'struct clk' means that it looks like any clk can be passed, when that isn't true. It would be better to restrict it to the one use case so that the scope of the problem doesn't grow. I understand that it duplicates a few lines of code, but that looks like a fair tradeoff vs. exposing an API that can be used for other clks in the future. it would be nice to keep all the Rpi specific stuff out of the DRM driver, since there more users of it. Instead of 'all' did you mean 'any'? yes Why? This firmware is written specific for the Raspberry Pi and not stable from interface point of view. So i'm afraid that the DRM driver is only usable for the Raspberry Pi at the end with all these board specific dependencies. Emma invested a lot of time to make this open source and now it looks that like that more and more functionality moves back to firmware.
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Quoting Stefan Wahren (2022-09-14 11:09:04) > Am 14.09.22 um 20:05 schrieb Stephen Boyd: > > Quoting Stefan Wahren (2022-09-14 10:45:48) > >> Am 14.09.22 um 17:50 schrieb Stephen Boyd: > >>> Furthermore, I wonder if even that part needs to be implemented. Why > >>> not make a direct call to rpi_firmware_property() and get the max rate? > >>> All of that can live in the drm driver. Making it a generic API that > >>> takes a 'struct clk' means that it looks like any clk can be passed, > >>> when that isn't true. It would be better to restrict it to the one use > >>> case so that the scope of the problem doesn't grow. I understand that it > >>> duplicates a few lines of code, but that looks like a fair tradeoff vs. > >>> exposing an API that can be used for other clks in the future. > >> it would be nice to keep all the Rpi specific stuff out of the DRM > >> driver, since there more users of it. > > Instead of 'all' did you mean 'any'? > yes Another idea is to populate an OPP table in the rpi firmware driver for this platform device with the adjusted max frequency. That would be an SoC/firmware agnostic interface that expresses the constraints. I'm almost certain we talked about this before.
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Quoting Stefan Wahren (2022-09-14 11:09:04) > Am 14.09.22 um 20:05 schrieb Stephen Boyd: > > Quoting Stefan Wahren (2022-09-14 10:45:48) > >> Am 14.09.22 um 17:50 schrieb Stephen Boyd: > >>> Furthermore, I wonder if even that part needs to be implemented. Why > >>> not make a direct call to rpi_firmware_property() and get the max rate? > >>> All of that can live in the drm driver. Making it a generic API that > >>> takes a 'struct clk' means that it looks like any clk can be passed, > >>> when that isn't true. It would be better to restrict it to the one use > >>> case so that the scope of the problem doesn't grow. I understand that it > >>> duplicates a few lines of code, but that looks like a fair tradeoff vs. > >>> exposing an API that can be used for other clks in the future. > >> it would be nice to keep all the Rpi specific stuff out of the DRM > >> driver, since there more users of it. > > Instead of 'all' did you mean 'any'? > yes Why?
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Am 14.09.22 um 20:05 schrieb Stephen Boyd: Quoting Stefan Wahren (2022-09-14 10:45:48) Am 14.09.22 um 17:50 schrieb Stephen Boyd: Furthermore, I wonder if even that part needs to be implemented. Why not make a direct call to rpi_firmware_property() and get the max rate? All of that can live in the drm driver. Making it a generic API that takes a 'struct clk' means that it looks like any clk can be passed, when that isn't true. It would be better to restrict it to the one use case so that the scope of the problem doesn't grow. I understand that it duplicates a few lines of code, but that looks like a fair tradeoff vs. exposing an API that can be used for other clks in the future. it would be nice to keep all the Rpi specific stuff out of the DRM driver, since there more users of it. Instead of 'all' did you mean 'any'? yes
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Quoting Maxime Ripard (2022-09-14 09:15:02)
> Hi Stephen,
>
> Thanks for reviewing that series
>
> On Wed, Sep 14, 2022 at 08:50:33AM -0700, Stephen Boyd wrote:
> > Quoting Maxime Ripard (2022-08-15 08:31:24)
> > > @@ -254,6 +255,33 @@ static int raspberrypi_fw_dumb_determine_rate(struct
> > > clk_hw *hw,
> > > return 0;
> > > }
> > >
> > > +unsigned long rpi_firmware_clk_get_max_rate(struct clk *clk)
> > > +{
> > > + const struct raspberrypi_clk_data *data;
> > > + struct raspberrypi_clk *rpi;
> > > + struct clk_hw *hw;
> > > + u32 max_rate;
> > > + int ret;
> > > +
> > > + if (!clk)
> > > + return 0;
> > > +
> > > + hw = __clk_get_hw(clk);
> >
> > Ideally we don't add more users of this API. I should document that :/
>
> What should be the proper way to implement this?
>
> > It begs the question though, why do we need this API to take a 'struct
> > clk'? Can it simply hardcode the data->id value for the clk you care
> > about and call rpi_firmware_property() directly (or some wrapper of it)?
>
> You mean push it down to the consumer?
>
> We will have two users of that function eventually. The KMS driver, and
> the codec driver that isn't upstream yet. AFAIK, both are using a
> different clock, so we can' really hardcode it, and duplicating it at
> the consumer level would be weird.
Can you make an API that returns 'max freq for KMS' and 'max freq for
codec'? For example, it could take the enum value that the clk driver
uses for data->id?
>
> > Furthermore, I wonder if even that part needs to be implemented. Why
> > not make a direct call to rpi_firmware_property() and get the max rate?
> > All of that can live in the drm driver. Making it a generic API that
> > takes a 'struct clk' means that it looks like any clk can be passed,
> > when that isn't true. It would be better to restrict it to the one use
> > case so that the scope of the problem doesn't grow. I understand that it
> > duplicates a few lines of code, but that looks like a fair tradeoff vs.
> > exposing an API that can be used for other clks in the future.
>
> So we'll want to have that function shared between the KMS and codec
> drivers eventually. The clock id used by both drivers is stored in the
> DT so we would create a function (outside of the clock drivers) that
> would parse the clocks property, get the ID, and then queries the
> firmware for it. Would that make sense?
>
Sure. Is the ID ever changing? If not then a simpler design would be to
ask for the particular ID and hardcode that in the driver.
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Quoting Stefan Wahren (2022-09-14 10:45:48) > Am 14.09.22 um 17:50 schrieb Stephen Boyd: > > > > Furthermore, I wonder if even that part needs to be implemented. Why > > not make a direct call to rpi_firmware_property() and get the max rate? > > All of that can live in the drm driver. Making it a generic API that > > takes a 'struct clk' means that it looks like any clk can be passed, > > when that isn't true. It would be better to restrict it to the one use > > case so that the scope of the problem doesn't grow. I understand that it > > duplicates a few lines of code, but that looks like a fair tradeoff vs. > > exposing an API that can be used for other clks in the future. > it would be nice to keep all the Rpi specific stuff out of the DRM > driver, since there more users of it. Instead of 'all' did you mean 'any'?
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Hi,
Am 14.09.22 um 17:50 schrieb Stephen Boyd:
Quoting Maxime Ripard (2022-08-15 08:31:24)
@@ -254,6 +255,33 @@ static int raspberrypi_fw_dumb_determine_rate(struct
clk_hw *hw,
return 0;
}
+unsigned long rpi_firmware_clk_get_max_rate(struct clk *clk)
+{
+ const struct raspberrypi_clk_data *data;
+ struct raspberrypi_clk *rpi;
+ struct clk_hw *hw;
+ u32 max_rate;
+ int ret;
+
+ if (!clk)
+ return 0;
+
+ hw = __clk_get_hw(clk);
Ideally we don't add more users of this API. I should document that :/
It begs the question though, why do we need this API to take a 'struct
clk'? Can it simply hardcode the data->id value for the clk you care
about and call rpi_firmware_property() directly (or some wrapper of it)?
Furthermore, I wonder if even that part needs to be implemented. Why
not make a direct call to rpi_firmware_property() and get the max rate?
All of that can live in the drm driver. Making it a generic API that
takes a 'struct clk' means that it looks like any clk can be passed,
when that isn't true. It would be better to restrict it to the one use
case so that the scope of the problem doesn't grow. I understand that it
duplicates a few lines of code, but that looks like a fair tradeoff vs.
exposing an API that can be used for other clks in the future.
it would be nice to keep all the Rpi specific stuff out of the DRM
driver, since there more users of it.
+ if (!hw)
+ return 0;
+
+ data = clk_hw_to_data(hw);
+ rpi = data->rpi;
+ ret = raspberrypi_clock_property(rpi->firmware, data,
+RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
+&max_rate);
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Hi Stephen,
Thanks for reviewing that series
On Wed, Sep 14, 2022 at 08:50:33AM -0700, Stephen Boyd wrote:
> Quoting Maxime Ripard (2022-08-15 08:31:24)
> > @@ -254,6 +255,33 @@ static int raspberrypi_fw_dumb_determine_rate(struct
> > clk_hw *hw,
> > return 0;
> > }
> >
> > +unsigned long rpi_firmware_clk_get_max_rate(struct clk *clk)
> > +{
> > + const struct raspberrypi_clk_data *data;
> > + struct raspberrypi_clk *rpi;
> > + struct clk_hw *hw;
> > + u32 max_rate;
> > + int ret;
> > +
> > + if (!clk)
> > + return 0;
> > +
> > + hw = __clk_get_hw(clk);
>
> Ideally we don't add more users of this API. I should document that :/
What should be the proper way to implement this?
> It begs the question though, why do we need this API to take a 'struct
> clk'? Can it simply hardcode the data->id value for the clk you care
> about and call rpi_firmware_property() directly (or some wrapper of it)?
You mean push it down to the consumer?
We will have two users of that function eventually. The KMS driver, and
the codec driver that isn't upstream yet. AFAIK, both are using a
different clock, so we can' really hardcode it, and duplicating it at
the consumer level would be weird.
> Furthermore, I wonder if even that part needs to be implemented. Why
> not make a direct call to rpi_firmware_property() and get the max rate?
> All of that can live in the drm driver. Making it a generic API that
> takes a 'struct clk' means that it looks like any clk can be passed,
> when that isn't true. It would be better to restrict it to the one use
> case so that the scope of the problem doesn't grow. I understand that it
> duplicates a few lines of code, but that looks like a fair tradeoff vs.
> exposing an API that can be used for other clks in the future.
So we'll want to have that function shared between the KMS and codec
drivers eventually. The clock id used by both drivers is stored in the
DT so we would create a function (outside of the clock drivers) that
would parse the clocks property, get the ID, and then queries the
firmware for it. Would that make sense?
Maxime
signature.asc
Description: PGP signature
Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
Quoting Maxime Ripard (2022-08-15 08:31:24)
> @@ -254,6 +255,33 @@ static int raspberrypi_fw_dumb_determine_rate(struct
> clk_hw *hw,
> return 0;
> }
>
> +unsigned long rpi_firmware_clk_get_max_rate(struct clk *clk)
> +{
> + const struct raspberrypi_clk_data *data;
> + struct raspberrypi_clk *rpi;
> + struct clk_hw *hw;
> + u32 max_rate;
> + int ret;
> +
> + if (!clk)
> + return 0;
> +
> + hw = __clk_get_hw(clk);
Ideally we don't add more users of this API. I should document that :/
It begs the question though, why do we need this API to take a 'struct
clk'? Can it simply hardcode the data->id value for the clk you care
about and call rpi_firmware_property() directly (or some wrapper of it)?
Furthermore, I wonder if even that part needs to be implemented. Why
not make a direct call to rpi_firmware_property() and get the max rate?
All of that can live in the drm driver. Making it a generic API that
takes a 'struct clk' means that it looks like any clk can be passed,
when that isn't true. It would be better to restrict it to the one use
case so that the scope of the problem doesn't grow. I understand that it
duplicates a few lines of code, but that looks like a fair tradeoff vs.
exposing an API that can be used for other clks in the future.
> + if (!hw)
> + return 0;
> +
> + data = clk_hw_to_data(hw);
> + rpi = data->rpi;
> + ret = raspberrypi_clock_property(rpi->firmware, data,
> +RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
> +&max_rate);
[PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the maximum
The RaspberryPi firmware can be configured by the end user using the
config.txt file.
Some of these options will affect the kernel capabilities, and we thus
need to be able to detect it to operate reliably.
One of such parameters is the hdmi_enable_4kp60 parameter that will
setup the clocks in a way that is suitable to reach the pixel
frequencies required by the 4k at 60Hz and higher modes.
If the user forgot to enable it, then those modes will simply not work
but are still likely to be picked up by the userspace, which is a poor
user-experience.
The kernel can't access the config.txt file directly, but one of the
effect that parameter has is that the core clock frequency maximum will
be much higher. Thus we can infer whether it was enabled or not by
querying the firmware for that maximum, and if it isn't prevent any of
the modes that wouldn't work.
The HDMI driver is already doing this, but was relying on a behaviour of
clk_round_rate() that got changed recently, and doesn't return the
result we would like anymore.
We also considered introducing a CCF function to access the maximum of a
given struct clk, but that wouldn't work if the clock is further
constrained by another user.
It was thus suggested to create a small, ad-hoc function to query the
RaspberryPi firmware for the maximum rate a given clock has.
Suggested-by: Stephen Boyd
Signed-off-by: Maxime Ripard
diff --git a/drivers/clk/bcm/clk-raspberrypi.c
b/drivers/clk/bcm/clk-raspberrypi.c
index 6c0a0fd6cd79..182e8817eac2 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -16,6 +16,7 @@
#include
#include
+#include
#include
enum rpi_firmware_clk_id {
@@ -254,6 +255,33 @@ static int raspberrypi_fw_dumb_determine_rate(struct
clk_hw *hw,
return 0;
}
+unsigned long rpi_firmware_clk_get_max_rate(struct clk *clk)
+{
+ const struct raspberrypi_clk_data *data;
+ struct raspberrypi_clk *rpi;
+ struct clk_hw *hw;
+ u32 max_rate;
+ int ret;
+
+ if (!clk)
+ return 0;
+
+ hw = __clk_get_hw(clk);
+ if (!hw)
+ return 0;
+
+ data = clk_hw_to_data(hw);
+ rpi = data->rpi;
+ ret = raspberrypi_clock_property(rpi->firmware, data,
+RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
+&max_rate);
+ if (ret)
+ return 0;
+
+ return max_rate;
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_clk_get_max_rate);
+
static const struct clk_ops raspberrypi_firmware_clk_ops = {
.is_prepared= raspberrypi_fw_is_prepared,
.recalc_rate= raspberrypi_fw_get_rate,
diff --git a/include/soc/bcm2835/raspberrypi-clocks.h
b/include/soc/bcm2835/raspberrypi-clocks.h
new file mode 100644
index ..ff0b608b51a8
--- /dev/null
+++ b/include/soc/bcm2835/raspberrypi-clocks.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __SOC_RASPBERRY_CLOCKS_H__
+#define __SOC_RASPBERRY_CLOCKS_H__
+
+#if IS_ENABLED(CONFIG_CLK_RASPBERRYPI)
+unsigned long rpi_firmware_clk_get_max_rate(struct clk *clk);
+#else
+static inline unsigned long rpi_firmware_clk_get_max_rate(struct clk *clk)
+{
+ return ULONG_MAX;
+}
+#endif
+
+#endif /* __SOC_RASPBERRY_CLOCKS_H__ */
--
b4 0.10.0-dev-a76f5
