On Mon, 01 Aug 2016, Dave Gordon <david.s.gordon@xxxxxxxxx> wrote:
> On 01/08/16 14:54, Jani Nikula wrote:
>> On Fri, 22 Jul 2016, Dave Gordon <david.s.gordon@xxxxxxxxx> wrote:
>>> - } else if (i915.enable_guc_submission > 1) {
>>> + } else if (i915.enable_guc_submission >= GUC_SUBMISSION_MANDATORY) {
>>
>> I like the patches in general, but now these >= and <= seem rather out
>> of place. How about using == and != exclusively?
>>
>> BR,
>> Jani.
>
> That would leave us with undefined behaviour for values outside the
> recognised range. This way it clips out-of-range values to the nearest
> extremum. Of course we could make it fail completely for invalid values,
> but that's just really annoying for the developer or admin who's
> mistyped -1 as -2 or forgotten what the maximum supported value is in
> this release. Alternatively we could convert all out-of-range values to
> "system default" i.e. ignored, which might still be annoying but not
> quite as much.
I'm not a huge fan of making assumptions about what the user possibly
meant when giving incorrect input, "as a convenience". It teaches the
user to be sloppy about it, and might lead to super annoying surprises
when we actually start using those values for something else.
> Any other suggestions for how to handle out-of-range values?
>
> But if we were changing the policy shouldn't that be a separate patch?
> This patch is supposed to change only the way the code is written, with
> no effect to existing behaviour!
Oh, completely agreed here, while I didn't spell this out in my first
reply. This shouldn't block the patch.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
