[Intel-gfx] [PATCH 1/3] drm/atomic: move waiting for hw_done to a helper

[Maarten Lankhorst]

Op 26-01-17 om 19:29 schreef Ville Syrjälä:
> On Mon, Jan 09, 2017 at 06:06:56PM +0100, Maarten Lankhorst wrote:
>> This will allow i915 to perform a wait on pending modeset using the
>> same code.
> I don't like commit messages that refer to the subject like this. I
> can't even see the subject when I'm replying so I have approximately
> zero idea what's being said here.
>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> ---
>>  drivers/gpu/drm/drm_atomic_helper.c | 51 +++++++++++++++++++++++++++----------
>>  include/drm/drm_atomic_helper.h     |  1 +
>>  2 files changed, 38 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 7b71ac48b8a4..3a8a46510c36 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1936,6 +1936,41 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>>  EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
>>  
>>  /**
>> + * drm_atomic_helper_wait_for_hw_done - wait for all previous hw updates to complete.
>> + * @crtc: crtc to wait on.
>> + *
>> + * This function waits for all previous hardware updates queued on @crtc
>> + * to complete.
>> + *
>> + * Returns:
>> + * 0 on success, negative error code on failure.
>> + */
>> +int drm_atomic_helper_wait_for_hw_done(struct drm_crtc *crtc)
>> +{
>> +	struct drm_crtc_commit *commit;
>> +	long ret;
>> +
>> +	spin_lock(&crtc->commit_lock);
>> +	commit = list_first_entry_or_null(&crtc->commit_list,
>> +			struct drm_crtc_commit, commit_entry);
>> +	if (commit)
>> +		drm_crtc_commit_get(commit);
>> +	spin_unlock(&crtc->commit_lock);
>> +
>> +	if (!commit)
>> +		return 0;
>> +
>> +	ret = wait_for_completion_timeout(&commit->hw_done, 10 * HZ);
>> +	drm_crtc_commit_put(commit);
>> +
>> +	if (ret == 0)
>> +		return -EBUSY;
>> +
>> +	return ret > 0 ? 0 : ret;
>> +}
>> +EXPORT_SYMBOL(drm_atomic_helper_wait_for_hw_done);
> Seems sane enough. I was pondering if we could share the code with the
> other hw_done wait as well. But it grabs a different commit from the
> list, so not sure if we'd end up with less code or not.
>
> Hmm. maybe pass the commit index from the caller all the way from the
> caller to preceeding_commit() (which would obviously need a rename)?

I think this helper is useful for waiting for crtc idle, I'm not sure it's useful in
general to wait for other commits, since we probably want to wait for all previous
commits to be completed.

~Maarten