Hi Paul.
On Tue, Sep 15, 2020 at 02:38:17PM +0200, Paul Cercueil wrote:
> Old Ingenic SoCs can overclock very well, up to +50% of their nominal
> clock rate, whithout requiring overvolting or anything like that, just
> by changing the rate of the main PLL. Unfortunately, all clocks on the
> system are derived from that PLL, and when the PLL rate is updated, so
> is our pixel clock.
>
> To counter that issue, we make sure that the panel is in VBLANK before
> the rate change happens, and we will then re-set the pixel clock rate
> afterwards, once the PLL has been changed, to be as close as possible to
> the pixel rate requested by the encoder.
>
> Signed-off-by: Paul Cercueil <p...@crapouillou.net>
> ---
> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 49 ++++++++++++++++++++++-
> 1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index fb62869befdc..aa32660033d2 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -12,6 +12,7 @@
> #include <linux/dma-noncoherent.h>
> #include <linux/io.h>
> #include <linux/module.h>
> +#include <linux/mutex.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> @@ -73,6 +74,9 @@ struct ingenic_drm {
>
> bool panel_is_sharp;
> bool no_vblank;
> + bool update_clk_rate;
> + struct mutex clk_mutex;
Please add comment about what the mutex protects.
Especially since the mutex is locked and unlocked based on a
notification.
With the comment added:
Acked-by: Sam Ravnborg <s...@ravnborg.org>
> + struct notifier_block clock_nb;
> };
>
> static bool ingenic_drm_cached_gem_buf;
> @@ -115,6 +119,29 @@ static inline struct ingenic_drm
> *drm_crtc_get_priv(struct drm_crtc *crtc)
> return container_of(crtc, struct ingenic_drm, crtc);
> }
>
> +static inline struct ingenic_drm *drm_nb_get_priv(struct notifier_block *nb)
> +{
> + return container_of(nb, struct ingenic_drm, clock_nb);
> +}
> +
> +static int ingenic_drm_update_pixclk(struct notifier_block *nb,
> + unsigned long action,
> + void *data)
> +{
> + struct ingenic_drm *priv = drm_nb_get_priv(nb);
> +
> + switch (action) {
> + case PRE_RATE_CHANGE:
> + mutex_lock(&priv->clk_mutex);
> + priv->update_clk_rate = true;
> + drm_crtc_wait_one_vblank(&priv->crtc);
> + return NOTIFY_OK;
> + default:
> + mutex_unlock(&priv->clk_mutex);
Any risk the POST_RATE_CHANGE or ABORT_RATE_CHANGE may go missing so we
fail to unlock the mutex?
I think not but wanted to make sure you had thought about it.
> + return NOTIFY_OK;
> + }
> +}
> +
> static void ingenic_drm_crtc_atomic_enable(struct drm_crtc *crtc,
> struct drm_crtc_state *state)
> {
> @@ -280,8 +307,14 @@ static void ingenic_drm_crtc_atomic_flush(struct
> drm_crtc *crtc,
>
> if (drm_atomic_crtc_needs_modeset(state)) {
> ingenic_drm_crtc_update_timings(priv, &state->mode);
> + priv->update_clk_rate = true;
> + }
>
> + if (priv->update_clk_rate) {
> + mutex_lock(&priv->clk_mutex);
> clk_set_rate(priv->pix_clk, state->adjusted_mode.clock * 1000);
> + priv->update_clk_rate = false;
> + mutex_unlock(&priv->clk_mutex);
> }
>
> if (event) {
> @@ -1046,16 +1079,28 @@ static int ingenic_drm_bind(struct device *dev, bool
> has_components)
> if (soc_info->has_osd)
> regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
>
> + mutex_init(&priv->clk_mutex);
> + priv->clock_nb.notifier_call = ingenic_drm_update_pixclk;
> +
> + parent_clk = clk_get_parent(priv->pix_clk);
> + ret = clk_notifier_register(parent_clk, &priv->clock_nb);
> + if (ret) {
> + dev_err(dev, "Unable to register clock notifier\n");
> + goto err_devclk_disable;
> + }
> +
> ret = drm_dev_register(drm, 0);
> if (ret) {
> dev_err(dev, "Failed to register DRM driver\n");
> - goto err_devclk_disable;
> + goto err_clk_notifier_unregister;
> }
>
> drm_fbdev_generic_setup(drm, 32);
>
> return 0;
>
> +err_clk_notifier_unregister:
> + clk_notifier_unregister(parent_clk, &priv->clock_nb);
> err_devclk_disable:
> if (priv->lcd_clk)
> clk_disable_unprepare(priv->lcd_clk);
> @@ -1077,7 +1122,9 @@ static int compare_of(struct device *dev, void *data)
> static void ingenic_drm_unbind(struct device *dev)
> {
> struct ingenic_drm *priv = dev_get_drvdata(dev);
> + struct clk *parent_clk = clk_get_parent(priv->pix_clk);
>
> + clk_notifier_unregister(parent_clk, &priv->clock_nb);
> if (priv->lcd_clk)
> clk_disable_unprepare(priv->lcd_clk);
> clk_disable_unprepare(priv->pix_clk);
> --
> 2.28.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
