Re: [PATCH v3 1/4] drm/i915/guc: symbolic names for GuC submission preferences

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 01 Aug 2016, Dave Gordon <david.s.gordon@intel.com> wrote:
> On 01/08/16 14:54, Jani Nikula wrote:
>> On Fri, 22 Jul 2016, Dave Gordon <david.s.gordon@intel.com> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx