Re: [RESEND 2/3] drm :fsl-dcu: Cleanup vblank interrupt mask and status setting code

[Stefan Agner <stefan@agner.ch>]

[also added Jianwei Wang, since he is the current maintainer of this
driver]

On 2015-12-01 00:16, Dongsheng Wang wrote:
> From: Jianwei Wang <jianwei.wang.chn@gmail.com>
> 
> Switch update interrupt mask bit with regmap_update_bits, and clear
> interrupt status by writing 1 to relevant bit before setting mask in
> fsl_dcu_drm_irq_init function.
> 
> Signed-off-by: Jianwei Wang <jianwei.wang.chn@gmail.com>
> Signed-off-by: Yi Meng <b56799@freescale.com>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> index 1930234..5c29ff7 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -38,21 +38,17 @@ static const struct regmap_config fsl_dcu_regmap_config = {
>  static int fsl_dcu_drm_irq_init(struct drm_device *dev)
>  {
>  	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
> -	unsigned int value;
>  	int ret;
>  
>  	ret = drm_irq_install(dev, fsl_dev->irq);
>  	if (ret < 0)
>  		dev_err(dev->dev, "failed to install IRQ handler\n");
>  
> -	ret = regmap_write(fsl_dev->regmap, DCU_INT_STATUS, 0);
> +	ret = regmap_write(fsl_dev->regmap, DCU_INT_STATUS, 0xffffffff);
>  	if (ret)
>  		dev_err(dev->dev, "set DCU_INT_STATUS failed\n");
> -	ret = regmap_read(fsl_dev->regmap, DCU_INT_MASK, &value);
> -	if (ret)
> -		dev_err(dev->dev, "read DCU_INT_MASK failed\n");
> -	value &= DCU_INT_MASK_VBLANK;
> -	ret = regmap_write(fsl_dev->regmap, DCU_INT_MASK, value);
> +	ret = regmap_update_bits(fsl_dev->regmap, DCU_INT_MASK,
> +				 DCU_INT_MASK_VBLANK, ~DCU_INT_MASK_VBLANK);

Just realized that patch tries to solve the issue:
https://lkml.org/lkml/2015/11/18/951

However, in my variant, VBLANK is not enabled on init. Afaik,
enable_vblank will be called anyway, hence there is no need to unmask
VBLANK here...

This patchset and my patchset have some overlap, we need to serialize
this patches.

@Jianwei/DRM maintainers, what do you think, how do we proceed on this?

--
Stefan

>  	if (ret)
>  		dev_err(dev->dev, "set DCU_INT_MASK failed\n");
>  	ret = regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
> @@ -143,14 +139,10 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
>  static int fsl_dcu_drm_enable_vblank(struct drm_device *dev, unsigned int pipe)
>  {
>  	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
> -	unsigned int value;
>  	int ret;
>  
> -	ret = regmap_read(fsl_dev->regmap, DCU_INT_MASK, &value);
> -	if (ret)
> -		dev_err(dev->dev, "read DCU_INT_MASK failed\n");
> -	value &= ~DCU_INT_MASK_VBLANK;
> -	ret = regmap_write(fsl_dev->regmap, DCU_INT_MASK, value);
> +	ret = regmap_update_bits(fsl_dev->regmap, DCU_INT_MASK,
> +				 DCU_INT_MASK_VBLANK, ~DCU_INT_MASK_VBLANK);
>  	if (ret)
>  		dev_err(dev->dev, "set DCU_INT_MASK failed\n");
>  	return 0;
> @@ -160,14 +152,10 @@ static void fsl_dcu_drm_disable_vblank(struct
> drm_device *dev,
>  				       unsigned int pipe)
>  {
>  	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
> -	unsigned int value;
>  	int ret;
>  
> -	ret = regmap_read(fsl_dev->regmap, DCU_INT_MASK, &value);
> -	if (ret)
> -		dev_err(dev->dev, "read DCU_INT_MASK failed\n");
> -	value |= DCU_INT_MASK_VBLANK;
> -	ret = regmap_write(fsl_dev->regmap, DCU_INT_MASK, value);
> +	ret = regmap_update_bits(fsl_dev->regmap, DCU_INT_MASK,
> +				 DCU_INT_MASK_VBLANK, DCU_INT_MASK_VBLANK);
>  	if (ret)
>  		dev_err(dev->dev, "set DCU_INT_MASK failed\n");
>  }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel