diff mbox series

[1/1] drm: bridge: ldb: Warn if LDB clock does not match requested link frequency

Message ID 20221208065538.1753666-1-alexander.stein@ew.tq-group.com (mailing list archive)
State New, archived
Headers show
Series [1/1] drm: bridge: ldb: Warn if LDB clock does not match requested link frequency | expand

Commit Message

Alexander Stein Dec. 8, 2022, 6:55 a.m. UTC
The LDB clock needs to be exactly 7-times the pixel clock used by the
display.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
i.MX8MP has a dedicated LDB clock which defines the actual LVDS link frequency.
This has to be (exactly) the 7-time of the pixel clock.
Although the clock min/max range is available, panel-simple does not (yet) use
the range to find a (perfect) frequency which can be used down the chain, which
is also in range.
Depending on the pixel clock the exact multiple might not be configured.
Raise a warning if there is a mismatch, which might cause an invalid display
image.

 drivers/gpu/drm/bridge/fsl-ldb.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

Comments

Alexander Stein Jan. 19, 2023, 7:38 a.m. UTC | #1
Hi everyone,

Am Donnerstag, 8. Dezember 2022, 07:55:38 CET schrieb Alexander Stein:
> The LDB clock needs to be exactly 7-times the pixel clock used by the
> display.

Any feedback on this?

Thanks
Alexander

> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> i.MX8MP has a dedicated LDB clock which defines the actual LVDS link
> frequency. This has to be (exactly) the 7-time of the pixel clock.
> Although the clock min/max range is available, panel-simple does not (yet)
> use the range to find a (perfect) frequency which can be used down the
> chain, which is also in range.
> Depending on the pixel clock the exact multiple might not be configured.
> Raise a warning if there is a mismatch, which might cause an invalid display
> image.
> 
>  drivers/gpu/drm/bridge/fsl-ldb.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c
> b/drivers/gpu/drm/bridge/fsl-ldb.c index f9e0f8d992680..9bcba8fc57e74
> 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -66,6 +66,14 @@ static inline struct fsl_ldb *to_fsl_ldb(struct
> drm_bridge *bridge) return container_of(bridge, struct fsl_ldb, bridge);
>  }
> 
> +static unsigned long fsl_ldb_link_frequency(struct fsl_ldb *fsl_ldb, int
> clock) +{
> +	if (fsl_ldb->lvds_dual_link)
> +		return clock * 3500;
> +	else
> +		return clock * 7000;
> +}
> +
>  static int fsl_ldb_attach(struct drm_bridge *bridge,
>  			  enum drm_bridge_attach_flags flags)
>  {
> @@ -85,6 +93,8 @@ static void fsl_ldb_atomic_enable(struct drm_bridge
> *bridge, const struct drm_display_mode *mode;
>  	struct drm_connector *connector;
>  	struct drm_crtc *crtc;
> +	unsigned long configured_link_freq;
> +	unsigned long requested_link_freq;
>  	bool lvds_format_24bpp;
>  	bool lvds_format_jeida;
>  	u32 reg;
> @@ -128,10 +138,15 @@ static void fsl_ldb_atomic_enable(struct drm_bridge
> *bridge, crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>  	mode = &crtc_state->adjusted_mode;
> 
> -	if (fsl_ldb->lvds_dual_link)
> -		clk_set_rate(fsl_ldb->clk, mode->clock * 3500);
> -	else
> -		clk_set_rate(fsl_ldb->clk, mode->clock * 7000);
> +	requested_link_freq = fsl_ldb_link_frequency(fsl_ldb, mode->clock);
> +	clk_set_rate(fsl_ldb->clk, requested_link_freq);
> +
> +	configured_link_freq = clk_get_rate(fsl_ldb->clk);
> +	if (configured_link_freq != requested_link_freq)
> +		dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does 
not match
> requested LVDS clock: %lu Hz", +			 
configured_link_freq,
> +			 requested_link_freq);
> +
>  	clk_prepare_enable(fsl_ldb->clk);
> 
>  	/* Program LDB_CTRL */
Neil Armstrong Jan. 19, 2023, 7:49 a.m. UTC | #2
On 08/12/2022 07:55, Alexander Stein wrote:
> The LDB clock needs to be exactly 7-times the pixel clock used by the
> display.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> i.MX8MP has a dedicated LDB clock which defines the actual LVDS link frequency.
> This has to be (exactly) the 7-time of the pixel clock.
> Although the clock min/max range is available, panel-simple does not (yet) use
> the range to find a (perfect) frequency which can be used down the chain, which
> is also in range.
> Depending on the pixel clock the exact multiple might not be configured.
> Raise a warning if there is a mismatch, which might cause an invalid display
> image.
> 
>   drivers/gpu/drm/bridge/fsl-ldb.c | 23 +++++++++++++++++++----
>   1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
> index f9e0f8d992680..9bcba8fc57e74 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -66,6 +66,14 @@ static inline struct fsl_ldb *to_fsl_ldb(struct drm_bridge *bridge)
>   	return container_of(bridge, struct fsl_ldb, bridge);
>   }
>   
> +static unsigned long fsl_ldb_link_frequency(struct fsl_ldb *fsl_ldb, int clock)
> +{
> +	if (fsl_ldb->lvds_dual_link)
> +		return clock * 3500;
> +	else
> +		return clock * 7000;
> +}
> +
>   static int fsl_ldb_attach(struct drm_bridge *bridge,
>   			  enum drm_bridge_attach_flags flags)
>   {
> @@ -85,6 +93,8 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
>   	const struct drm_display_mode *mode;
>   	struct drm_connector *connector;
>   	struct drm_crtc *crtc;
> +	unsigned long configured_link_freq;
> +	unsigned long requested_link_freq;
>   	bool lvds_format_24bpp;
>   	bool lvds_format_jeida;
>   	u32 reg;
> @@ -128,10 +138,15 @@ static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
>   	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>   	mode = &crtc_state->adjusted_mode;
>   
> -	if (fsl_ldb->lvds_dual_link)
> -		clk_set_rate(fsl_ldb->clk, mode->clock * 3500);
> -	else
> -		clk_set_rate(fsl_ldb->clk, mode->clock * 7000);
> +	requested_link_freq = fsl_ldb_link_frequency(fsl_ldb, mode->clock);
> +	clk_set_rate(fsl_ldb->clk, requested_link_freq);
> +
> +	configured_link_freq = clk_get_rate(fsl_ldb->clk);
> +	if (configured_link_freq != requested_link_freq)
> +		dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not match requested LVDS clock: %lu Hz",
> +			 configured_link_freq,
> +			 requested_link_freq);
> +
>   	clk_prepare_enable(fsl_ldb->clk);
>   
>   	/* Program LDB_CTRL */

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Neil Armstrong Jan. 19, 2023, 7:53 a.m. UTC | #3
Hi,

On Thu, 08 Dec 2022 07:55:38 +0100, Alexander Stein wrote:
> The LDB clock needs to be exactly 7-times the pixel clock used by the
> display.
> 
> 

Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)

[1/1] drm: bridge: ldb: Warn if LDB clock does not match requested link frequency
      https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bd43a9844bc6f78e00fdc91db47f6969d10c5ac5
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
index f9e0f8d992680..9bcba8fc57e74 100644
--- a/drivers/gpu/drm/bridge/fsl-ldb.c
+++ b/drivers/gpu/drm/bridge/fsl-ldb.c
@@ -66,6 +66,14 @@  static inline struct fsl_ldb *to_fsl_ldb(struct drm_bridge *bridge)
 	return container_of(bridge, struct fsl_ldb, bridge);
 }
 
+static unsigned long fsl_ldb_link_frequency(struct fsl_ldb *fsl_ldb, int clock)
+{
+	if (fsl_ldb->lvds_dual_link)
+		return clock * 3500;
+	else
+		return clock * 7000;
+}
+
 static int fsl_ldb_attach(struct drm_bridge *bridge,
 			  enum drm_bridge_attach_flags flags)
 {
@@ -85,6 +93,8 @@  static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
 	const struct drm_display_mode *mode;
 	struct drm_connector *connector;
 	struct drm_crtc *crtc;
+	unsigned long configured_link_freq;
+	unsigned long requested_link_freq;
 	bool lvds_format_24bpp;
 	bool lvds_format_jeida;
 	u32 reg;
@@ -128,10 +138,15 @@  static void fsl_ldb_atomic_enable(struct drm_bridge *bridge,
 	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 	mode = &crtc_state->adjusted_mode;
 
-	if (fsl_ldb->lvds_dual_link)
-		clk_set_rate(fsl_ldb->clk, mode->clock * 3500);
-	else
-		clk_set_rate(fsl_ldb->clk, mode->clock * 7000);
+	requested_link_freq = fsl_ldb_link_frequency(fsl_ldb, mode->clock);
+	clk_set_rate(fsl_ldb->clk, requested_link_freq);
+
+	configured_link_freq = clk_get_rate(fsl_ldb->clk);
+	if (configured_link_freq != requested_link_freq)
+		dev_warn(fsl_ldb->dev, "Configured LDB clock (%lu Hz) does not match requested LVDS clock: %lu Hz",
+			 configured_link_freq,
+			 requested_link_freq);
+
 	clk_prepare_enable(fsl_ldb->clk);
 
 	/* Program LDB_CTRL */