[PATCH] drm/tilcdc: Allocate register storage based on the actual number registers

[PATCH] drm/tilcdc: Allocate register storage based on the actual number registers

[Jyri Sarha]

Allocate suspend/resume register storage based on the actual number
registers the driver is aware of. The static allocation for register
storage had falen behind badly.

Reported-by: Michael Bode <michael@bumbleB.de>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 20 +++++++++++++++++++-
 drivers/gpu/drm/tilcdc/tilcdc_drv.h |  2 +-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 4908c1f..2f87263 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -139,11 +139,14 @@ static int tilcdc_unload(struct drm_device *dev)
 
 	pm_runtime_disable(dev->dev);
 
+	kfree(priv->saved_register);
 	kfree(priv);
 
 	return 0;
 }
 
+static size_t tilcdc_num_regs(void);
+
 static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 {
 	struct platform_device *pdev = dev->platformdev;
@@ -155,7 +158,11 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 	int ret;
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
+	if (priv)
+		priv->saved_register = kcalloc(sizeof(*priv->saved_register),
+					       tilcdc_num_regs(), GFP_KERNEL);
+	if (!priv || !priv->saved_register) {
+		kfree(priv);
 		dev_err(dev->dev, "failed to allocate private data\n");
 		return -ENOMEM;
 	}
@@ -345,6 +352,7 @@ fail_free_wq:
 
 fail_free_priv:
 	dev->dev_private = NULL;
+	kfree(priv->saved_register);
 	kfree(priv);
 	return ret;
 }
@@ -467,6 +475,16 @@ static const struct {
 		REG(2, true,  LCDC_INT_ENABLE_SET_REG),
 #undef REG
 };
+
+static size_t tilcdc_num_regs(void)
+{
+	return ARRAY_SIZE(registers);
+}
+#else
+static size_t tilcdc_num_regs(void)
+{
+	return 0;
+}
 #endif
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index e863ad0..bc94835 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -67,7 +67,7 @@ struct tilcdc_drm_private {
 	uint32_t max_width;
 
 	/* register contents saved across suspend/resume: */
-	u32 saved_register[12];
+	u32 *saved_register;
 
 #ifdef CONFIG_CPU_FREQ
 	struct notifier_block freq_transition;
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tilcdc: Allocate register storage based on the actual number registers

[Michael Bode]

[-- Attachment #1.1: Type: text/plain, Size: 2831 bytes --]



Reviewed-By: Michael Bode <michael@bumbleB.de>

Jyri,

good idea to allocate the space for registers dynamically.
This eliminates the need of synchronizing driver and header file in an 
elegant way.

Only thing I personally don't like, is potentially passing a NULL 
pointer to kfree.
But I learned that this is valid kernel code.

Br,
Michael



On 07/03/2015 12:45 PM, Jyri Sarha wrote:
> Allocate suspend/resume register storage based on the actual number
> registers the driver is aware of. The static allocation for register
> storage had falen behind badly.
>
> Reported-by: Michael Bode <michael@bumbleB.de>
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>   drivers/gpu/drm/tilcdc/tilcdc_drv.c | 20 +++++++++++++++++++-
>   drivers/gpu/drm/tilcdc/tilcdc_drv.h |  2 +-
>   2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 4908c1f..2f87263 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -139,11 +139,14 @@ static int tilcdc_unload(struct drm_device *dev)
>   
>   	pm_runtime_disable(dev->dev);
>   
> +	kfree(priv->saved_register);
>   	kfree(priv);
>   
>   	return 0;
>   }
>   
> +static size_t tilcdc_num_regs(void);
> +
>   static int tilcdc_load(struct drm_device *dev, unsigned long flags)
>   {
>   	struct platform_device *pdev = dev->platformdev;
> @@ -155,7 +158,11 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
>   	int ret;
>   
>   	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> -	if (!priv) {
> +	if (priv)
> +		priv->saved_register = kcalloc(sizeof(*priv->saved_register),
> +					       tilcdc_num_regs(), GFP_KERNEL);
> +	if (!priv || !priv->saved_register) {
> +		kfree(priv);
>   		dev_err(dev->dev, "failed to allocate private data\n");
>   		return -ENOMEM;
>   	}
> @@ -345,6 +352,7 @@ fail_free_wq:
>   
>   fail_free_priv:
>   	dev->dev_private = NULL;
> +	kfree(priv->saved_register);
>   	kfree(priv);
>   	return ret;
>   }
> @@ -467,6 +475,16 @@ static const struct {
>   		REG(2, true,  LCDC_INT_ENABLE_SET_REG),
>   #undef REG
>   };
> +
> +static size_t tilcdc_num_regs(void)
> +{
> +	return ARRAY_SIZE(registers);
> +}
> +#else
> +static size_t tilcdc_num_regs(void)
> +{
> +	return 0;
> +}
>   #endif
>   
>   #ifdef CONFIG_DEBUG_FS
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> index e863ad0..bc94835 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> @@ -67,7 +67,7 @@ struct tilcdc_drm_private {
>   	uint32_t max_width;
>   
>   	/* register contents saved across suspend/resume: */
> -	u32 saved_register[12];
> +	u32 *saved_register;
>   
>   #ifdef CONFIG_CPU_FREQ
>   	struct notifier_block freq_transition;


[-- Attachment #1.2: Type: text/html, Size: 3594 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel