On Wed, Oct 19, 2016 at 12:07 AM, Philipp Zabel p.zabel@pengutronix.de wrote:

Without this patch, after enabling the overlay plane with an RGBA framebuffer, switching to a framebuffer without alpha channel would cause the plane to vanish, since the pixel local alpha is constant zero in that case. Disable local alpha again when setting an opaque framebuffer.

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

Looking at the big picture, we've got some hard coding for DP CSC&alpha :( However, looking at this patch itself, it is acceptable to me.

Regards, Liu Ying

---

drivers/gpu/drm/imx/ipuv3-plane.c | 1 + 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 3a03fd8..d4a771c 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -434,6 +434,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane, ipu_dp_set_global_alpha(ipu_plane->dp, false, 0, false); break; default:

•                   ipu_dp_set_global_alpha(ipu_plane->dp, true, 0, true);
                    break;
            }
    }
  

-- 2.9.3

On Wed, Oct 19, 2016 at 12:07 AM, Philipp Zabel p.zabel@pengutronix.de wrote:

If the framebuffer pixel format is planar YUV and unchanged, but the U or V plane offsets change, do not return an error, but request a modeset instead.

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

Acked-by: Liu Ying gnuiyl@gmail.com

---

drivers/gpu/drm/imx/ipuv3-plane.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index d4a771c..f217444 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -356,13 +356,11 @@ static int ipu_plane_atomic_check(struct drm_plane *plane, if ((ubo > 0xfffff8) || (vbo > 0xfffff8)) return -EINVAL;

•           if (old_fb &&
  

•               (old_fb->pixel_format == DRM_FORMAT_YUV420 ||
  

•                old_fb->pixel_format == DRM_FORMAT_YVU420)) {
  

•           if (old_fb && (fb->pixel_format == old_fb->pixel_format)) {
                    old_ubo = drm_plane_state_to_ubo(old_state);
                    old_vbo = drm_plane_state_to_vbo(old_state);
                    if (ubo != old_ubo || vbo != old_vbo)
  

•                           return -EINVAL;
  

•                           crtc_state->mode_changed = true;
            }
  
            if (fb->pitches[1] != fb->pitches[2])
  

-- 2.9.3

On Wed, Oct 19, 2016 at 12:07 AM, Philipp Zabel p.zabel@pengutronix.de wrote:

ipu_plane_atomic_set_base is called from ipu_plane_atomic_update in two different places, depending on whether drm_atomic_crtc_needs_modeset is true. Also depending on the same condition, this function does two different things. This patch removes the indirection by merging the relevant parts into ipu_plane_atomic_update, making the actual code flow more obvious as a result. Also remove the duplicate planar format comment, which is already found in ipu_plane_atomic_check.

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

Acked-by: Liu Ying gnuiyl@gmail.com

---

drivers/gpu/drm/imx/ipuv3-plane.c | 97 ++++++++++++++------------------------- 1 file changed, 34 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index f217444..70fd55d 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -103,62 +103,6 @@ drm_plane_state_to_vbo(struct drm_plane_state *state) (state->src_x >> 16) / 2 - eba; }

-static void ipu_plane_atomic_set_base(struct ipu_plane *ipu_plane) -{

•   struct drm_plane *plane = &ipu_plane->base;
  

•   struct drm_plane_state *state = plane->state;
  

•   struct drm_crtc_state *crtc_state = state->crtc->state;
  

•   struct drm_framebuffer *fb = state->fb;
  

•   unsigned long eba, ubo, vbo;
  

•   int active;
  

•   eba = drm_plane_state_to_eba(state);
  

•   switch (fb->pixel_format) {
  

•   case DRM_FORMAT_YUV420:
  

•   case DRM_FORMAT_YVU420:
  

•           if (!drm_atomic_crtc_needs_modeset(crtc_state))
  

•                   break;
  

•           /*
  

•            * Multiplanar formats have to meet the following restrictions:
  

•            * - The (up to) three plane addresses are EBA, EBA+UBO, EBA+VBO
  

•            * - EBA, UBO and VBO are a multiple of 8
  

•            * - UBO and VBO are unsigned and not larger than 0xfffff8
  

•            * - Only EBA may be changed while scanout is active
  

•            * - The strides of U and V planes must be identical.
  

•            */
  

•           ubo = drm_plane_state_to_ubo(state);
  

•           vbo = drm_plane_state_to_vbo(state);
  

•           if (fb->pixel_format == DRM_FORMAT_YUV420)
  

•                   ipu_cpmem_set_yuv_planar_full(ipu_plane->ipu_ch,
  

•                                                 fb->pitches[1], ubo, vbo);
  

•           else
  

•                   ipu_cpmem_set_yuv_planar_full(ipu_plane->ipu_ch,
  

•                                                 fb->pitches[1], vbo, ubo);
  

•           dev_dbg(ipu_plane->base.dev->dev,
  

•                   "phy = %lu %lu %lu, x = %d, y = %d", eba, ubo, vbo,
  

•                   state->src_x >> 16, state->src_y >> 16);
  

•           break;
  

•   default:
  

•           dev_dbg(ipu_plane->base.dev->dev, "phys = %lu, x = %d, y = %d",
  

•                   eba, state->src_x >> 16, state->src_y >> 16);
  

•           break;
  

•   }
  

•   if (!drm_atomic_crtc_needs_modeset(crtc_state)) {
  

•           active = ipu_idmac_get_current_buffer(ipu_plane->ipu_ch);
  

•           ipu_cpmem_set_buffer(ipu_plane->ipu_ch, !active, eba);
  

•           ipu_idmac_select_buffer(ipu_plane->ipu_ch, !active);
  

•   } else {
  

•           ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 0, eba);
  

•           ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 1, eba);
  

•   }
  

-}

void ipu_plane_put_resources(struct ipu_plane *ipu_plane) { if (!IS_ERR_OR_NULL(ipu_plane->dp)) @@ -394,15 +338,19 @@ static void ipu_plane_atomic_update(struct drm_plane *plane, { struct ipu_plane *ipu_plane = to_ipu_plane(plane); struct drm_plane_state *state = plane->state;

•   struct drm_crtc_state *crtc_state = state->crtc->state;
  

•   struct drm_framebuffer *fb = state->fb;
  

•   unsigned long eba, ubo, vbo;
    enum ipu_color_space ics;
  

•   int active;
  
  

•   if (old_state->fb) {
  

•           struct drm_crtc_state *crtc_state = state->crtc->state;
  

•   eba = drm_plane_state_to_eba(state);
  
  

•           if (!drm_atomic_crtc_needs_modeset(crtc_state)) {
  

•                   ipu_plane_atomic_set_base(ipu_plane);
  

•                   return;
  

•           }
  

•   if (old_state->fb && !drm_atomic_crtc_needs_modeset(crtc_state)) {
  

•           active = ipu_idmac_get_current_buffer(ipu_plane->ipu_ch);
  

•           ipu_cpmem_set_buffer(ipu_plane->ipu_ch, !active, eba);
  

•           ipu_idmac_select_buffer(ipu_plane->ipu_ch, !active);
  

•           return;
    }
  
    switch (ipu_plane->dp_flow) {
  

@@ -446,7 +394,30 @@ static void ipu_plane_atomic_update(struct drm_plane *plane, ipu_cpmem_set_high_priority(ipu_plane->ipu_ch); ipu_idmac_set_double_buffer(ipu_plane->ipu_ch, 1); ipu_cpmem_set_stride(ipu_plane->ipu_ch, state->fb->pitches[0]);

•   ipu_plane_atomic_set_base(ipu_plane);
  

•   switch (fb->pixel_format) {
  

•   case DRM_FORMAT_YUV420:
  

•   case DRM_FORMAT_YVU420:
  

•           ubo = drm_plane_state_to_ubo(state);
  

•           vbo = drm_plane_state_to_vbo(state);
  

•           if (fb->pixel_format == DRM_FORMAT_YUV420)
  

•                   ipu_cpmem_set_yuv_planar_full(ipu_plane->ipu_ch,
  

•                                                 fb->pitches[1], ubo, vbo);
  

•           else
  

•                   ipu_cpmem_set_yuv_planar_full(ipu_plane->ipu_ch,
  

•                                                 fb->pitches[1], vbo, ubo);
  

•           dev_dbg(ipu_plane->base.dev->dev,
  

•                   "phy = %lu %lu %lu, x = %d, y = %d", eba, ubo, vbo,
  

•                   state->src_x >> 16, state->src_y >> 16);
  

•           break;
  

•   default:
  

•           dev_dbg(ipu_plane->base.dev->dev, "phys = %lu, x = %d, y = %d",
  

•                   eba, state->src_x >> 16, state->src_y >> 16);
  

•           break;
  

•   }
  

•   ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 0, eba);
  

•   ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 1, eba);
    ipu_plane_enable(ipu_plane);
  

}

-- 2.9.3

On Wed, Oct 19, 2016 at 12:07 AM, Philipp Zabel p.zabel@pengutronix.de wrote:

To support 4:2:2 or 4:4:4 chroma subsampling, divide the x/y offsets in drm_plane_state_to_ubo/vbo only if necessary for the given pixel format.

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

drivers/gpu/drm/imx/ipuv3-plane.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 70fd55d..88829b9 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -64,13 +64,14 @@ drm_plane_state_to_eba(struct drm_plane_state *state) { struct drm_framebuffer *fb = state->fb; struct drm_gem_cma_object *cma_obj;

•   int x = state->src_x >> 16;
  

•   int y = state->src_y >> 16;
  
    cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
    BUG_ON(!cma_obj);
  
  

•   return cma_obj->paddr + fb->offsets[0] +
  

•          fb->pitches[0] * (state->src_y >> 16) +
  

•          (fb->bits_per_pixel >> 3) * (state->src_x >> 16);
  

•   return cma_obj->paddr + fb->offsets[0] + fb->pitches[0] * y +
  

•          drm_format_plane_cpp(fb->pixel_format, 0) * x;
  

I need some time to look into drm_format_plane_cpp(). Will provide comments after that.

Regards, Liu Ying

}

static inline unsigned long @@ -79,13 +80,17 @@ drm_plane_state_to_ubo(struct drm_plane_state *state) struct drm_framebuffer *fb = state->fb; struct drm_gem_cma_object *cma_obj; unsigned long eba = drm_plane_state_to_eba(state);

•   int x = state->src_x >> 16;
  

•   int y = state->src_y >> 16;
  
    cma_obj = drm_fb_cma_get_gem_obj(fb, 1);
    BUG_ON(!cma_obj);
  
  

•   return cma_obj->paddr + fb->offsets[1] +
  

•          fb->pitches[1] * (state->src_y >> 16) / 2 +
  

•          (state->src_x >> 16) / 2 - eba;
  

•   x /= drm_format_horz_chroma_subsampling(fb->pixel_format);
  

•   y /= drm_format_vert_chroma_subsampling(fb->pixel_format);
  

•   return cma_obj->paddr + fb->offsets[1] + fb->pitches[1] * y +
  

•          drm_format_plane_cpp(fb->pixel_format, 1) * x - eba;
  

}

static inline unsigned long @@ -94,13 +99,17 @@ drm_plane_state_to_vbo(struct drm_plane_state *state) struct drm_framebuffer *fb = state->fb; struct drm_gem_cma_object *cma_obj; unsigned long eba = drm_plane_state_to_eba(state);

•   int x = state->src_x >> 16;
  

•   int y = state->src_y >> 16;
  
    cma_obj = drm_fb_cma_get_gem_obj(fb, 2);
    BUG_ON(!cma_obj);
  
  

•   return cma_obj->paddr + fb->offsets[2] +
  

•          fb->pitches[2] * (state->src_y >> 16) / 2 +
  

•          (state->src_x >> 16) / 2 - eba;
  

•   x /= drm_format_horz_chroma_subsampling(fb->pixel_format);
  

•   y /= drm_format_vert_chroma_subsampling(fb->pixel_format);
  

•   return cma_obj->paddr + fb->offsets[2] + fb->pitches[2] * y +
  

•          drm_format_plane_cpp(fb->pixel_format, 2) * x - eba;
  

}

void ipu_plane_put_resources(struct ipu_plane *ipu_plane)

2.9.3

On Wed, Oct 19, 2016 at 12:07 AM, Philipp Zabel p.zabel@pengutronix.de wrote:

ipu_cpmem_set_fmt is already prepared to handle DRM_FORMAT_YVU422.

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

drivers/gpu/ipu-v3/ipu-cpmem.c | 5 +++++ 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c index fcb7dc8..e6b8bae 100644 --- a/drivers/gpu/ipu-v3/ipu-cpmem.c +++ b/drivers/gpu/ipu-v3/ipu-cpmem.c @@ -440,6 +440,11 @@ void ipu_cpmem_set_yuv_planar(struct ipuv3_channel *ch, u_offset = stride * height; v_offset = u_offset + (uv_stride * height); break;

•   case DRM_FORMAT_YVU422:
  

•           uv_stride = stride / 2;
  

•           v_offset = stride * height;
  

•           u_offset = v_offset + (uv_stride * height);
  

•           break;
  

It looks no one is using this function. How about removing it directly?

Regards, Liu Ying

    case DRM_FORMAT_NV12:
    case DRM_FORMAT_NV16:
            uv_stride = stride;

-- 2.9.3

On Wed, Oct 19, 2016 at 12:07 AM, Philipp Zabel p.zabel@pengutronix.de wrote:

The IDMAC does support reading and writing DRM_FORMAT_YUV444 and DRM_FORMAT_YVU444.

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

drivers/gpu/ipu-v3/ipu-common.c | 2 ++ drivers/gpu/ipu-v3/ipu-cpmem.c | 17 +++++++++++++++++ 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c index b9539f7..b7d7bd6 100644 --- a/drivers/gpu/ipu-v3/ipu-common.c +++ b/drivers/gpu/ipu-v3/ipu-common.c @@ -88,6 +88,8 @@ enum ipu_color_space ipu_drm_fourcc_to_colorspace(u32 drm_fourcc) case DRM_FORMAT_YVU420: case DRM_FORMAT_YUV422: case DRM_FORMAT_YVU422:

•   case DRM_FORMAT_YUV444:
  

•   case DRM_FORMAT_YVU444:
    case DRM_FORMAT_NV12:
    case DRM_FORMAT_NV21:
    case DRM_FORMAT_NV16:
  

diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c index e6b8bae..4db7475 100644 --- a/drivers/gpu/ipu-v3/ipu-cpmem.c +++ b/drivers/gpu/ipu-v3/ipu-cpmem.c @@ -445,6 +445,16 @@ void ipu_cpmem_set_yuv_planar(struct ipuv3_channel *ch, v_offset = stride * height; u_offset = v_offset + (uv_stride * height); break;

•   case DRM_FORMAT_YUV444:
  

•           uv_stride = stride;
  

•           u_offset = stride * height;
  

•           v_offset = u_offset + (uv_stride * height);
  

•           break;
  

•   case DRM_FORMAT_YVU444:
  

•           uv_stride = stride;
  

•           v_offset = stride * height;
  

•           u_offset = v_offset + (uv_stride * height);
  

•           break;
  

This function can be removed probably as I mentioned in comments for patch 6/9.

With that addressed: Acked-by: Liu Ying gnuiyl@gmail.com

Regards, Liu Ying

    case DRM_FORMAT_NV12:
    case DRM_FORMAT_NV16:
            uv_stride = stride;

@@ -595,6 +605,13 @@ int ipu_cpmem_set_fmt(struct ipuv3_channel *ch, u32 drm_fourcc) /* burst size */ ipu_ch_param_write_field(ch, IPU_FIELD_NPB, 31); break;

•   case DRM_FORMAT_YUV444:
  

•   case DRM_FORMAT_YVU444:
  

•           /* pix format */
  

•           ipu_ch_param_write_field(ch, IPU_FIELD_PFS, 0);
  

•           /* burst size */
  

•           ipu_ch_param_write_field(ch, IPU_FIELD_NPB, 31);
  

•           break;
    case DRM_FORMAT_NV12:
            /* pix format */
            ipu_ch_param_write_field(ch, IPU_FIELD_PFS, 4);
  

-- 2.9.3

Am Mittwoch, den 19.10.2016, 15:49 +0800 schrieb Ying Liu: [...]

...

@@ -406,10 +425,16 @@ static void ipu_plane_atomic_update(struct drm_plane *plane, switch (fb->pixel_format) { case DRM_FORMAT_YUV420: case DRM_FORMAT_YVU420:

•   case DRM_FORMAT_YUV422:
  

•   case DRM_FORMAT_YVU422:
  

•   case DRM_FORMAT_YUV444:
  

•   case DRM_FORMAT_YVU444:
            ubo = drm_plane_state_to_ubo(state);
            vbo = drm_plane_state_to_vbo(state);
  
  

•           if (fb->pixel_format == DRM_FORMAT_YUV420)
  

•           if (fb->pixel_format != DRM_FORMAT_YVU420 &&
  

•               fb->pixel_format != DRM_FORMAT_YVU422 &&
  

•               fb->pixel_format != DRM_FORMAT_YVU444)
  

Nit: This looks more straightforward, perhaps.

•           if (fb->pixel_format == DRM_FORMAT_YUV420 ||
  

•               fb->pixel_format == DRM_FORMAT_YUV422 ||
  

•               fb->pixel_format == DRM_FORMAT_YUV444)
  

I've inverted it at first because I mistakenly thought I'd also have to add NV12 and NV16 to the list otherwise. I'd like to change this to:

ubo = drm_plane_state_to_ubo(state); vbo = drm_plane_state_to_vbo(state);

if (fb->pixel_format == DRM_FORMAT_YVU420 || fb->pixel_format == DRM_FORMAT_YVU422 || fb->pixel_format == DRM_FORMAT_YVU444) swap(ubo, vbo);

ipu_cpmem_set_yuv_planar_full(ipu_plane->ipu_ch, fb->pitches[1], ubo, vbo);

to make it even more obvious that we just switch the U/V buffer offsets for YVU formats.

thanks Philipp

On Wed, Oct 19, 2016 at 12:07 AM, Philipp Zabel p.zabel@pengutronix.de wrote:

If we want to stop resetting the IPU in the future, masking all interrupts before registering the irq handlers will not be enough to avoid spurious interrupts. We also have to clear them.

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

Acked-by: Liu Ying gnuiyl@gmail.com

---

drivers/gpu/ipu-v3/ipu-common.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c index b7d7bd6..97218af 100644 --- a/drivers/gpu/ipu-v3/ipu-common.c +++ b/drivers/gpu/ipu-v3/ipu-common.c @@ -1286,8 +1286,11 @@ static int ipu_irq_init(struct ipu_soc *ipu) return ret; }

•   for (i = 0; i < IPU_NUM_IRQS; i += 32)
  

•   /* Mask and clear all interrupts */
  

•   for (i = 0; i < IPU_NUM_IRQS; i += 32) {
            ipu_cm_write(ipu, 0, IPU_INT_CTRL(i / 32));
  

•           ipu_cm_write(ipu, ~unused[i / 32], IPU_INT_STAT(i / 32));
  

•   }
  
    for (i = 0; i < IPU_NUM_IRQS; i += 32) {
            gc = irq_get_domain_generic_chip(ipu->domain, i);
  

-- 2.9.3

Am Mittwoch, den 19.10.2016, 15:12 +0800 schrieb Ying Liu:

On Wed, Oct 19, 2016 at 12:07 AM, Philipp Zabel p.zabel@pengutronix.de wrote:

...

Odd x/y offsets are not allowed for chroma subsampled planar YUV formats.

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++++++++ 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 5c34299..3a03fd8 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -259,6 +259,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane, struct drm_framebuffer *fb = state->fb; struct drm_framebuffer *old_fb = old_state->fb; unsigned long eba, ubo, vbo, old_ubo, old_vbo;

•   int hsub, vsub;
  
    /* Ok to disable */
    if (!fb)
  

@@ -372,6 +373,13 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,

            if (old_fb && old_fb->pitches[1] != fb->pitches[1])
                    crtc_state->mode_changed = true;

•           /* x and y offsets must be even in case of chroma subsampling */
  

True for x offsets, but not for y offsets, e.g., NV16.

That should be ok, for NV16 vsub==1, so (y&(vsub-1)) will always be false. Only for 4:2:0 subsampled formats is vsub==2, and then (y&(vsub-1)) will only be true if y is odd. I'll change the comment to: /* * The x/y offsets must be even in case of horizontal/vertical * chroma subsampling. */

regards Philipp

Am Mittwoch, den 19.10.2016, 10:58 +0300 schrieb Ville Syrjälä:

On Tue, Oct 18, 2016 at 06:07:49PM +0200, Philipp Zabel wrote:

...

Odd x/y offsets are not allowed for chroma subsampled planar YUV formats.

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++++++++ 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 5c34299..3a03fd8 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -259,6 +259,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane, struct drm_framebuffer *fb = state->fb; struct drm_framebuffer *old_fb = old_state->fb; unsigned long eba, ubo, vbo, old_ubo, old_vbo;

• int hsub, vsub;

  /* Ok to disable */ if (!fb)

@@ -372,6 +373,13 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,

if (old_fb && old_fb->pitches[1] != fb->pitches[1])
	crtc_state->mode_changed = true;

• /* x and y offsets must be even in case of chroma subsampling */
  

• hsub = drm_format_horz_chroma_subsampling(fb->pixel_format);
  

• vsub = drm_format_vert_chroma_subsampling(fb->pixel_format);
  

• if (((state->src_x >> 16) & (hsub - 1)) ||
  

•     ((state->src_y >> 16) & (vsub - 1)))
  

BTW you should use clipped coordinates all over instead of the raw user coordinates. I see you're rejecting negative crtc user coordinates and whanot so I guess it sort of works, but that's not how it's supposed to be done. drm_plane_helper_check_state() should give you most of what's needed.

Thank you for the pointer, I'll add a patch to use drm_plane_helper_check_state.

regards Philipp