Re: [PATCH 3/5] drm/atomic: Move drm_crtc_commit to drm_crtc_state, v2.

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 30-08-17 om 15:09 schreef Laurent Pinchart:
> Hi Maarten,
>
> Thank you for the patch.
>
> On Wednesday, 30 August 2017 15:17:50 EEST Maarten Lankhorst wrote:
>> Most code only cares about the current commit or previous commit.
>> Fortuantely we already have a place to track those. Move it to
>> drm_crtc_state where it belongs. :)
>>
>> The per-crtc commit_list is kept for places where we have to look
>> deeper than the current or previous commit for checking whether to stall
>> on unpin. This is used in drm_atomic_helper_setup_commit and
>> intel_has_pending_fb_unpin.
>>
>> Changes since v1:
>> - Update kerneldoc for drm_crtc.commit_list. (danvet)
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>  drivers/gpu/drm/drm_atomic.c        |  7 ---
>>  drivers/gpu/drm/drm_atomic_helper.c | 92 +++++++++-------------------------
>>  include/drm/drm_atomic.h            |  1 -
>>  include/drm/drm_crtc.h              | 23 ++++++++--
>>  4 files changed, 42 insertions(+), 81 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 2fd383d7253a..2cce48f203e0 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -163,13 +163,6 @@ void drm_atomic_state_default_clear(struct
>> drm_atomic_state *state) crtc->funcs->atomic_destroy_state(crtc,
>>  						  state->crtcs[i].state);
>>
>> -		if (state->crtcs[i].commit) {
>> -			kfree(state->crtcs[i].commit->event);
>> -			state->crtcs[i].commit->event = NULL;
>> -			drm_crtc_commit_put(state->crtcs[i].commit);
>> -		}
>> -
>> -		state->crtcs[i].commit = NULL;
>>  		state->crtcs[i].ptr = NULL;
>>  		state->crtcs[i].state = NULL;
>>  	}
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> b/drivers/gpu/drm/drm_atomic_helper.c index 11d0e94a2181..8ccb8b6536c0
>> 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1262,12 +1262,12 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
>>  void drm_atomic_helper_wait_for_flip_done(struct drm_device *dev,
>>  					  struct drm_atomic_state *old_state)
>>  {
>> -	struct drm_crtc_state *unused;
>> +	struct drm_crtc_state *new_crtc_state;
>>  	struct drm_crtc *crtc;
>>  	int i;
>>
>> -	for_each_new_crtc_in_state(old_state, crtc, unused, i) {
>> -		struct drm_crtc_commit *commit = old_state->crtcs[i].commit;
>> +	for_each_new_crtc_in_state(old_state, crtc, new_crtc_state, i) {
>> +		struct drm_crtc_commit *commit = new_crtc_state->commit;
>>  		int ret;
>>
>>  		if (!commit)
>> @@ -1388,11 +1388,10 @@ int drm_atomic_helper_async_check(struct drm_device
>> *dev, {
>>  	struct drm_crtc *crtc;
>>  	struct drm_crtc_state *crtc_state;
>> -	struct drm_crtc_commit *commit;
>>  	struct drm_plane *__plane, *plane = NULL;
>>  	struct drm_plane_state *__plane_state, *plane_state = NULL;
>>  	const struct drm_plane_helper_funcs *funcs;
>> -	int i, j, n_planes = 0;
>> +	int i, n_planes = 0;
>>
>>  	for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
>>  		if (drm_atomic_crtc_needs_modeset(crtc_state))
>> @@ -1420,33 +1419,10 @@ int drm_atomic_helper_async_check(struct drm_device
>> *dev, return -EINVAL;
>>
>>  	/*
>> -	 * Don't do an async update if there is an outstanding commit modifying
>> +	 * TODO: Don't do an async update if there is an outstanding commit
>> modifying
>> 	 * the plane.  This prevents our async update's changes from getting
>> 	 * overridden by a previous synchronous update's state.
>>  	 */
> As mentioned in a comment to your previous patch, this is unrelated to 
> $SUBJECT. You should mention and explain this change in the commit message 
> (possibly splitting it out in a separate commit if you think that would make 
> more sense, up to you).
I'm all for removing this, it doesn't work.. But I g uess it can be kept in for now and I'll drop this hunk.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel