Hi,
Those patches used to be part of a larger clock fixes series:
https://lore.kernel.org/linux-clk/[email protected]/
However, that series doesn't seem to be getting anywhere, so I've split out
these patches that fix a regression that has been there since 5.18 and that
prevents the 4k output from working on the RaspberryPi4.
Hopefully, we will be able to merge those patches through the DRM tree to avoid
any further disruption.
Let me know what you think,
Maxime
To: Florian Fainelli <[email protected]>
To: Broadcom internal kernel review list <[email protected]>
To: Ray Jui <[email protected]>
To: Scott Branden <[email protected]>
To: Michael Turquette <[email protected]>
To: Stephen Boyd <[email protected]>
To: Emma Anholt <[email protected]>
To: Maxime Ripard <[email protected]>
To: David Airlie <[email protected]>
To: Daniel Vetter <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
Changes in v2:
- Dropped the clock patches, made an ad-hoc function in the firmware driver
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Dom Cobley (1):
drm/vc4: hdmi: Add more checks for 4k resolutions
Maxime Ripard (6):
firmware: raspberrypi: Introduce rpi_firmware_find_node()
firmware: raspberrypi: Move the clock IDs to the firmware header
firmware: raspberrypi: Provide a helper to query a clock max rate
drm/vc4: hdmi: Fix hdmi_enable_4kp60 detection
drm/vc4: hdmi: Rework hdmi_enable_4kp60 detection code
drm/vc4: Make sure we don't end up with a core clock too high
drivers/clk/bcm/clk-raspberrypi.c | 18 -----------
drivers/firmware/raspberrypi.c | 22 +++++++++++++
drivers/gpu/drm/vc4/vc4_drv.h | 16 ++++++++++
drivers/gpu/drm/vc4/vc4_hdmi.c | 25 ++++++++-------
drivers/gpu/drm/vc4/vc4_hdmi.h | 8 -----
drivers/gpu/drm/vc4/vc4_hvs.c | 26 +++++++++++++++
drivers/gpu/drm/vc4/vc4_kms.c | 13 +++++---
include/soc/bcm2835/raspberrypi-firmware.h | 51 ++++++++++++++++++++++++++++++
8 files changed, 136 insertions(+), 43 deletions(-)
---
base-commit: 521a547ced6477c54b4b0cc206000406c221b4d6
change-id: 20220815-rpi-fix-4k-60-17273650429d
Best regards,
--
Maxime Ripard <[email protected]>
In order to support higher HDMI frequencies, users have to set the
hdmi_enable_4kp60 parameter in their config.txt file.
This will have the side-effect of raising the maximum of the core clock,
tied to the HVS, and managed by the HVS driver.
However, we are querying this in the HDMI driver by poking into the HVS
structure to get our struct clk handle.
Let's make this part of the HVS bind implementation to have all the core
clock related setup in the same place.
Signed-off-by: Maxime Ripard <[email protected]>
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 1beb96b77b8c..09cdbdb7fff0 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -328,6 +328,8 @@ struct vc4_hvs {
struct clk *core_clk;
+ unsigned long max_core_rate;
+
/* Memory manager for CRTCs to allocate space in the display
* list. Units are dwords.
*/
@@ -339,6 +341,14 @@ struct vc4_hvs {
struct drm_mm_node mitchell_netravali_filter;
struct debugfs_regset32 regset;
+
+ /*
+ * Even if HDMI0 on the RPi4 can output modes requiring a pixel
+ * rate higher than 297MHz, it needs some adjustments in the
+ * config.txt file to be able to do so and thus won't always be
+ * available.
+ */
+ bool vc5_hdmi_enable_scrambling;
};
struct vc4_plane {
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 3b75ac6fa0db..f367f93ca832 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -46,7 +46,6 @@
#include <linux/pm_runtime.h>
#include <linux/rational.h>
#include <linux/reset.h>
-#include <soc/bcm2835/raspberrypi-clocks.h>
#include <sound/dmaengine_pcm.h>
#include <sound/hdmi-codec.h>
#include <sound/pcm_drm_eld.h>
@@ -277,6 +276,7 @@ static void vc4_hdmi_connector_destroy(struct drm_connector *connector)
static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
+ struct vc4_dev *vc4 = to_vc4_dev(connector->dev);
int ret = 0;
struct edid *edid;
@@ -293,7 +293,7 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
- if (vc4_hdmi->disable_4kp60) {
+ if (!vc4->hvs->vc5_hdmi_enable_scrambling) {
struct drm_device *drm = connector->dev;
struct drm_display_mode *mode;
@@ -1480,11 +1480,12 @@ vc4_hdmi_encoder_clock_valid(const struct vc4_hdmi *vc4_hdmi,
{
const struct drm_connector *connector = &vc4_hdmi->connector;
const struct drm_display_info *info = &connector->display_info;
+ struct vc4_dev *vc4 = to_vc4_dev(connector->dev);
if (clock > vc4_hdmi->variant->max_pixel_clock)
return MODE_CLOCK_HIGH;
- if (vc4_hdmi->disable_4kp60 && clock > HDMI_14_MAX_TMDS_CLK)
+ if (!vc4->hvs->vc5_hdmi_enable_scrambling && clock > HDMI_14_MAX_TMDS_CLK)
return MODE_CLOCK_HIGH;
if (info->max_tmds_clock && clock > (info->max_tmds_clock * 1000))
@@ -2965,14 +2966,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
vc4_hdmi->disable_wifi_frequencies =
of_property_read_bool(dev->of_node, "wifi-2.4ghz-coexistence");
- if (variant->max_pixel_clock == 600000000) {
- struct vc4_dev *vc4 = to_vc4_dev(drm);
- unsigned long max_rate = rpi_firmware_clk_get_max_rate(vc4->hvs->core_clk);
-
- if (max_rate < 550000000)
- vc4_hdmi->disable_4kp60 = true;
- }
-
pm_runtime_enable(dev);
/*
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index c3ed2b07df23..7506943050cf 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -155,14 +155,6 @@ struct vc4_hdmi {
*/
bool disable_wifi_frequencies;
- /*
- * Even if HDMI0 on the RPi4 can output modes requiring a pixel
- * rate higher than 297MHz, it needs some adjustments in the
- * config.txt file to be able to do so and thus won't always be
- * available.
- */
- bool disable_4kp60;
-
struct cec_adapter *cec_adap;
struct cec_msg cec_rx_msg;
bool cec_tx_ok;
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index fbaa741dda5f..e28a13a75ec2 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -27,6 +27,8 @@
#include <drm/drm_atomic_helper.h>
#include <drm/drm_vblank.h>
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
#include "vc4_drv.h"
#include "vc4_regs.h"
@@ -671,12 +673,33 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
hvs->regset.nregs = ARRAY_SIZE(hvs_regs);
if (vc4->is_vc5) {
+ struct rpi_firmware *firmware;
+ struct device_node *node;
+ unsigned long max_rate;
+
+ node = rpi_firmware_find_node();
+ if (!node)
+ return -EINVAL;
+
+ firmware = rpi_firmware_get(node);
+ of_node_put(node);
+ if (!firmware)
+ return -EPROBE_DEFER;
+
hvs->core_clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(hvs->core_clk)) {
dev_err(&pdev->dev, "Couldn't get core clock\n");
return PTR_ERR(hvs->core_clk);
}
+ max_rate = rpi_firmware_clk_get_max_rate(firmware,
+ RPI_FIRMWARE_CORE_CLK_ID);
+ rpi_firmware_put(firmware);
+ if (max_rate >= 550000000)
+ hvs->vc5_hdmi_enable_scrambling = true;
+
+ hvs->max_core_rate = max_rate;
+
ret = clk_prepare_enable(hvs->core_clk);
if (ret) {
dev_err(&pdev->dev, "Couldn't enable the core clock\n");
--
b4 0.10.0
The firmware allows to query for its clocks the operating range of a
given clock. We'll need this for some drivers (KMS, in particular) to
infer the state of some configuration options, so let's create a
function to do so.
Signed-off-by: Maxime Ripard <[email protected]>
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index b916e1e171f8..c4b9ea70f5a7 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -228,6 +228,21 @@ static void rpi_register_clk_driver(struct device *dev)
-1, NULL, 0);
}
+unsigned int rpi_firmware_clk_get_max_rate(struct rpi_firmware *fw, unsigned int id)
+{
+ struct rpi_firmware_clk_rate_request msg =
+ RPI_FIRMWARE_CLK_RATE_REQUEST(id);
+ int ret;
+
+ ret = rpi_firmware_property(fw, RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
+ &msg, sizeof(msg));
+ if (ret)
+ return 0;
+
+ return le32_to_cpu(msg.rate);
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_clk_get_max_rate);
+
static void rpi_firmware_delete(struct kref *kref)
{
struct rpi_firmware *fw = container_of(kref, struct rpi_firmware,
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index 74c7bcc1ac2a..10248c370229 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -154,12 +154,32 @@ enum rpi_firmware_clk_id {
RPI_FIRMWARE_NUM_CLK_ID,
};
+/**
+ * struct rpi_firmware_clk_rate_request - Firmware Request for a rate
+ * @id: ID of the clock being queried
+ * @rate: Rate in Hertz. Set by the firmware.
+ *
+ * Used by @RPI_FIRMWARE_GET_CLOCK_RATE, @RPI_FIRMWARE_GET_CLOCK_MEASURED,
+ * @RPI_FIRMWARE_GET_MAX_CLOCK_RATE and @RPI_FIRMWARE_GET_MIN_CLOCK_RATE.
+ */
+struct rpi_firmware_clk_rate_request {
+ __le32 id;
+ __le32 rate;
+} __packed;
+
+#define RPI_FIRMWARE_CLK_RATE_REQUEST(_id) \
+ { \
+ .id = _id, \
+ }
+
#if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
int rpi_firmware_property(struct rpi_firmware *fw,
u32 tag, void *data, size_t len);
int rpi_firmware_property_list(struct rpi_firmware *fw,
void *data, size_t tag_size);
void rpi_firmware_put(struct rpi_firmware *fw);
+unsigned int rpi_firmware_clk_get_max_rate(struct rpi_firmware *fw,
+ unsigned int id);
struct device_node *rpi_firmware_find_node(void);
struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
@@ -179,6 +199,12 @@ static inline int rpi_firmware_property_list(struct rpi_firmware *fw,
static inline void rpi_firmware_put(struct rpi_firmware *fw) { }
+static inline unsigned int rpi_firmware_clk_get_max_rate(struct rpi_firmware *fw,
+ unsigned int id)
+{
+ return UINT_MAX;
+}
+
static inline struct device_node *rpi_firmware_find_node(void)
{
return NULL;
--
b4 0.10.0
Quoting Maxime Ripard (2022-09-20 05:50:22)
> The firmware allows to query for its clocks the operating range of a
> given clock. We'll need this for some drivers (KMS, in particular) to
> infer the state of some configuration options, so let's create a
> function to do so.
>
> Signed-off-by: Maxime Ripard <[email protected]>
>
Acked-by: Stephen Boyd <[email protected]>
Hi Florian,
On Tue, Sep 20, 2022 at 02:50:19PM +0200, Maxime Ripard wrote:
> Those patches used to be part of a larger clock fixes series:
> https://lore.kernel.org/linux-clk/[email protected]/
>
> However, that series doesn't seem to be getting anywhere, so I've split out
> these patches that fix a regression that has been there since 5.18 and that
> prevents the 4k output from working on the RaspberryPi4.
>
> Hopefully, we will be able to merge those patches through the DRM tree to avoid
> any further disruption.
Could you review this? Ideally this would be merged through drm-misc due
to the dependencies between the new firmware functions and the DRM
patches.
Thanks!
Maxime
Hi Maxime,
Am 20.09.22 um 14:50 schrieb Maxime Ripard:
> The firmware allows to query for its clocks the operating range of a
> given clock. We'll need this for some drivers (KMS, in particular) to
> infer the state of some configuration options, so let's create a
> function to do so.
>
> Signed-off-by: Maxime Ripard <[email protected]>
>
> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> index b916e1e171f8..c4b9ea70f5a7 100644
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -228,6 +228,21 @@ static void rpi_register_clk_driver(struct device *dev)
> -1, NULL, 0);
> }
>
> +unsigned int rpi_firmware_clk_get_max_rate(struct rpi_firmware *fw, unsigned int id)
> +{
> + struct rpi_firmware_clk_rate_request msg =
> + RPI_FIRMWARE_CLK_RATE_REQUEST(id);
> + int ret;
> +
> + ret = rpi_firmware_property(fw, RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
> + &msg, sizeof(msg));
> + if (ret)
> + return 0;
> +
> + return le32_to_cpu(msg.rate);
> +}
> +EXPORT_SYMBOL_GPL(rpi_firmware_clk_get_max_rate);
> +
> static void rpi_firmware_delete(struct kref *kref)
> {
> struct rpi_firmware *fw = container_of(kref, struct rpi_firmware,
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
> index 74c7bcc1ac2a..10248c370229 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -154,12 +154,32 @@ enum rpi_firmware_clk_id {
> RPI_FIRMWARE_NUM_CLK_ID,
> };
>
> +/**
> + * struct rpi_firmware_clk_rate_request - Firmware Request for a rate
> + * @id: ID of the clock being queried
> + * @rate: Rate in Hertz. Set by the firmware.
> + *
> + * Used by @RPI_FIRMWARE_GET_CLOCK_RATE, @RPI_FIRMWARE_GET_CLOCK_MEASURED,
> + * @RPI_FIRMWARE_GET_MAX_CLOCK_RATE and @RPI_FIRMWARE_GET_MIN_CLOCK_RATE.
> + */
> +struct rpi_firmware_clk_rate_request {
> + __le32 id;
> + __le32 rate;
> +} __packed;
> +
> +#define RPI_FIRMWARE_CLK_RATE_REQUEST(_id) \
> + { \
> + .id = _id, \
> + }
> +
> #if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
> int rpi_firmware_property(struct rpi_firmware *fw,
> u32 tag, void *data, size_t len);
> int rpi_firmware_property_list(struct rpi_firmware *fw,
> void *data, size_t tag_size);
> void rpi_firmware_put(struct rpi_firmware *fw);
> +unsigned int rpi_firmware_clk_get_max_rate(struct rpi_firmware *fw,
> + unsigned int id);
> struct device_node *rpi_firmware_find_node(void);
> struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
> struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
> @@ -179,6 +199,12 @@ static inline int rpi_firmware_property_list(struct rpi_firmware *fw,
>
> static inline void rpi_firmware_put(struct rpi_firmware *fw) { }
>
> +static inline unsigned int rpi_firmware_clk_get_max_rate(struct rpi_firmware *fw,
> + unsigned int id)
> +{
> + return UINT_MAX;
In case the driver is disabled the function return UINT_MAX, but in case
the firmware doesn't support RPI_FIRMWARE_GET_MAX_CLOCK_RATE it returns
0. This looks a little bit inconsistent to me.
> +}
> +
> static inline struct device_node *rpi_firmware_find_node(void)
> {
> return NULL;
>
On 10/10/22 04:44, Maxime Ripard wrote:
> Hi Florian,
>
> On Tue, Sep 20, 2022 at 02:50:19PM +0200, Maxime Ripard wrote:
>> Those patches used to be part of a larger clock fixes series:
>> https://lore.kernel.org/linux-clk/[email protected]/
>>
>> However, that series doesn't seem to be getting anywhere, so I've split out
>> these patches that fix a regression that has been there since 5.18 and that
>> prevents the 4k output from working on the RaspberryPi4.
>>
>> Hopefully, we will be able to merge those patches through the DRM tree to avoid
>> any further disruption.
>
> Could you review this? Ideally this would be merged through drm-misc due
> to the dependencies between the new firmware functions and the DRM
> patches.
I suppose I can review the firmware parts if you would like me to, for
vc4 I am pretty much clueless, and despite efforts from Emma to get the
vc4 driver to be usable on platforms other than Pi, that never happened
unfortunately. It would be better to keep the firmware and vc4 drivers
decoupled, just so "wrong" assumptions are not made, but for all
practical purposes this is the only combination that exists.
--
Florian
Hi Florian,
On Mon, Oct 10, 2022 at 12:07:22PM -0700, Florian Fainelli wrote:
> On 10/10/22 04:44, Maxime Ripard wrote:
> > Hi Florian,
> >
> > On Tue, Sep 20, 2022 at 02:50:19PM +0200, Maxime Ripard wrote:
> > > Those patches used to be part of a larger clock fixes series:
> > > https://lore.kernel.org/linux-clk/[email protected]/
> > >
> > > However, that series doesn't seem to be getting anywhere, so I've split out
> > > these patches that fix a regression that has been there since 5.18 and that
> > > prevents the 4k output from working on the RaspberryPi4.
> > >
> > > Hopefully, we will be able to merge those patches through the DRM tree to avoid
> > > any further disruption.
> >
> > Could you review this? Ideally this would be merged through drm-misc due
> > to the dependencies between the new firmware functions and the DRM
> > patches.
>
> I suppose I can review the firmware parts if you would like me to
I was of course asking for the firmware parts :)
> for vc4 I am pretty much clueless, and despite efforts from Emma to
> get the vc4 driver to be usable on platforms other than Pi, that never
> happened unfortunately.
Stefan had the same concerns, but I don't think that's a big one. If
needs be, we can move the call to the firware into an if statement or
whatever and support a firmware-less device.
> It would be better to keep the firmware and vc4 drivers decoupled,
> just so "wrong" assumptions are not made, but for all practical
> purposes this is the only combination that exists.
I know, and my initial proposal was relying on a generic CCF function to
implement this. Stephen didn't feel like a single user for it was
enough, and there were some technical drawbacks too that might not have
made this solution robust enough. Hence the firmware solution.
Maxime