Re: [RFC 4/7] drm/i915: Remove mkwrite_device_info

From: Jani Nikula <jani.nikula@intel.com>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	Intel-gfx@lists.freedesktop.org
Subject: Re: [RFC 4/7] drm/i915: Remove mkwrite_device_info
Date: Tue, 13 Nov 2018 13:28:11 +0200	[thread overview]
Message-ID: <87k1lhgqt0.fsf@intel.com> (raw)
In-Reply-To: <3f00e395-c753-4164-7b11-471370dc7038@linux.intel.com>

On Tue, 13 Nov 2018, Tvrtko Ursulin <tvrtko.ursulin@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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx