2018-11-05 10:46:51

by Maxime Jourdan

[permalink] [raw]
Subject: [PATCH 0/2] drm/meson: Allow using optional canvas provider

The meson DRM driver currently uses constant, static canvas indexes.

This is not optimal and could conflict with other drivers also using
canvases.

This patch series allows the driver to optionnally use a canvas provider
module that is able to dispatch canvases, on demand and free of conflict.

In the future, the current way of doing things will be deprecated.

Maxime Jourdan (2):
dt-bindings: display: amlogic, meson-vpu: Add optional canvas provider
node
drm/meson: Use optional canvas provider

.../bindings/display/amlogic,meson-vpu.txt | 2 +
drivers/gpu/drm/meson/Kconfig | 1 +
drivers/gpu/drm/meson/meson_crtc.c | 14 ++++--
drivers/gpu/drm/meson/meson_drv.c | 46 ++++++++++++-------
drivers/gpu/drm/meson/meson_drv.h | 4 ++
drivers/gpu/drm/meson/meson_plane.c | 8 +++-
6 files changed, 53 insertions(+), 22 deletions(-)

--
2.19.1



2018-11-05 10:46:01

by Maxime Jourdan

[permalink] [raw]
Subject: [PATCH 2/2] drm/meson: Use optional canvas provider

This is the first step into converting the meson/drm driver to use
the canvas module.

If a canvas provider node is detected in DT, use it. Otherwise,
fall back to what is currently being done.

Signed-off-by: Maxime Jourdan <[email protected]>
---
drivers/gpu/drm/meson/Kconfig | 1 +
drivers/gpu/drm/meson/meson_crtc.c | 14 ++++++---
drivers/gpu/drm/meson/meson_drv.c | 46 ++++++++++++++++++-----------
drivers/gpu/drm/meson/meson_drv.h | 4 +++
drivers/gpu/drm/meson/meson_plane.c | 8 ++++-
5 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig
index 3ce51d8dfe1c..c28b69f48555 100644
--- a/drivers/gpu/drm/meson/Kconfig
+++ b/drivers/gpu/drm/meson/Kconfig
@@ -7,6 +7,7 @@ config DRM_MESON
select DRM_GEM_CMA_HELPER
select VIDEOMODE_HELPERS
select REGMAP_MMIO
+ select MESON_CANVAS

config DRM_MESON_DW_HDMI
tristate "HDMI Synopsys Controller support for Amlogic Meson Display"
diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c
index 05520202c967..b3bc0b0ee07f 100644
--- a/drivers/gpu/drm/meson/meson_crtc.c
+++ b/drivers/gpu/drm/meson/meson_crtc.c
@@ -193,10 +193,16 @@ void meson_crtc_irq(struct meson_drm *priv)
} else
meson_vpp_disable_interlace_vscaler_osd1(priv);

- meson_canvas_setup(priv, MESON_CANVAS_ID_OSD1,
- priv->viu.osd1_addr, priv->viu.osd1_stride,
- priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE,
- MESON_CANVAS_BLKMODE_LINEAR);
+ if (priv->canvas)
+ meson_canvas_config(priv->canvas, priv->canvas_id_osd1,
+ priv->viu.osd1_addr, priv->viu.osd1_stride,
+ priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE,
+ MESON_CANVAS_BLKMODE_LINEAR, 0);
+ else
+ meson_canvas_setup(priv, MESON_CANVAS_ID_OSD1,
+ priv->viu.osd1_addr, priv->viu.osd1_stride,
+ priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE,
+ MESON_CANVAS_BLKMODE_LINEAR);

/* Enable OSD1 */
writel_bits_relaxed(VPP_OSD1_POSTBLEND, VPP_OSD1_POSTBLEND,
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index d3443125e661..b39c38c2350d 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -216,24 +216,33 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
goto free_drm;
}

- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc");
- if (!res) {
- ret = -EINVAL;
- goto free_drm;
- }
- /* Simply ioremap since it may be a shared register zone */
- regs = devm_ioremap(dev, res->start, resource_size(res));
- if (!regs) {
- ret = -EADDRNOTAVAIL;
- goto free_drm;
- }
+ priv->canvas = meson_canvas_get(dev);
+ if (!IS_ERR(priv->canvas)) {
+ ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_osd1);
+ if (ret)
+ goto free_drm;
+ } else {
+ priv->canvas = NULL;

- priv->dmc = devm_regmap_init_mmio(dev, regs,
- &meson_regmap_config);
- if (IS_ERR(priv->dmc)) {
- dev_err(&pdev->dev, "Couldn't create the DMC regmap\n");
- ret = PTR_ERR(priv->dmc);
- goto free_drm;
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc");
+ if (!res) {
+ ret = -EINVAL;
+ goto free_drm;
+ }
+ /* Simply ioremap since it may be a shared register zone */
+ regs = devm_ioremap(dev, res->start, resource_size(res));
+ if (!regs) {
+ ret = -EADDRNOTAVAIL;
+ goto free_drm;
+ }
+
+ priv->dmc = devm_regmap_init_mmio(dev, regs,
+ &meson_regmap_config);
+ if (IS_ERR(priv->dmc)) {
+ dev_err(&pdev->dev, "Couldn't create the DMC regmap\n");
+ ret = PTR_ERR(priv->dmc);
+ goto free_drm;
+ }
}

priv->vsync_irq = platform_get_irq(pdev, 0);
@@ -315,6 +324,9 @@ static void meson_drv_unbind(struct device *dev)
struct drm_device *drm = dev_get_drvdata(dev);
struct meson_drm *priv = drm->dev_private;

+ if (priv->canvas)
+ meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
+
drm_dev_unregister(drm);
drm_kms_helper_poll_fini(drm);
drm_fbdev_cma_fini(priv->fbdev);
diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
index 8450d6ac8c9b..728d0ca33732 100644
--- a/drivers/gpu/drm/meson/meson_drv.h
+++ b/drivers/gpu/drm/meson/meson_drv.h
@@ -22,6 +22,7 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/of.h>
+#include <linux/soc/amlogic/meson-canvas.h>
#include <drm/drmP.h>

struct meson_drm {
@@ -31,6 +32,9 @@ struct meson_drm {
struct regmap *dmc;
int vsync_irq;

+ struct meson_canvas *canvas;
+ u8 canvas_id_osd1;
+
struct drm_device *drm;
struct drm_crtc *crtc;
struct drm_fbdev_cma *fbdev;
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 12c80dfcff59..51bec8e98a39 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -90,6 +90,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
.y2 = state->crtc_y + state->crtc_h,
};
unsigned long flags;
+ u8 canvas_id_osd1;

/*
* Update Coordinates
@@ -104,8 +105,13 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
(0xFF << OSD_GLOBAL_ALPHA_SHIFT) |
OSD_BLK0_ENABLE;

+ if (priv->canvas)
+ canvas_id_osd1 = priv->canvas_id_osd1;
+ else
+ canvas_id_osd1 = MESON_CANVAS_ID_OSD1;
+
/* Set up BLK0 to point to the right canvas */
- priv->viu.osd1_blk0_cfg[0] = ((MESON_CANVAS_ID_OSD1 << OSD_CANVAS_SEL) |
+ priv->viu.osd1_blk0_cfg[0] = ((canvas_id_osd1 << OSD_CANVAS_SEL) |
OSD_ENDIANNESS_LE);

/* On GXBB, Use the old non-HDR RGB2YUV converter */
--
2.19.1


2018-11-05 10:47:18

by Maxime Jourdan

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: display: amlogic, meson-vpu: Add optional canvas provider node

Allows using the new canvas provider module if present.

Signed-off-by: Maxime Jourdan <[email protected]>
---
Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
index 057b81335775..c65fd7a7467c 100644
--- a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
+++ b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
@@ -67,6 +67,8 @@ Required properties:
Optional properties:
- power-domains: Optional phandle to associated power domain as described in
the file ../power/power_domain.txt
+- amlogic,canvas: phandle to canvas provider node as described in the file
+ ../soc/amlogic/amlogic,canvas.txt

Required nodes:

--
2.19.1


2018-11-05 12:52:39

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH 0/2] drm/meson: Allow using optional canvas provider

Hi Maxime,

On 05/11/2018 11:45, Maxime Jourdan wrote:
> The meson DRM driver currently uses constant, static canvas indexes.
>
> This is not optimal and could conflict with other drivers also using
> canvases.

Indeed, it's now time to use the canvas provider merged in 4.20-rc1 !

>
> This patch series allows the driver to optionnally use a canvas provider
> module that is able to dispatch canvases, on demand and free of conflict.
>
> In the future, the current way of doing things will be deprecated.

Do you confirm you will send a patch to remove the legacy canvas code when
everything is merged (including DT changes) ?

I'll wait until the bindings are reviewed, then I'll push the serie to drm-misc-next.

Thanks,
Neil

>
> Maxime Jourdan (2):
> dt-bindings: display: amlogic, meson-vpu: Add optional canvas provider
> node
> drm/meson: Use optional canvas provider
>
> .../bindings/display/amlogic,meson-vpu.txt | 2 +
> drivers/gpu/drm/meson/Kconfig | 1 +
> drivers/gpu/drm/meson/meson_crtc.c | 14 ++++--
> drivers/gpu/drm/meson/meson_drv.c | 46 ++++++++++++-------
> drivers/gpu/drm/meson/meson_drv.h | 4 ++
> drivers/gpu/drm/meson/meson_plane.c | 8 +++-
> 6 files changed, 53 insertions(+), 22 deletions(-)
>

2018-11-05 12:53:48

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH 2/2] drm/meson: Use optional canvas provider

On 05/11/2018 11:45, Maxime Jourdan wrote:
> This is the first step into converting the meson/drm driver to use
> the canvas module.
>
> If a canvas provider node is detected in DT, use it. Otherwise,
> fall back to what is currently being done.
>
> Signed-off-by: Maxime Jourdan <[email protected]>
> ---
> drivers/gpu/drm/meson/Kconfig | 1 +
> drivers/gpu/drm/meson/meson_crtc.c | 14 ++++++---
> drivers/gpu/drm/meson/meson_drv.c | 46 ++++++++++++++++++-----------
> drivers/gpu/drm/meson/meson_drv.h | 4 +++
> drivers/gpu/drm/meson/meson_plane.c | 8 ++++-
> 5 files changed, 51 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig
> index 3ce51d8dfe1c..c28b69f48555 100644
> --- a/drivers/gpu/drm/meson/Kconfig
> +++ b/drivers/gpu/drm/meson/Kconfig
> @@ -7,6 +7,7 @@ config DRM_MESON
> select DRM_GEM_CMA_HELPER
> select VIDEOMODE_HELPERS
> select REGMAP_MMIO
> + select MESON_CANVAS
>
> config DRM_MESON_DW_HDMI
> tristate "HDMI Synopsys Controller support for Amlogic Meson Display"
> diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c
> index 05520202c967..b3bc0b0ee07f 100644
> --- a/drivers/gpu/drm/meson/meson_crtc.c
> +++ b/drivers/gpu/drm/meson/meson_crtc.c
> @@ -193,10 +193,16 @@ void meson_crtc_irq(struct meson_drm *priv)
> } else
> meson_vpp_disable_interlace_vscaler_osd1(priv);
>
> - meson_canvas_setup(priv, MESON_CANVAS_ID_OSD1,
> - priv->viu.osd1_addr, priv->viu.osd1_stride,
> - priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE,
> - MESON_CANVAS_BLKMODE_LINEAR);
> + if (priv->canvas)
> + meson_canvas_config(priv->canvas, priv->canvas_id_osd1,
> + priv->viu.osd1_addr, priv->viu.osd1_stride,
> + priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE,
> + MESON_CANVAS_BLKMODE_LINEAR, 0);
> + else
> + meson_canvas_setup(priv, MESON_CANVAS_ID_OSD1,
> + priv->viu.osd1_addr, priv->viu.osd1_stride,
> + priv->viu.osd1_height, MESON_CANVAS_WRAP_NONE,
> + MESON_CANVAS_BLKMODE_LINEAR);
>
> /* Enable OSD1 */
> writel_bits_relaxed(VPP_OSD1_POSTBLEND, VPP_OSD1_POSTBLEND,
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index d3443125e661..b39c38c2350d 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -216,24 +216,33 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> goto free_drm;
> }
>
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc");
> - if (!res) {
> - ret = -EINVAL;
> - goto free_drm;
> - }
> - /* Simply ioremap since it may be a shared register zone */
> - regs = devm_ioremap(dev, res->start, resource_size(res));
> - if (!regs) {
> - ret = -EADDRNOTAVAIL;
> - goto free_drm;
> - }
> + priv->canvas = meson_canvas_get(dev);
> + if (!IS_ERR(priv->canvas)) {
> + ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_osd1);
> + if (ret)
> + goto free_drm;
> + } else {
> + priv->canvas = NULL;
>
> - priv->dmc = devm_regmap_init_mmio(dev, regs,
> - &meson_regmap_config);
> - if (IS_ERR(priv->dmc)) {
> - dev_err(&pdev->dev, "Couldn't create the DMC regmap\n");
> - ret = PTR_ERR(priv->dmc);
> - goto free_drm;
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc");
> + if (!res) {
> + ret = -EINVAL;
> + goto free_drm;
> + }
> + /* Simply ioremap since it may be a shared register zone */
> + regs = devm_ioremap(dev, res->start, resource_size(res));
> + if (!regs) {
> + ret = -EADDRNOTAVAIL;
> + goto free_drm;
> + }
> +
> + priv->dmc = devm_regmap_init_mmio(dev, regs,
> + &meson_regmap_config);
> + if (IS_ERR(priv->dmc)) {
> + dev_err(&pdev->dev, "Couldn't create the DMC regmap\n");
> + ret = PTR_ERR(priv->dmc);
> + goto free_drm;
> + }
> }
>
> priv->vsync_irq = platform_get_irq(pdev, 0);
> @@ -315,6 +324,9 @@ static void meson_drv_unbind(struct device *dev)
> struct drm_device *drm = dev_get_drvdata(dev);
> struct meson_drm *priv = drm->dev_private;
>
> + if (priv->canvas)
> + meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> +
> drm_dev_unregister(drm);
> drm_kms_helper_poll_fini(drm);
> drm_fbdev_cma_fini(priv->fbdev);
> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
> index 8450d6ac8c9b..728d0ca33732 100644
> --- a/drivers/gpu/drm/meson/meson_drv.h
> +++ b/drivers/gpu/drm/meson/meson_drv.h
> @@ -22,6 +22,7 @@
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> #include <linux/of.h>
> +#include <linux/soc/amlogic/meson-canvas.h>
> #include <drm/drmP.h>
>
> struct meson_drm {
> @@ -31,6 +32,9 @@ struct meson_drm {
> struct regmap *dmc;
> int vsync_irq;
>
> + struct meson_canvas *canvas;
> + u8 canvas_id_osd1;
> +
> struct drm_device *drm;
> struct drm_crtc *crtc;
> struct drm_fbdev_cma *fbdev;
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 12c80dfcff59..51bec8e98a39 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -90,6 +90,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
> .y2 = state->crtc_y + state->crtc_h,
> };
> unsigned long flags;
> + u8 canvas_id_osd1;
>
> /*
> * Update Coordinates
> @@ -104,8 +105,13 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
> (0xFF << OSD_GLOBAL_ALPHA_SHIFT) |
> OSD_BLK0_ENABLE;
>
> + if (priv->canvas)
> + canvas_id_osd1 = priv->canvas_id_osd1;
> + else
> + canvas_id_osd1 = MESON_CANVAS_ID_OSD1;
> +
> /* Set up BLK0 to point to the right canvas */
> - priv->viu.osd1_blk0_cfg[0] = ((MESON_CANVAS_ID_OSD1 << OSD_CANVAS_SEL) |
> + priv->viu.osd1_blk0_cfg[0] = ((canvas_id_osd1 << OSD_CANVAS_SEL) |
> OSD_ENDIANNESS_LE);
>
> /* On GXBB, Use the old non-HDR RGB2YUV converter */
>

Reviewed-by: Neil Armstrong <[email protected]>

2018-11-05 14:03:34

by Maxime Jourdan

[permalink] [raw]
Subject: Re: [PATCH 0/2] drm/meson: Allow using optional canvas provider

Hi Neil,

On Mon, Nov 5, 2018 at 1:51 PM Neil Armstrong <[email protected]> wrote:
>
> Hi Maxime,
>
> On 05/11/2018 11:45, Maxime Jourdan wrote:
> > The meson DRM driver currently uses constant, static canvas indexes.
> >
> > This is not optimal and could conflict with other drivers also using
> > canvases.
>
> Indeed, it's now time to use the canvas provider merged in 4.20-rc1 !
>
> >
> > This patch series allows the driver to optionnally use a canvas provider
> > module that is able to dispatch canvases, on demand and free of conflict.
> >
> > In the future, the current way of doing things will be deprecated.
>
> Do you confirm you will send a patch to remove the legacy canvas code when
> everything is merged (including DT changes) ?
>
> I'll wait until the bindings are reviewed, then I'll push the serie to drm-misc-next.
>

Yes, ultimately it will be removed.

The plan is:

4.21: allow using both modes, add the "amlogic,canvas" property to the dts files
4.22: remove the old way and change the amlogic,canvas property to mandatory

This should allow for a nice transition.

Maxime

> Thanks,
> Neil
>
> >
> > Maxime Jourdan (2):
> > dt-bindings: display: amlogic, meson-vpu: Add optional canvas provider
> > node
> > drm/meson: Use optional canvas provider
> >
> > .../bindings/display/amlogic,meson-vpu.txt | 2 +
> > drivers/gpu/drm/meson/Kconfig | 1 +
> > drivers/gpu/drm/meson/meson_crtc.c | 14 ++++--
> > drivers/gpu/drm/meson/meson_drv.c | 46 ++++++++++++-------
> > drivers/gpu/drm/meson/meson_drv.h | 4 ++
> > drivers/gpu/drm/meson/meson_plane.c | 8 +++-
> > 6 files changed, 53 insertions(+), 22 deletions(-)
> >

2018-11-13 13:33:31

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH 0/2] drm/meson: Allow using optional canvas provider

On 05/11/2018 15:02, Maxime Jourdan wrote:
> Hi Neil,
>
> On Mon, Nov 5, 2018 at 1:51 PM Neil Armstrong <[email protected]> wrote:
>>
>> Hi Maxime,
>>
>> On 05/11/2018 11:45, Maxime Jourdan wrote:
>>> The meson DRM driver currently uses constant, static canvas indexes.
>>>
>>> This is not optimal and could conflict with other drivers also using
>>> canvases.
>>
>> Indeed, it's now time to use the canvas provider merged in 4.20-rc1 !
>>
>>>
>>> This patch series allows the driver to optionnally use a canvas provider
>>> module that is able to dispatch canvases, on demand and free of conflict.
>>>
>>> In the future, the current way of doing things will be deprecated.
>>
>> Do you confirm you will send a patch to remove the legacy canvas code when
>> everything is merged (including DT changes) ?
>>
>> I'll wait until the bindings are reviewed, then I'll push the serie to drm-misc-next.
>>
>
> Yes, ultimately it will be removed.
>
> The plan is:
>
> 4.21: allow using both modes, add the "amlogic,canvas" property to the dts files
> 4.22: remove the old way and change the amlogic,canvas property to mandatory
>
> This should allow for a nice transition.
>
> Maxime
>
>> Thanks,
>> Neil
>>
>>>
>>> Maxime Jourdan (2):
>>> dt-bindings: display: amlogic, meson-vpu: Add optional canvas provider
>>> node
>>> drm/meson: Use optional canvas provider
>>>
>>> .../bindings/display/amlogic,meson-vpu.txt | 2 +
>>> drivers/gpu/drm/meson/Kconfig | 1 +
>>> drivers/gpu/drm/meson/meson_crtc.c | 14 ++++--
>>> drivers/gpu/drm/meson/meson_drv.c | 46 ++++++++++++-------
>>> drivers/gpu/drm/meson/meson_drv.h | 4 ++
>>> drivers/gpu/drm/meson/meson_plane.c | 8 +++-
>>> 6 files changed, 53 insertions(+), 22 deletions(-)
>>>

Applied