diff mbox

[1/3] drm/tilcdc: Take mode config lock while updating the crtc clock rate

Message ID 202b0a53ecee8d3104d25b22758dc0dffb156cef.1473148345.git.jsarha@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jyri Sarha Sept. 6, 2016, 8:19 a.m. UTC
Take mode config lock while updating the crtc clock rate. To avoid a
race in tilcdc_crtc_update_clk(), we do not want the mode to change
while we update crtc clock.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 5 +++++
 drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 ++
 2 files changed, 7 insertions(+)

Comments

Tomi Valkeinen Sept. 6, 2016, 9:07 a.m. UTC | #1
On 06/09/16 11:19, Jyri Sarha wrote:
> Take mode config lock while updating the crtc clock rate. To avoid a
> race in tilcdc_crtc_update_clk(), we do not want the mode to change
> while we update crtc clock.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c | 5 +++++
>  drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index f8892e9..882d9b5 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -184,10 +184,14 @@ static int cpufreq_transition(struct notifier_block *nb,
>  {
>  	struct tilcdc_drm_private *priv = container_of(nb,
>  			struct tilcdc_drm_private, freq_transition);
> +	struct drm_mode_config *config = &priv->dev->mode_config;
> +
>  	if (val == CPUFREQ_POSTCHANGE) {
>  		if (priv->lcd_fck_rate != clk_get_rate(priv->clk)) {
> +			mutex_lock(&config->mutex);

drm_modeset_lock_crtc()? Or drm_modeset_lock_all() if per-crtc is not
suitable.

 Tomi
Jyri Sarha Sept. 6, 2016, 12:07 p.m. UTC | #2
On 09/06/16 12:07, Tomi Valkeinen wrote:
> 
> 
> On 06/09/16 11:19, Jyri Sarha wrote:
>> Take mode config lock while updating the crtc clock rate. To avoid a
>> race in tilcdc_crtc_update_clk(), we do not want the mode to change
>> while we update crtc clock.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  drivers/gpu/drm/tilcdc/tilcdc_drv.c | 5 +++++
>>  drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 ++
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> index f8892e9..882d9b5 100644
>> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
>> @@ -184,10 +184,14 @@ static int cpufreq_transition(struct notifier_block *nb,
>>  {
>>  	struct tilcdc_drm_private *priv = container_of(nb,
>>  			struct tilcdc_drm_private, freq_transition);
>> +	struct drm_mode_config *config = &priv->dev->mode_config;
>> +
>>  	if (val == CPUFREQ_POSTCHANGE) {
>>  		if (priv->lcd_fck_rate != clk_get_rate(priv->clk)) {
>> +			mutex_lock(&config->mutex);
> 
> drm_modeset_lock_crtc()? Or drm_modeset_lock_all() if per-crtc is not
> suitable.
> 

I guess that should work too, all I need is just to make sure no one
calls mode_set_nofb() while the clock is updated.

I just thought that since I am not actually touching drm state at all
the back off mechanisms etc add just unnecessary complexity. But now
after reading a little bit drm locking code, for sure the mode config
mutex is not the light weight lock I was looking for.

One solution would be adding tilcdc internal mutex to synchronize
mode_set_nofb() and cpufreq_transition(), but after all it is probably
better to use the mechanisms that are already there and just use
drm_modeset_lock_crtc() despite its apparent complexity.

BR,
Jyri
diff mbox

Patch

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index f8892e9..882d9b5 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -184,10 +184,14 @@  static int cpufreq_transition(struct notifier_block *nb,
 {
 	struct tilcdc_drm_private *priv = container_of(nb,
 			struct tilcdc_drm_private, freq_transition);
+	struct drm_mode_config *config = &priv->dev->mode_config;
+
 	if (val == CPUFREQ_POSTCHANGE) {
 		if (priv->lcd_fck_rate != clk_get_rate(priv->clk)) {
+			mutex_lock(&config->mutex);
 			priv->lcd_fck_rate = clk_get_rate(priv->clk);
 			tilcdc_crtc_update_clk(priv->crtc);
+			mutex_unlock(&config->mutex);
 		}
 	}
 
@@ -251,6 +255,7 @@  static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 	}
 
 	dev->dev_private = priv;
+	priv->dev = dev;
 
 	priv->is_componentized =
 		tilcdc_get_external_components(dev->dev, NULL) > 0;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index a6e5e6d..6caecfc 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -49,6 +49,8 @@ 
 struct tilcdc_drm_private {
 	void __iomem *mmio;
 
+	struct drm_device *dev;
+
 	struct clk *clk;         /* functional clock */
 	int rev;                 /* IP revision */