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

Dave Stevenson
2020-12-09T15:25:00Z

Hi Maxime

On Mon, 7 Dec 2020 at 15:57, Maxime Ripard <maxime@cerno.tech> 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 <tzimmermann@suse.de>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

> ---
> 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 54000000
>
> -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