Re: [PATCH 2/4] drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/4] drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip
Date: Fri, 12 Jun 2015 12:31:37 +0200	[thread overview]
Message-ID: <557AB509.9080202@linux.intel.com> (raw)
In-Reply-To: <20150612101600.GT5176@intel.com>

Op 12-06-15 om 12:16 schreef Ville Syrjälä:
> On Fri, Jun 12, 2015 at 11:15:40AM +0200, Maarten Lankhorst wrote:
>> Use a full atomic call instead. intel_crtc_page_flip will still
>> have to live until async updates are allowed.
>>
>> This doesn't seem to be a regression from the convert to atomic,
>> part 3 patch. During GPU reset it fixes the following warning:
>>
>>  ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 752 at drivers/gpu/drm/drm_crtc.c:5337 drm_mode_page_flip_ioctl+0x27b/0x360()
>> Modules linked in: i915
>> CPU: 0 PID: 752 Comm: Xorg Not tainted 4.1.0-rc7-patser+ #4090
>> Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015
>>  ffffffff81c90866 ffff8800d87c3ca8 ffffffff817f7d87 0000000080000001
>>  0000000000000000 ffff8800d87c3ce8 ffffffff81084955 ffff880000000000
>>  ffff8800d87c3dc0 ffff8800d93d1208 0000000000000000 ffff8800b7d1f3e0
>> Call Trace:
>>  [<ffffffff817f7d87>] dump_stack+0x4f/0x7b
>>  [<ffffffff81084955>] warn_slowpath_common+0x85/0xc0
>>  [<ffffffff81084a35>] warn_slowpath_null+0x15/0x20
>>  [<ffffffff8146dffb>] drm_mode_page_flip_ioctl+0x27b/0x360
>>  [<ffffffff8145ccb0>] drm_ioctl+0x1a0/0x6a0
>>  [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50
>>  [<ffffffff812e5540>] ? avc_has_perm+0x20/0x280
>>  [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50
>>  [<ffffffff811ea0f8>] do_vfs_ioctl+0x2f8/0x530
>>  [<ffffffff811f6001>] ? expand_files+0x261/0x270
>>  [<ffffffff812e7c16>] ? selinux_file_ioctl+0x56/0x100
>>  [<ffffffff811ea3b1>] SyS_ioctl+0x81/0xa0
>>  [<ffffffff81801b97>] system_call_fastpath+0x12/0x6f
>> ---[ end trace 9ce834560085bd64 ]---
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++++++++++++++++-
>>  1 file changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 7abaffeda7ce..cdf6549c8e74 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11618,8 +11618,35 @@ free_work:
>>  	kfree(work);
>>  
>>  	if (ret == -EIO) {
>> +		struct drm_atomic_state *state;
>> +		struct drm_plane_state *plane_state;
>> +
>>  out_hang:
>> -		ret = intel_plane_restore(primary);
> So what exactly is wrong with intel_plane_restore() (ie.
> drm_plane_helper_update())? Shouldn't you fix that instead of spreading
> the uglyness here?
>
intel_plane_restore uses the transitional helpers. This is currently used for
setting color key, enabling sprite planes after a modeset and this function.
- Color key will be made atomic by making ckey part of the plane state.
- Sprite plane updates after modeset will be made unnecessary by calling
  drm_atomic_helper_commit_planes_on_crtc after .crtc_enable.
- This function needs to update plane->fb, and intel_plane_restore doesn't provide that.

That leaves only this function calling intel_plane_restore, if we can get rid of it
there will be no need of transitional helper calls any more.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx