Re: [PATCH 10/10] Revert "drm: crc: Wait for a frame before returning from open()"

[Tomeu Vizoso <tomeu.vizoso@collabora.com>]

On 06/22/2018 02:30 PM, Maarten Lankhorst wrote:
> Op 22-06-18 om 12:41 schreef Mahesh Kumar:
>> This reverts commit e8fa5671183c80342d520ad81d14fa79a9d4a680.
>>
>> Don't wait for first CRC during crtc_crc_open. It avoids one frame wait
>> during open. If application want to wait after read call, it can use
>> poll/read blocking read() call.

Hi,

unfortunately I don't remember why that was deemed undesirable, and I 
failed to explain so in the commit message.

My only comment is that it would be good to make the motivation for this 
revert explicit, in case someone else in the future wonders.

I would also test it in a machine with eDP, IGT should be able to test there.

Thanks,

Tomeu

>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/drm_debugfs_crc.c | 16 ----------------
>>   1 file changed, 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
>> index 834bc7ee5550..69ec2728727e 100644
>> --- a/drivers/gpu/drm/drm_debugfs_crc.c
>> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
>> @@ -211,24 +211,8 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
>>   	if (ret)
>>   		goto err;
>>   
>> -	spin_lock_irq(&crc->lock);
>> -	/*
>> -	 * Only return once we got a first frame, so userspace doesn't have to
>> -	 * guess when this particular piece of HW will be ready to start
>> -	 * generating CRCs.
>> -	 */
>> -	ret = wait_event_interruptible_lock_irq(crc->wq,
>> -						crtc_crc_data_count(crc),
>> -						crc->lock);
>> -	spin_unlock_irq(&crc->lock);
>> -
>> -	if (ret)
>> -		goto err_disable;
>> -
>>   	return 0;
>>   
>> -err_disable:
>> -	crtc->funcs->set_crc_source(crtc, NULL);
>>   err:
>>   	spin_lock_irq(&crc->lock);
>>   	crtc_crc_cleanup(crc);
> 
> Adding Tomeu Vizoso to the cc.
> 
> Can you resend the series, and add dri-devel@lists.freedesktop.org and the driver maintainers to the cc? You'll need to get acks from the maintainers to merge this through drm-misc. :)
> 
> ~Maarten
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx