Re: [PATCH v3 03/10] drm/imx: atomic phase 1: Use transitional atomic CRTC and plane helpers

[Russell King - ARM Linux <linux@armlinux.org.uk>]

On Sat, Aug 13, 2016 at 11:11:54AM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 04, 2016 at 03:40:32PM +0800, Liu Ying wrote:
> > Use the drm_plane_helper_update/disable() and drm_helper_crtc_mode_set()
> > transitional atomic helpers.  The crtc->mode_set_nofb callback is added
> > so that the primary plane is no longer tied to the CRTC.  Check/update
> > logics are separated to make sure crtc->mode_set_nofb and plane->atomic_update
> > are always successful.  Also, some necessary logics are tweaked for a smooth
> > transition.
> > 
> > Signed-off-by: Liu Ying <gnuiyl@gmail.com>
> 
> This patch causes a regression with my xorg ddx driver:
> 
> [    29.850] (EE) armada(0): [drm] failed to set mode on crtc 24: Invalid argument
> 
> I'm not sure why (yet).

Hi,

Enabling DRM debugging doesn't seem to provide much in the way of clues:

[drm:drm_ioctl] pid=1015, dev=0xe200, auth=1, DRM_IOCTL_MODE_SETCRTC
[drm:drm_mode_setcrtc] [CRTC:24:crtc-0]
[drm:drm_mode_setcrtc] [CONNECTOR:34:HDMI-A-1]
[drm:drm_crtc_helper_set_config]
[drm:drm_crtc_helper_set_config] [CRTC:24:crtc-0] [FB:48] #connectors=1 (x y) (0 0)
[drm:drm_crtc_helper_set_config] [CONNECTOR:34:HDMI-A-1] to [CRTC:24:crtc-0]
[drm:drm_crtc_helper_set_config] attempting to set mode from userspace
[drm:drm_mode_debug_printmodeline] Modeline 49:"1920x1080" 0 148500 1920 2448 2492 2640 1080 1084 1089 1125 0x0 0x5
[drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
[drm:drm_vblank_off] crtc 0, vblank enabled 0, inmodeset 0
[drm:drm_update_vblank_count] updating vblank count on crtc 0: current=2, diff=0, hw=0 hw_last=0
[drm:drm_mode_object_reference] OBJ ID: 51 (1)
[drm:drm_mode_object_unreference] OBJ ID: 51 (2)
[drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81ee00
[drm:drm_mode_object_reference] OBJ ID: 47 (3)
[drm:drm_mode_object_unreference] OBJ ID: 47 (4)
[drm:drm_mode_object_reference] OBJ ID: 48 (2)
[drm:drm_atomic_set_fb_for_plane] Set [FB:48] for plane state ed632d00
[drm:drm_mode_object_unreference] OBJ ID: 48 (3)
[drm:drm_mode_object_unreference] OBJ ID: 51 (1)
[drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]
[drm:drm_crtc_helper_set_mode] [CRTC:24:crtc-0]
[drm:drm_mode_object_reference] OBJ ID: 52 (1)
[drm:drm_mode_object_unreference] OBJ ID: 52 (2)
[drm:drm_atomic_set_mode_for_crtc] Set [MODE:1920x1080] for CRTC state ed81f600
[drm:drm_mode_object_reference] OBJ ID: 47 (3)
[drm:drm_mode_object_unreference] OBJ ID: 47 (4)
[drm:drm_mode_object_reference] OBJ ID: 47 (3)
[drm:drm_atomic_set_fb_for_plane] Set [FB:47] for plane state ed632d00
[drm:drm_mode_object_unreference] OBJ ID: 47 (4)
[drm:drm_mode_object_unreference] OBJ ID: 52 (1)
[drm:drm_crtc_helper_set_mode] [ENCODER:33:TMDS-33] set [MODE:46:1920x1080]
[drm:drm_vblank_on] crtc 0, vblank enabled 0, inmodeset 1
[drm:drm_calc_timestamping_constants] crtc 24: hwmode: htotal 2640, vtotal 1125, vdisplay 1080
[drm:drm_calc_timestamping_constants] crtc 24: clock 148500 kHz framedur 20000000 linedur 17777
[drm:drm_mode_object_reference] OBJ ID: 47 (3)
[drm:drm_mode_object_unreference] OBJ ID: 47 (4)
[drm:drm_mode_object_unreference] OBJ ID: 48 (2)
[drm:drm_mode_object_unreference] OBJ ID: 34 (4)
[drm:drm_ioctl] ret = -22

With a bit more debugging, this is what's failing:

ipu_plane_atomic_check:442: fail
[drm:drm_crtc_helper_set_config] *ERROR* failed to set mode on [CRTC:24:crtc-0]

        if (old_fb && (state->src_w != old_state->src_w ||
                              state->src_h != old_state->src_h ||
                              fb->pixel_format != old_fb->pixel_format)) {
                printk("%s:%d: fail\n", __func__, __LINE__);
                return -EINVAL;
        }

This test is stupid as it stands - it means we can _never_ change the
format or size of _any_ plane, something that the old code allowed:

-       if (ipu_plane->enabled) {
-               if (src_w != ipu_plane->w || src_h != ipu_plane->h ||
-                   fb->pixel_format != ipu_plane->base.fb->pixel_format)
-                       return -EINVAL;

This used to work because we'd call ipu_crtc_prepare()->ipu_fb_disable()
before the mode set, which would clear ipu_plane->enabled, causing this
test to be skipped.  However, in the new code, we merely test for the
presence of the previous framebuffer, which is really not the same thing
at all.

The old functionality needs to be restored because significantly
changing the displayed mode is something which must be supported with
HDMI.

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel