Re: [PATCH 1/2] drm/ingenic: Switch IPU plane to type OVERLAY

2021-03-30 Thread Simon Ser 
On Tuesday, March 30th, 2021 at 1:53 PM, Paul Cercueil  
wrote:

> > I don't know about this driver but… is this really the same as the
> > previous
> > condition? The previous condition would match two planes, this one
> > seems to
> > match only a single plane. What am I missing?
>
> There are three planes, which we will call here f0, f1, and ipu.
> Previously, the "plane->type == DRM_PLANE_TYPE_PRIMARY" matched f1 and
> ipu. Since ipu is now OVERLAY we have to change the condition or the
> behaviour will be different, as otherwise it would only match the f1
> plane.

Oh okay, I thought f0 was one of the primary planes, but it's not.
Thanks for the explanation.

For the user-space visible change:

Acked-by: Simon Ser

Re: [PATCH 1/2] drm/ingenic: Switch IPU plane to type OVERLAY

2021-03-30 Thread Paul Cercueil 

Hi Simon,

Le mar. 30 mars 2021 à 7:23, Simon Ser  a écrit :

 It should have been an OVERLAY from the beginning. The documentation
 stipulates that there should be an unique PRIMARY plane per CRTC.


Thanks for the quick patch! One comment below…


 Fixes: fc1acf317b01 ("drm/ingenic: Add support for the IPU")
 Cc:  # 5.8+
 Signed-off-by: Paul Cercueil 
 ---
  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 11 +--
  drivers/gpu/drm/ingenic/ingenic-ipu.c |  2 +-
  2 files changed, 6 insertions(+), 7 deletions(-)

 diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c 
b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c

 index 29742ec5ab95..09225b770bb8 100644
 --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
 +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
 @@ -419,7 +419,7 @@ static void ingenic_drm_plane_enable(struct 
ingenic_drm *priv,

unsigned int en_bit;

if (priv->soc_info->has_osd) {
 -  if (plane->type == DRM_PLANE_TYPE_PRIMARY)
 +  if (plane != &priv->f0)


I don't know about this driver but… is this really the same as the 
previous
condition? The previous condition would match two planes, this one 
seems to

match only a single plane. What am I missing?


There are three planes, which we will call here f0, f1, and ipu.

Previously, the "plane->type == DRM_PLANE_TYPE_PRIMARY" matched f1 and 
ipu. Since ipu is now OVERLAY we have to change the condition or the 
behaviour will be different, as otherwise it would only match the f1 
plane.


Cheers,
-Paul

Re: [PATCH 1/2] drm/ingenic: Switch IPU plane to type OVERLAY

2021-03-30 Thread Simon Ser 
> It should have been an OVERLAY from the beginning. The documentation
> stipulates that there should be an unique PRIMARY plane per CRTC.

Thanks for the quick patch! One comment below…

> Fixes: fc1acf317b01 ("drm/ingenic: Add support for the IPU")
> Cc:  # 5.8+
> Signed-off-by: Paul Cercueil 
> ---
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 11 +--
>  drivers/gpu/drm/ingenic/ingenic-ipu.c |  2 +-
>  2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index 29742ec5ab95..09225b770bb8 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -419,7 +419,7 @@ static void ingenic_drm_plane_enable(struct ingenic_drm 
> *priv,
>   unsigned int en_bit;
>
>   if (priv->soc_info->has_osd) {
> - if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> + if (plane != &priv->f0)

I don't know about this driver but… is this really the same as the previous
condition? The previous condition would match two planes, this one seems to
match only a single plane. What am I missing?

[PATCH 1/2] drm/ingenic: Switch IPU plane to type OVERLAY

2021-03-29 Thread Paul Cercueil 
It should have been an OVERLAY from the beginning. The documentation
stipulates that there should be an unique PRIMARY plane per CRTC.

Fixes: fc1acf317b01 ("drm/ingenic: Add support for the IPU")
Cc:  # 5.8+
Signed-off-by: Paul Cercueil 
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 11 +--
 drivers/gpu/drm/ingenic/ingenic-ipu.c |  2 +-
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c 
b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 29742ec5ab95..09225b770bb8 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -419,7 +419,7 @@ static void ingenic_drm_plane_enable(struct ingenic_drm 
*priv,
unsigned int en_bit;
 
if (priv->soc_info->has_osd) {
-   if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+   if (plane != &priv->f0)
en_bit = JZ_LCD_OSDC_F1EN;
else
en_bit = JZ_LCD_OSDC_F0EN;
@@ -434,7 +434,7 @@ void ingenic_drm_plane_disable(struct device *dev, struct 
drm_plane *plane)
unsigned int en_bit;
 
if (priv->soc_info->has_osd) {
-   if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+   if (plane != &priv->f0)
en_bit = JZ_LCD_OSDC_F1EN;
else
en_bit = JZ_LCD_OSDC_F0EN;
@@ -461,8 +461,7 @@ void ingenic_drm_plane_config(struct device *dev,
 
ingenic_drm_plane_enable(priv, plane);
 
-   if (priv->soc_info->has_osd &&
-   plane->type == DRM_PLANE_TYPE_PRIMARY) {
+   if (priv->soc_info->has_osd && plane != &priv->f0) {
switch (fourcc) {
case DRM_FORMAT_XRGB1555:
ctrl |= JZ_LCD_OSDCTRL_RGB555;
@@ -510,7 +509,7 @@ void ingenic_drm_plane_config(struct device *dev,
}
 
if (priv->soc_info->has_osd) {
-   if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
+   if (plane != &priv->f0) {
xy_reg = JZ_REG_LCD_XYP1;
size_reg = JZ_REG_LCD_SIZE1;
} else {
@@ -561,7 +560,7 @@ static void ingenic_drm_plane_atomic_update(struct 
drm_plane *plane,
height = newstate->src_h >> 16;
cpp = newstate->fb->format->cpp[0];
 
-   if (!priv->soc_info->has_osd || plane->type == 
DRM_PLANE_TYPE_OVERLAY)
+   if (!priv->soc_info->has_osd || plane == &priv->f0)
hwdesc = &priv->dma_hwdescs->hwdesc_f0;
else
hwdesc = &priv->dma_hwdescs->hwdesc_f1;
diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c 
b/drivers/gpu/drm/ingenic/ingenic-ipu.c
index 5ae6adab8306..3b1091e7c0cd 100644
--- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
+++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
@@ -767,7 +767,7 @@ static int ingenic_ipu_bind(struct device *dev, struct 
device *master, void *d)
 
err = drm_universal_plane_init(drm, plane, 1, &ingenic_ipu_plane_funcs,
   soc_info->formats, soc_info->num_formats,
-  NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+  NULL, DRM_PLANE_TYPE_OVERLAY, NULL);
if (err) {
dev_err(dev, "Failed to init plane: %i\n", err);
return err;
-- 
2.30.2