Op 07-07-17 om 13:01 schreef Daniel Vetter:
> On Thu, Jul 06, 2017 at 03:03:15PM +0200, Maarten Lankhorst wrote:
>> Op 06-07-17 om 13:09 schreef Tomeu Vizoso:
>>> Looks good to me:
>>>
>>> Reviewed-by: Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx>
>>>
>>> I guess you have tested this with IGT? In any case, I think it would
>>> be good to mention how a patch has been tested in the changelog. That
>>> can be very useful to others if things go wrong at some point.
>> Testcase: debugfs_test.read_all_entries
>>
>> But I hit it by doing a recursive grep, which I guess is the same thing here. :)
>>
>> One further improvement I wanted to do was reject opening the CRC with -EIO
>> when the crtc is not active, that way the above test will not hang.
>> Does the below patch also look good to you?
>>
>> ----8<-----
>> Commit e8fa5671183c ("drm: crc: Wait for a frame before returning
>> from open()") adds a wait for CRC frame, but with the CRTC off
>> this will never be generated. For atomic drivers we know if a CRTC
>> is active through crtc_state->active, so when inactive reject the
>> open with -EIO.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> Fixes: e8fa5671183c ("drm: crc: Wait for a frame before returning from open()")
>> Testcase: debugfs_test.read_all_entries
> At least for the semantics I think this makes sense. Opening the CRC file
> when the crtc is off is undefined.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
>
> But pls get Tomeu's ack too.
Tomeu, can you ack? :)
I did some testing on IGT with both patches applied on all tests with
CRC in their name, no problems with opening CRC as far as I can see,
and all tests except kms_ccs and kms_mmap_write_crc succeed. The
former needs render compression, the latter fails on a crc failure,
so it can't have been caused by this patch.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
