Message-ID: <1480604047.2448.34.camel@pengutronix.de>
Subject: Re: [i.MX6 DRM IPUv3] Regression 4.9-rc5: greenish screen with
 YUV420 video
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Krzysztof =?UTF-8?Q?Ha=C5=82asa?= <khalasa@piap.pl>
Cc: David Airlie <airlied@linux.ie>, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
Date: Thu, 01 Dec 2016 15:54:07 +0100
In-Reply-To: <m3shqq30hc.fsf@t19.piap.pl>
References: <m3shqq30hc.fsf@t19.piap.pl>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1573
Lines: 41

Hi Krzysztof,

Am Donnerstag, den 17.11.2016, 10:43 +0100 schrieb Krzysztof Hałasa:
> Hi,
> 
> The following GStreamer pipeline causes screen to become green with
> v4.9-rc4+:
> 
> gst-launch-1.0 udpsrc uri=udp://239.1.2.2:5100 reuse=true caps="application/x-rtp,media=(string)video,clock-rate=(int)90000,encoding-name=(string)H264" ! rtph264depay ! h264parse ! v4l2video1dec capture-io-mode=dmabuf ! kmssink name=imx-drm sync=0
> 
> Reverting "drm/imx: ipuv3-plane: Skip setting u/vbo only when we don't
> need modeset", commit 81d553545a1510ff7c7c00cbc9b57d6172d411a4, fixes
> the problem.
> 
> Hardware is Gateworks Ventana GW5400, i.MX6Q, HDMI output, no X (console
> only).

I had already (accidentally) fixed this with 3fd8b292ae6b ("drm/imx:
ipuv3-plane: merge ipu_plane_atomic_set_base into atomic_update").

The bug is in this hunk in commit 81d553545a15:
 @@ -118,7 +117,7 @@ static void ipu_plane_atomic_set_base(struct ipu_plane *ipu_plane,
        switch (fb->pixel_format) {
        case DRM_FORMAT_YUV420:
        case DRM_FORMAT_YVU420:
-               if (old_state->fb)
+               if (!drm_atomic_crtc_needs_modeset(crtc_state))
                        break;
 
                /*

When the plane is first enabled, a modeset is not needed, so
drm_atomic_crtc_needs_modeset(crtc_state) is false and
ipu_plane_atomic_set_base then skipped the ubo/vbo setup.

When moving the code around in 3fd8b292ae6b I assumed that this
condition should never be true where the code ended up (as it was for
old_state->fb before), and I removed it.

regards
Philipp