| Message ID | 20210225155909.1853812-5-maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: hdmi: Support the 4k @ 60Hz modes | expand |
Hi Maxime On Thu, 25 Feb 2021 at 15:59, Maxime Ripard <maxime@cerno.tech> wrote: > > In order to reach the frequencies needed to output at 594MHz, the > firmware needs to be configured with the appropriate parameters in the > config.txt file (enable_hdmi_4kp60 and force_turbo). force_turbo isn't the right way to go about this as it permanently bumps all the clocks up, even if running the display at VGA. > Let's detect it at bind time, warn the user if we can't, and filter out > the relevant modes. > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- > drivers/gpu/drm/vc4/vc4_hdmi.c | 17 +++++++++++++++++ > drivers/gpu/drm/vc4/vc4_hdmi.h | 8 ++++++++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > index b5bc742993a4..f05f6da286f7 100644 > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > @@ -953,6 +953,9 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, > if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) > return -EINVAL; > > + if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK)) > + return -EINVAL; > + > vc4_state->pixel_rate = pixel_rate; > > return 0; > @@ -972,6 +975,9 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder, > if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock) > return MODE_CLOCK_HIGH; > > + if (vc4_hdmi->disable_4kp60 && ((mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK)) > + return MODE_CLOCK_HIGH; > + > return MODE_OK; > } > > @@ -1986,6 +1992,17 @@ 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); > + long max_rate = clk_get_max_rate(vc4->hvs->core_clk); > + > + if (max_rate < 550000000) { > + drm_warn(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz."); > + drm_warn(drm, "Please change your config.txt file to add hdmi_enable_4kp60 and force_turbo"); Do we really want to warn in bind? Again you could have a VGA resolution monitor attached but that would trigger this warning. Can we warn (once) on processing the mode list and filtering out a clk > HDMI_14_MAX_TMDS_CLK mode instead? And mentioning force_turbo is again wrong. Dave > + vc4_hdmi->disable_4kp60 = true; > + } > + } > + > if (vc4_hdmi->variant->reset) > vc4_hdmi->variant->reset(vc4_hdmi); > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h > index 3cebd1fd00fc..3cd021136402 100644 > --- a/drivers/gpu/drm/vc4/vc4_hdmi.h > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h > @@ -154,6 +154,14 @@ 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; > -- > 2.29.2 >
Hi Dave, Thanks for your review On Thu, Feb 25, 2021 at 04:38:37PM +0000, Dave Stevenson wrote: > On Thu, 25 Feb 2021 at 15:59, Maxime Ripard <maxime@cerno.tech> wrote: > > > > In order to reach the frequencies needed to output at 594MHz, the > > firmware needs to be configured with the appropriate parameters in the > > config.txt file (enable_hdmi_4kp60 and force_turbo). > > force_turbo isn't the right way to go about this as it permanently > bumps all the clocks up, even if running the display at VGA. so enable_hdmi_4kp60 is enough? > > Let's detect it at bind time, warn the user if we can't, and filter out > > the relevant modes. > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > --- > > drivers/gpu/drm/vc4/vc4_hdmi.c | 17 +++++++++++++++++ > > drivers/gpu/drm/vc4/vc4_hdmi.h | 8 ++++++++ > > 2 files changed, 25 insertions(+) > > > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > > index b5bc742993a4..f05f6da286f7 100644 > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > > @@ -953,6 +953,9 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, > > if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) > > return -EINVAL; > > > > + if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK)) > > + return -EINVAL; > > + > > vc4_state->pixel_rate = pixel_rate; > > > > return 0; > > @@ -972,6 +975,9 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder, > > if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock) > > return MODE_CLOCK_HIGH; > > > > + if (vc4_hdmi->disable_4kp60 && ((mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK)) > > + return MODE_CLOCK_HIGH; > > + > > return MODE_OK; > > } > > > > @@ -1986,6 +1992,17 @@ 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); > > + long max_rate = clk_get_max_rate(vc4->hvs->core_clk); > > + > > + if (max_rate < 550000000) { > > + drm_warn(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz."); > > + drm_warn(drm, "Please change your config.txt file to add hdmi_enable_4kp60 and force_turbo"); > > Do we really want to warn in bind? Again you could have a VGA > resolution monitor attached but that would trigger this warning. > Can we warn (once) on processing the mode list and filtering out a clk > > HDMI_14_MAX_TMDS_CLK mode instead? That's a good idea indeed, I'll rework the patch to do that Thanks! Maxime
On Tue, 2 Mar 2021 at 13:02, Maxime Ripard <maxime@cerno.tech> wrote: > > Hi Dave, > > Thanks for your review > > On Thu, Feb 25, 2021 at 04:38:37PM +0000, Dave Stevenson wrote: > > On Thu, 25 Feb 2021 at 15:59, Maxime Ripard <maxime@cerno.tech> wrote: > > > > > > In order to reach the frequencies needed to output at 594MHz, the > > > firmware needs to be configured with the appropriate parameters in the > > > config.txt file (enable_hdmi_4kp60 and force_turbo). > > > > force_turbo isn't the right way to go about this as it permanently > > bumps all the clocks up, even if running the display at VGA. > > so enable_hdmi_4kp60 is enough? No, but force_turbo=1 is the wrong way to go about fixing that. I'll start a thread with Dom & Tim to discuss the best way of doing this. Immediate thoughts are that either vc4_hdmi needs to request the core clock be increased, or potentially the firmware could note the boosted BVB rate and increase core accordingly. Dave > > > Let's detect it at bind time, warn the user if we can't, and filter out > > > the relevant modes. > > > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > > --- > > > drivers/gpu/drm/vc4/vc4_hdmi.c | 17 +++++++++++++++++ > > > drivers/gpu/drm/vc4/vc4_hdmi.h | 8 ++++++++ > > > 2 files changed, 25 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > > > index b5bc742993a4..f05f6da286f7 100644 > > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > > > @@ -953,6 +953,9 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, > > > if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) > > > return -EINVAL; > > > > > > + if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK)) > > > + return -EINVAL; > > > + > > > vc4_state->pixel_rate = pixel_rate; > > > > > > return 0; > > > @@ -972,6 +975,9 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder, > > > if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock) > > > return MODE_CLOCK_HIGH; > > > > > > + if (vc4_hdmi->disable_4kp60 && ((mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK)) > > > + return MODE_CLOCK_HIGH; > > > + > > > return MODE_OK; > > > } > > > > > > @@ -1986,6 +1992,17 @@ 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); > > > + long max_rate = clk_get_max_rate(vc4->hvs->core_clk); > > > + > > > + if (max_rate < 550000000) { > > > + drm_warn(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz."); > > > + drm_warn(drm, "Please change your config.txt file to add hdmi_enable_4kp60 and force_turbo"); > > > > Do we really want to warn in bind? Again you could have a VGA > > resolution monitor attached but that would trigger this warning. > > Can we warn (once) on processing the mode list and filtering out a clk > > > HDMI_14_MAX_TMDS_CLK mode instead? > > That's a good idea indeed, I'll rework the patch to do that > > Thanks! > Maxime
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index b5bc742993a4..f05f6da286f7 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -953,6 +953,9 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) return -EINVAL; + if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK)) + return -EINVAL; + vc4_state->pixel_rate = pixel_rate; return 0; @@ -972,6 +975,9 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder, if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock) return MODE_CLOCK_HIGH; + if (vc4_hdmi->disable_4kp60 && ((mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK)) + return MODE_CLOCK_HIGH; + return MODE_OK; } @@ -1986,6 +1992,17 @@ 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); + long max_rate = clk_get_max_rate(vc4->hvs->core_clk); + + if (max_rate < 550000000) { + drm_warn(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz."); + drm_warn(drm, "Please change your config.txt file to add hdmi_enable_4kp60 and force_turbo"); + vc4_hdmi->disable_4kp60 = true; + } + } + if (vc4_hdmi->variant->reset) vc4_hdmi->variant->reset(vc4_hdmi); diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 3cebd1fd00fc..3cd021136402 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -154,6 +154,14 @@ 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;
In order to reach the frequencies needed to output at 594MHz, the firmware needs to be configured with the appropriate parameters in the config.txt file (enable_hdmi_4kp60 and force_turbo). Let's detect it at bind time, warn the user if we can't, and filter out the relevant modes. Signed-off-by: Maxime Ripard <maxime@cerno.tech> --- drivers/gpu/drm/vc4/vc4_hdmi.c | 17 +++++++++++++++++ drivers/gpu/drm/vc4/vc4_hdmi.h | 8 ++++++++ 2 files changed, 25 insertions(+)