On ke, 2016-02-24 at 07:42 +0000, Tian, Kevin wrote: > > From: Wang, Zhi A > > Sent: Tuesday, February 23, 2016 9:23 PM > > > > --- a/drivers/gpu/drm/i915/gvt/gvt.c > > > > +++ b/drivers/gpu/drm/i915/gvt/gvt.c > > > > @@ -348,6 +348,10 @@ void *gvt_create_pgt_device(struct drm_i915_private > > *dev_priv) > > > > goto err; > > > > } > > > > > > > > + dev_priv->gvt.host_fence_sz = gvt.host_fence_sz; > > > > + dev_priv->gvt.host_low_gm_sz_in_mb = gvt.host_low_gm_sz; > > > > + dev_priv->gvt.host_high_gm_sz_in_mb = gvt.host_high_gm_sz; > > > > > > I'm thinking, could we expose the pgt_device struct (at least > > > partially, and then have a PIMPL pattern), to avoid this kind of > > > duplication of declarations and unnecessary copies between i915 and > > > i915_gvt modules? > > > > > > It's little rough that the gvt driver writes to i915_private struct. > > > I'd rather see that gvt.host_fence_sz and other variables get sanitized > > > and then written to pgt_device (maybe the public part would be > > > i915_pgt_device) and used by gvt and i915 code. > > > > > > Was this ever considered? > > > > > The previous version do something similar like that, both i915 and gvt > > reads GVT module kernel parameter but considered that GVT modules could > > be configured as "n" in kernel configuration, probably we should let > > i915 to store this information and GVT to configure it if GVT is active? > > Agree with Joonas. We don't need another gvt wrap. Let's just expose > pgt_device directly. I believe all other information can be encapsulated > under pgt_device. > > btw to match other description in the code, is it clear to rename pgt_device > to gvt_device? > > Another minor thing needs Joonas' feedback. Is it usual to capture > all module parameters belonging to one feature structurized together > (like 'gvt' in this patch), or just to leave them directly exposed? > I think it's a good idea to group them as they're currently grouped. Regards, Joonas > Thanks > Kevin -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
