[Intel-gfx] [RFC 4/7] drm/i915: Remove mkwrite_device_info

Jani Nikula jani.nikula at intel.com
Tue Nov 13 11:28:11 UTC 2018


On Tue, 13 Nov 2018, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> wrote:
> On 12/11/2018 17:25, Chris Wilson wrote:
>> Quoting Tvrtko Ursulin (2018-11-12 17:12:39)
>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>
>>> Now that we are down to one caller, which does not even modify copied
>>> device info, we can remove the mkwrite_device_info helper and convert the
>>> device info pointer itself to be a pointer to static table instead of a
>>> copy.
>> 
>> The copy was deliberate to avoid the extra pointer. How does the change
>> in code size now compare?
>
> AFAIR grows a bit, but overall series still ends up overall smaller. I 
> need to re-run the numbers for more concrete info.
>
> However, if we keep having a copy, ie. do not make device info properly 
> read-only, are we still interested in splitting the two? Benefit would 
> be diminished, but presumably people still think it would be worth it?

If we don't turn device info into a pointer to the static const structs,
IMO there's simply no point in any of this. Then we might just as well
drop the const and mkwrite_device_info, and use the mutable thing all
over the place.

Modifying the device info copy was supposed to be an exception at the
time of the copy, but with mkwrite_device_info it has proliferated. It's
silly and ugly and wrong with no benefits. The const was put in place to
hint that it's not supposed to be modified, but that didn't work out.

So which is it, runtime/static split with an extra pointer chase to the
const data in rodata (I'm in favor), or just drop the const from the
dev_priv copy and remove mkwrite_device_info?

The latter makes future attempts at utilizing DCE/LTO with a reduced
number of device infos much harder I think.


BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center


More information about the Intel-gfx mailing list