On Mon, Dec 19, 2016 at 11:15:40PM +0000, Pandiyan, Dhinakaran wrote: > On Sun, 2016-12-18 at 14:43 +0100, Daniel Vetter wrote: > > On Sat, Dec 17, 2016 at 05:47:56AM +0000, Pandiyan, Dhinakaran wrote: > > > On Fri, 2016-12-16 at 16:47 +0200, Jani Nikula wrote: > > > > On Fri, 16 Dec 2016, Daniel Vetter <daniel@xxxxxxxx> wrote: > > > > > On Fri, Dec 16, 2016 at 12:29:05PM +0200, Jani Nikula wrote: > > > > >> The two remaining patches from [1], rebased. > > > > >> > > > > >> BR, > > > > >> Jani. > > > > >> > > > > >> > > > > >> [1] http://mid.mail-archive.com/1480984058-552-1-git-send-email-manasi.d.navare@xxxxxxxxx > > > > > > > > > > Just for the record, I think the only thing missing here is the Xorg > > > > > review on the -modesetting patch. As soon as we have that I can vacuum > > > > > this up (probably best through drm-misc, but not sure). > > > > > > > > Yeah I rebased this (and provided a debug hack privately) so Martin can > > > > test the modesetting changes. > > > > > > > > BR, > > > > Jani. > > > > > > > > > > > > > > I tested the -modesetting patch, which Martin had provided to Manasi, > > > with a compliance testing device (DPR-120) that can simulate link > > > training failure. The link rate correctly lowered after the link_status > > > property was set to BAD by the kernel and the userspace responded with a > > > modeset. > > > > > > One thing that was not straight forward to figure out was I had to boot > > > with i915.nuclear_pageflip=1. Is it documented somewhere that the > > > property needs DRIVER_ATOMIC to be set, or is it implicit? > > > > It should work without DRIVER_ATOMIC. At least the property should be > > exposed ... If this does only work with DRIVER_ATOMIC set then we have a > > bug somewhere. Can you pls try to figure out why it doesn't work? > > > > The property is exposed even without DRIVER_ATOMIC set, but the value is > always GOOD (0). > We set connector->state->link_status to BAD when link training fails but > the getconnector() ioctl ends up reading obj->properties->values[i] if > DRIVER_ATOMIC is NOT set. But with DRIVER_ATOMIC set, getconnector() > calls into drm_atomic_connector_get_property() and retrieves the value > stored in connector->state->link_status. That sounds like a bug in the getconnector code. This needs the same treatment as other places, see e.g. commit d3a46183db97536a53df5de6b556bd61197842b2 Author: Daniel Vetter <daniel.vetter@xxxxxxxx> Date: Wed Jun 8 14:19:15 2016 +0200 drm: Replace fb_helper->atomic with mode_config->atomic_commit I think it'd be good to extract this check into a drm_drv_uses_atomic_modeset to better self-document the code, roll it out to all existing places that check for atomic_commit and then also roll it out to the getproperty functions (for connectors, planes and crtcs). > > > The other thing I had trouble with -modesetting was, there was no > > > modeset following a long pulse from the sink at the begging of the test. > > > I had to force a modeset by changing the resolution so that the link > > > training path is executed. However, the link training failure induced a > > > modeset without any intervention. > > > > Sounds roughly like how it's supposed to work. For real mode configuration > > changes the desktop environment is supposed to set the mode it wants, by > > listening to the xrandr hotplug event. That's not the same as the udev > > hotplug event. You can listen for the xrandr hotplug event using > > > > $ xev -event randr > > > > Got it, -modesetting does indeed send out the hotplug events upon > hotplug. Excellent, so at least that's all working well. -Daniel > > -DK > > > > Cheers, Daniel > > > > > > > > -DK > > > > > > > > > > > -Daniel > > > > > > > > > >> > > > > >> > > > > >> Manasi Navare (2): > > > > >> drm: Add a new connector atomic property for link status > > > > >> drm/i915: Implement Link Rate fallback on Link training failure > > > > >> > > > > >> drivers/gpu/drm/drm_atomic.c | 16 +++++++++ > > > > >> drivers/gpu/drm/drm_atomic_helper.c | 15 ++++++++ > > > > >> drivers/gpu/drm/drm_connector.c | 52 +++++++++++++++++++++++++++ > > > > >> drivers/gpu/drm/i915/intel_dp.c | 27 ++++++++++++++ > > > > >> drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++++++++++-- > > > > >> drivers/gpu/drm/i915/intel_drv.h | 3 ++ > > > > >> include/drm/drm_connector.h | 19 ++++++++++ > > > > >> include/drm/drm_mode_config.h | 5 +++ > > > > >> include/uapi/drm/drm_mode.h | 4 +++ > > > > >> 9 files changed, 161 insertions(+), 2 deletions(-) > > > > >> > > > > >> -- > > > > >> 2.1.4 > > > > >> > > > > >> _______________________________________________ > > > > >> dri-devel mailing list > > > > >> dri-devel@xxxxxxxxxxxxxxxxxxxxx > > > > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > > > > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
