Re: [igt-dev] [PATCH i-g-t 5/5] lib/kms: warn if we commit without outputs

Maarten Lankhorst
2018-11-20T11:09:15Z

Op 02-11-18 om 13:11 schreef Maarten Lankhorst:
> Op 02-11-18 om 10:57 schreef Daniel Vetter:
>> With the high-level helpers requiring outputs there's not point
>> in silently ignoring issues anymore. Complain about that if it
>> ever happens.
>>
>> This reverts
>>
>> commit 212b71372bfbb73663d872df31118d6b396ada4f
>> Author: Chris Wilson <chris@chris-wilson.co.uk>
>> Date: Fri Sep 14 21:03:38 2018 +0100
>>
>> lib/kms: Skip no-op display updates
>>
>> which created an in my opinion serious API issue by silently dropping
>> possible errors on the floor. Instead of silently second guess what
>> the test might have wanted to do in the absence of display outputs
>> it's much better to be explicit, and enforce that.
>>
>> v2: Improve commit message.
>>
>> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>> lib/igt_kms.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 7214101e2696..5a35f6bbc177 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -3292,7 +3292,7 @@ static int do_display_commit(igt_display_t *display,
>> enum pipe pipe;
>> LOG_INDENT(display, "commit");
>>
>> - if (!display->n_pipes || !display->n_outputs)
>> + if (igt_warn_on(!display->n_pipes || !display->n_outputs))
>> return 0; /* nothing to do */
>>
>> igt_display_refresh(display);
>> @@ -3345,7 +3345,7 @@ int igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *
>> {
>> int ret;
>>
>> - if (!display->n_pipes || !display->n_outputs)
>> + if (igt_warn_on(!display->n_pipes || !display->n_outputs))
>> return 0; /* nothing to do */
>>
>> LOG_INDENT(display, "commit");
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
For the whole series. :)