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); 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"); }
From: Jianwei Wang jianwei.wang.chn@gmail.com
For DCU support atmost 16 layers(on ls1021a) or 64 layers(on vf610), add (total_layer - 1) overlay planes.
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_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c index a8932a8..7ed1a7e 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c @@ -235,7 +235,10 @@ static const u32 fsl_dcu_drm_plane_formats[] = {
struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev) { + struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; struct drm_plane *primary; + struct drm_plane *overlay; + unsigned int total_layer, formats_size, i; int ret;
primary = kzalloc(sizeof(*primary), GFP_KERNEL); @@ -244,11 +247,12 @@ struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev) return NULL; }
+ formats_size = ARRAY_SIZE(fsl_dcu_drm_plane_formats); /* possible_crtc's will be filled in later by crtc_init */ ret = drm_universal_plane_init(dev, primary, 0, &fsl_dcu_drm_plane_funcs, fsl_dcu_drm_plane_formats, - ARRAY_SIZE(fsl_dcu_drm_plane_formats), + formats_size, DRM_PLANE_TYPE_PRIMARY); if (ret) { kfree(primary); @@ -256,5 +260,26 @@ struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev) } drm_plane_helper_add(primary, &fsl_dcu_drm_plane_helper_funcs);
+ total_layer = fsl_dev->soc->total_layer; + for (i = 0; i < total_layer - 1; i++) { + overlay = kzalloc(sizeof(*overlay), GFP_KERNEL); + if (!overlay) { + DRM_DEBUG_KMS("Failed to allocate overlay plane\n"); + goto out; + } + + ret = drm_universal_plane_init(dev, overlay, 1, + &fsl_dcu_drm_plane_funcs, + fsl_dcu_drm_plane_formats, + formats_size, + DRM_PLANE_TYPE_OVERLAY); + if (ret) { + kfree(overlay); + goto out; + } + drm_plane_helper_add(overlay, &fsl_dcu_drm_plane_helper_funcs); + } + +out: return primary; }
Hi Dongsheng,
On 2015-12-01 00:16, Dongsheng Wang wrote:
From: Jianwei Wang jianwei.wang.chn@gmail.com
For DCU support atmost 16 layers(on ls1021a) or 64 layers(on vf610), add (total_layer - 1) overlay planes.
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_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c index a8932a8..7ed1a7e 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c @@ -235,7 +235,10 @@ static const u32 fsl_dcu_drm_plane_formats[] = {
struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev) {
It feels wrong to me to add multi layer support in a function called fsl_dcu_drm_primary_create_plane...
I suggest to either rename the function, or better create a new function for the overlay planes. The only shared variable is formats_size, which is anyway statically determined by the compiler.
-- Stefan
struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; struct drm_plane *primary;
struct drm_plane *overlay;
unsigned int total_layer, formats_size, i; int ret;
primary = kzalloc(sizeof(*primary), GFP_KERNEL);
@@ -244,11 +247,12 @@ struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev) return NULL; }
- formats_size = ARRAY_SIZE(fsl_dcu_drm_plane_formats); /* possible_crtc's will be filled in later by crtc_init */ ret = drm_universal_plane_init(dev, primary, 0, &fsl_dcu_drm_plane_funcs, fsl_dcu_drm_plane_formats,
ARRAY_SIZE(fsl_dcu_drm_plane_formats),
formats_size, DRM_PLANE_TYPE_PRIMARY);@@ -256,5 +260,26 @@ struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev) } drm_plane_helper_add(primary, &fsl_dcu_drm_plane_helper_funcs);
- total_layer = fsl_dev->soc->total_layer;
- for (i = 0; i < total_layer - 1; i++) {
overlay = kzalloc(sizeof(*overlay), GFP_KERNEL);if (!overlay) {DRM_DEBUG_KMS("Failed to allocate overlay plane\n");goto out;}ret = drm_universal_plane_init(dev, overlay, 1,&fsl_dcu_drm_plane_funcs,fsl_dcu_drm_plane_formats,formats_size,DRM_PLANE_TYPE_OVERLAY);if (ret) {kfree(overlay);
You basically allow to initialize only some layers, which is probably fine. But I think we should add at least an error message here...
-- Stefan
goto out;}drm_plane_helper_add(overlay, &fsl_dcu_drm_plane_helper_funcs);- }
+out: return primary; }
Hi Stefan,
Thanks, now I'm working on PSCIv1.0 for u-boot, and PCIe PM patches. Will reply your comments later(Maybe next week).
Regards, -Dongsheng
Hi Dongsheng,
On 2015-12-01 00:16, Dongsheng Wang wrote:
From: Jianwei Wang jianwei.wang.chn@gmail.com
For DCU support atmost 16 layers(on ls1021a) or 64 layers(on vf610), add (total_layer - 1) overlay planes.
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_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c index a8932a8..7ed1a7e 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c @@ -235,7 +235,10 @@ static const u32 fsl_dcu_drm_plane_formats[] = {
struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev) {
It feels wrong to me to add multi layer support in a function called fsl_dcu_drm_primary_create_plane...
I suggest to either rename the function, or better create a new function for the overlay planes. The only shared variable is formats_size, which is anyway statically determined by the compiler.
-- Stefan
struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; struct drm_plane *primary;
struct drm_plane *overlay;
unsigned int total_layer, formats_size, i; int ret;
primary = kzalloc(sizeof(*primary), GFP_KERNEL); @@ -244,11 +247,12
@@ struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev) return NULL; }
- formats_size = ARRAY_SIZE(fsl_dcu_drm_plane_formats); /* possible_crtc's will be filled in later by crtc_init */ ret = drm_universal_plane_init(dev, primary, 0, &fsl_dcu_drm_plane_funcs, fsl_dcu_drm_plane_formats,
ARRAY_SIZE(fsl_dcu_drm_plane_formats),
formats_size, DRM_PLANE_TYPE_PRIMARY);@@ -256,5 +260,26 @@ struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev) } drm_plane_helper_add(primary, &fsl_dcu_drm_plane_helper_funcs);
- total_layer = fsl_dev->soc->total_layer;
- for (i = 0; i < total_layer - 1; i++) {
overlay = kzalloc(sizeof(*overlay), GFP_KERNEL);if (!overlay) {DRM_DEBUG_KMS("Failed to allocate overlay plane\n");goto out;}ret = drm_universal_plane_init(dev, overlay, 1,&fsl_dcu_drm_plane_funcs,fsl_dcu_drm_plane_formats,formats_size,DRM_PLANE_TYPE_OVERLAY);if (ret) {kfree(overlay);You basically allow to initialize only some layers, which is probably fine. But I think we should add at least an error message here...
-- Stefan
goto out;}drm_plane_helper_add(overlay, &fsl_dcu_drm_plane_helper_funcs);- }
+out: return primary; }
[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);@@ -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);}
dri-devel@lists.freedesktop.org
-
Dongsheng Wang -
Stefan Agner -
Wang Dongsheng