Re: [PATCH 05/29] drm/i915: Store the default sseu setup on the engine

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 08/04/2019 16:57, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-04-08 15:54:01)
>>
>> On 08/04/2019 10:17, Chris Wilson wrote:
>>> As we push for better compartmentalisation, it is more convenient to
>>> copy the default sseu configuration from the engine into the derived
>>> logical context, than it is to dig it out from i915->runtime_info.
>>
>> Why it is more convenient?
> 
> Read on in the series.
> 
>>> +static inline struct intel_sseu
>>> +intel_device_default_sseu(const struct sseu_dev_info *sseu)
>>> +{
>>
>> I think function parameter change is dangerous. If caller can pass in
>> any pointer then it is slightly less "intel_*device_default*_sseu. It is
>> more a convert and copy from device sseu to intel_sseu. Like
>> intel_convert_device_sseu.
>>
>> Although if the function just kept taking i915 then the dilemma would
>> not even be there.
> 
> What i915? How about if it took the parameter that described the device
> and converted it into the internal format?

Don't know what i915 - function name says "default". If the caller 
passes in the pointer it is not a default but could be anything. So you 
can go with my first alternative, which is to give function a better 
name if you want to stick with the parameter change.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx