[Intel-gfx] [PATCH] drm/i915: Rename global i915 to i915_modparams

[Joonas Lahtinen]

On Tue, 2017-09-19 at 13:22 +0300, Ville Syrjälä wrote:
> On Tue, Sep 19, 2017 at 11:24:41AM +0300, Joonas Lahtinen wrote:
> > On Mon, 2017-09-18 at 22:07 +0200, Michal Wajdeczko wrote:
> > > On Mon, 18 Sep 2017 21:11:40 +0200, Jani Nikula <jani.nikula at intel.com>  
> > > wrote:
> > > 
> > > > On Mon, 18 Sep 2017, Michal Wajdeczko <michal.wajdeczko at intel.com> wrote:
> > > > > Our global struct with params is named exactly the same way
> > > > > as new preferred name for the drm_i915_private function parameter.
> > > > > To avoid such name reuse lets use different name for the global.
> > > > > 
> > > > > v4: introduction of mkwrite()
> > > > 
> > > > Why?
> > > > 
> > > > I don't know what you're trying to achieve with the mkwrite() stuff (the
> > > 
> > > I was trying to buy at least one more vote, as discussed on IRC
> > > 
> > > <quote>
> > > [14:23:36] <dolphin> I'll be glad to vote for i915_modparams +  
> > > i915_modparams_mkwrite()
> > > <quote/>
> > > 
> > > > commit message would be the perfect place to explain that) but no matter
> > > > what it should IMO be a separate patch.
> > > > 
> > > > I think the simple s/i915/i915_modparams/ would be fine, and we could
> > > > move on.
> > > 
> > > Note that it all started with this idea.
> > > See https://patchwork.freedesktop.org/patch/176409/
> > > 
> > 
> > I agree with Jani that the pure rename should be its own patch. That'll
> > make review much easier. Then have a follow-up that introduces
> > _mkwrite() and as a bonus makes the struct const or at least makes
> > sparse complain.
> 
> I know we abuse the const+mkwrite type of thing for the device info, but
> I'm not sure how safe that actually is on account of the compiler being
> free to assume that const stuff doesn't generally change. I guess if the
> mkwrite thing happens at some early controlled point it's going to be OK,
> but if it starts happening at some randomish times we might not be so
> lucky.

I see this more as a reason to introduce it. We can't simply change
enable_ppgtt on the run when there are potential readers contending for
the variable value, that _mkwrite() would just highlight the issue. So
any write to the variables should be really well thought out. An easier
option would be to get rid of as many of them as possible.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation