Re: [PATCH v5 7/9] drm/vc4: hdmi: Use the connector state pixel rate for the PHY

2020-12-09 Thread Dave Stevenson
Hi Maxime

On Mon, 7 Dec 2020 at 15:57, Maxime Ripard  wrote:
>
> The PHY initialisation parameters are not based on the pixel clock but
> the TMDS clock rate which can be the pixel clock in the standard case,
> but could be adjusted based on some parameters like the bits per color.
>
> Since the TMDS clock rate is stored in our custom connector state
> already, let's reuse it from there instead of computing it again.
>
> Acked-by: Thomas Zimmermann 
> Signed-off-by: Maxime Ripard 

Reviewed-by: Dave Stevenson 

> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c |  2 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.h | 11 +--
>  drivers/gpu/drm/vc4/vc4_hdmi_phy.c |  8 +---
>  3 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 83699105c7a5..5310e06efc82 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -714,7 +714,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct 
> drm_encoder *encoder,
> vc4_hdmi->variant->reset(vc4_hdmi);
>
> if (vc4_hdmi->variant->phy_init)
> -   vc4_hdmi->variant->phy_init(vc4_hdmi, mode);
> +   vc4_hdmi->variant->phy_init(vc4_hdmi, vc4_conn_state);
>
> HDMI_WRITE(HDMI_SCHEDULER_CONTROL,
>HDMI_READ(HDMI_SCHEDULER_CONTROL) |
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> index bca6943de884..60c53d7c9bad 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> @@ -21,10 +21,9 @@ to_vc4_hdmi_encoder(struct drm_encoder *encoder)
> return container_of(encoder, struct vc4_hdmi_encoder, base.base);
>  }
>
> -struct drm_display_mode;
> -
>  struct vc4_hdmi;
>  struct vc4_hdmi_register;
> +struct vc4_hdmi_connector_state;
>
>  enum vc4_hdmi_phy_channel {
> PHY_LANE_0 = 0,
> @@ -80,9 +79,9 @@ struct vc4_hdmi_variant {
> void (*set_timings)(struct vc4_hdmi *vc4_hdmi,
> struct drm_display_mode *mode);
>
> -   /* Callback to initialize the PHY according to the mode */
> +   /* Callback to initialize the PHY according to the connector state */
> void (*phy_init)(struct vc4_hdmi *vc4_hdmi,
> -struct drm_display_mode *mode);
> +struct vc4_hdmi_connector_state *vc4_conn_state);
>
> /* Callback to disable the PHY */
> void (*phy_disable)(struct vc4_hdmi *vc4_hdmi);
> @@ -192,13 +191,13 @@ conn_state_to_vc4_hdmi_conn_state(struct 
> drm_connector_state *conn_state)
>  }
>
>  void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi,
> -  struct drm_display_mode *mode);
> +  struct vc4_hdmi_connector_state *vc4_conn_state);
>  void vc4_hdmi_phy_disable(struct vc4_hdmi *vc4_hdmi);
>  void vc4_hdmi_phy_rng_enable(struct vc4_hdmi *vc4_hdmi);
>  void vc4_hdmi_phy_rng_disable(struct vc4_hdmi *vc4_hdmi);
>
>  void vc5_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi,
> -  struct drm_display_mode *mode);
> +  struct vc4_hdmi_connector_state *vc4_conn_state);
>  void vc5_hdmi_phy_disable(struct vc4_hdmi *vc4_hdmi);
>  void vc5_hdmi_phy_rng_enable(struct vc4_hdmi *vc4_hdmi);
>  void vc5_hdmi_phy_rng_disable(struct vc4_hdmi *vc4_hdmi);
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_phy.c 
> b/drivers/gpu/drm/vc4/vc4_hdmi_phy.c
> index 057796b54c51..36535480f8e2 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi_phy.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi_phy.c
> @@ -127,7 +127,8 @@
>
>  #define OSCILLATOR_FREQUENCY   5400
>
> -void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, struct drm_display_mode 
> *mode)
> +void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi,
> +  struct vc4_hdmi_connector_state *conn_state)
>  {
> /* PHY should be in reset, like
>  * vc4_hdmi_encoder_disable() does.
> @@ -339,11 +340,12 @@ static void vc5_hdmi_reset_phy(struct vc4_hdmi 
> *vc4_hdmi)
> HDMI_WRITE(HDMI_TX_PHY_POWERDOWN_CTL, BIT(10));
>  }
>
> -void vc5_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, struct drm_display_mode 
> *mode)
> +void vc5_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi,
> +  struct vc4_hdmi_connector_state *conn_state)
>  {
> const struct phy_lane_settings *chan0_settings, *chan1_settings, 
> *chan2_settings, *clock_settings;
> const struct vc4_hdmi_variant *variant = vc4_hdmi->variant;
> -   unsigned long long pixel_freq = mode->clock * 1000;
> +   unsigned long long pixel_freq = conn_state->pixel_rate;
> unsigned long long vco_freq;
> unsigned char word_sel;
> u8 vco_sel, vco_div;
> --
> 2.28.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 7/9] drm/vc4: hdmi: Use the connector state pixel rate for the PHY

2020-12-08 Thread Maxime Ripard
The PHY initialisation parameters are not based on the pixel clock but
the TMDS clock rate which can be the pixel clock in the standard case,
but could be adjusted based on some parameters like the bits per color.

Since the TMDS clock rate is stored in our custom connector state
already, let's reuse it from there instead of computing it again.

Acked-by: Thomas Zimmermann 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c |  2 +-
 drivers/gpu/drm/vc4/vc4_hdmi.h | 11 +--
 drivers/gpu/drm/vc4/vc4_hdmi_phy.c |  8 +---
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 83699105c7a5..5310e06efc82 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -714,7 +714,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct 
drm_encoder *encoder,
vc4_hdmi->variant->reset(vc4_hdmi);
 
if (vc4_hdmi->variant->phy_init)
-   vc4_hdmi->variant->phy_init(vc4_hdmi, mode);
+   vc4_hdmi->variant->phy_init(vc4_hdmi, vc4_conn_state);
 
HDMI_WRITE(HDMI_SCHEDULER_CONTROL,
   HDMI_READ(HDMI_SCHEDULER_CONTROL) |
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index bca6943de884..60c53d7c9bad 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -21,10 +21,9 @@ to_vc4_hdmi_encoder(struct drm_encoder *encoder)
return container_of(encoder, struct vc4_hdmi_encoder, base.base);
 }
 
-struct drm_display_mode;
-
 struct vc4_hdmi;
 struct vc4_hdmi_register;
+struct vc4_hdmi_connector_state;
 
 enum vc4_hdmi_phy_channel {
PHY_LANE_0 = 0,
@@ -80,9 +79,9 @@ struct vc4_hdmi_variant {
void (*set_timings)(struct vc4_hdmi *vc4_hdmi,
struct drm_display_mode *mode);
 
-   /* Callback to initialize the PHY according to the mode */
+   /* Callback to initialize the PHY according to the connector state */
void (*phy_init)(struct vc4_hdmi *vc4_hdmi,
-struct drm_display_mode *mode);
+struct vc4_hdmi_connector_state *vc4_conn_state);
 
/* Callback to disable the PHY */
void (*phy_disable)(struct vc4_hdmi *vc4_hdmi);
@@ -192,13 +191,13 @@ conn_state_to_vc4_hdmi_conn_state(struct 
drm_connector_state *conn_state)
 }
 
 void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi,
-  struct drm_display_mode *mode);
+  struct vc4_hdmi_connector_state *vc4_conn_state);
 void vc4_hdmi_phy_disable(struct vc4_hdmi *vc4_hdmi);
 void vc4_hdmi_phy_rng_enable(struct vc4_hdmi *vc4_hdmi);
 void vc4_hdmi_phy_rng_disable(struct vc4_hdmi *vc4_hdmi);
 
 void vc5_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi,
-  struct drm_display_mode *mode);
+  struct vc4_hdmi_connector_state *vc4_conn_state);
 void vc5_hdmi_phy_disable(struct vc4_hdmi *vc4_hdmi);
 void vc5_hdmi_phy_rng_enable(struct vc4_hdmi *vc4_hdmi);
 void vc5_hdmi_phy_rng_disable(struct vc4_hdmi *vc4_hdmi);
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_phy.c 
b/drivers/gpu/drm/vc4/vc4_hdmi_phy.c
index 057796b54c51..36535480f8e2 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi_phy.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi_phy.c
@@ -127,7 +127,8 @@
 
 #define OSCILLATOR_FREQUENCY   5400
 
-void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, struct drm_display_mode 
*mode)
+void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi,
+  struct vc4_hdmi_connector_state *conn_state)
 {
/* PHY should be in reset, like
 * vc4_hdmi_encoder_disable() does.
@@ -339,11 +340,12 @@ static void vc5_hdmi_reset_phy(struct vc4_hdmi *vc4_hdmi)
HDMI_WRITE(HDMI_TX_PHY_POWERDOWN_CTL, BIT(10));
 }
 
-void vc5_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, struct drm_display_mode 
*mode)
+void vc5_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi,
+  struct vc4_hdmi_connector_state *conn_state)
 {
const struct phy_lane_settings *chan0_settings, *chan1_settings, 
*chan2_settings, *clock_settings;
const struct vc4_hdmi_variant *variant = vc4_hdmi->variant;
-   unsigned long long pixel_freq = mode->clock * 1000;
+   unsigned long long pixel_freq = conn_state->pixel_rate;
unsigned long long vco_freq;
unsigned char word_sel;
u8 vco_sel, vco_div;
-- 
2.28.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel