On 2018.07.19 17:20:15 +0000, De Marchi, Lucas wrote: > CC'ing gvt maintainers (and fixing Jani's address in CC). > > See below > > On Wed, 2018-07-18 at 13:01 +0300, Ville Syrjälä wrote: > > On Tue, Jul 17, 2018 at 03:16:53PM -0700, Lucas De Marchi wrote: > > > On Fri, Jul 13, 2018 at 9:10 AM Ville Syrjälä > > > <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > > > > > > > On Fri, Jul 13, 2018 at 08:42:11AM -0700, Lucas De Marchi wrote: > > > > > Instead of defining all registers twice, define just a PCH_GPIO_BASE > > > > > that has the same address as PCH_GPIO_A and use that to calculate all > > > > > the others. This also brings VLV and !HAS_GMCH_DISPLAY in line, doing > > > > > the same thing. > > > > > > > > > > v2: Fix GMBUS registers to be relative to gpio base; create GPIO() > > > > > macro to return a particular gpio address and move the enum out of > > > > > i915_reg.h (suggested by Jani) > > > > > > > > > > v3: Move base offset inside the GPIO() macro so the GMBUS defines > > > > > don't > > > > > actually need to be changed (suggested by Daniel/Ville) > > > > > > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> > > > > > --- > > > > > drivers/gpu/drm/i915/gvt/handlers.c | 2 +- > > > > > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > > > > > drivers/gpu/drm/i915/i915_reg.h | 24 +++++------------------- > > > > > drivers/gpu/drm/i915/intel_drv.h | 16 ++++++++++++++++ > > > > > drivers/gpu/drm/i915/intel_i2c.c | 12 ++++-------- > > > > > 5 files changed, 28 insertions(+), 29 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c > > > > > b/drivers/gpu/drm/i915/gvt/handlers.c > > > > > index 7a58ca555197..ecff866bbbf1 100644 > > > > > --- a/drivers/gpu/drm/i915/gvt/handlers.c > > > > > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > > > > > @@ -2118,7 +2118,7 @@ static int init_generic_mmio_info(struct > > > > > intel_gvt *gvt) > > > > > > > > > > MMIO_F(PCH_GMBUS0, 4 * 4, 0, 0, 0, D_ALL, gmbus_mmio_read, > > > > > gmbus_mmio_write); > > > > > - MMIO_F(PCH_GPIOA, 6 * 4, F_UNALIGN, 0, 0, D_ALL, NULL, NULL); > > > > > + MMIO_F(GPIO(GPIOA), 6 * 4, F_UNALIGN, 0, 0, D_ALL, NULL, NULL); > > > > > > > > I have no idea of gpio_mmio_base is populated correctly at this point > > > > for gvt, and I'm too lazy to find out. > > > > > > humn, unfortunately it is not > > > > > > i915_driver_load() -> i915_load_modeset_init() -> intel_setup_gmbus() > > > -> dev_priv->gpio_mmio_base = ... > > > i915_driver_load() -> i915_driver_init_hw() -> intel_gvt_init() -> > > > intel_gvt_init_device() -> intel_gvt_setup_mmio_info() -> > > > init_generic_mmio_info() > > > > > > Is adding a single PCH_GPIO_BASE that doesn't depend on dev_priv being > > > populated for use on gvt an option? > > > > IIRC gvt already has some local register defines (possibly due to this > > same reason?). Could add a few more I suppose. +cc the gvt folks to get > > their input. > > Summary to gvt maintainers: I want to get rid of the multiple GPIO vs PCH_GPIO > defines we have today. For that I created the GPIO() macro, but it depends on > dev_priv being populated with the gpio's block offset already which is not > true while initializing gvt. > This also applys to GMBUS change, right? > I can define a PCH_GPIO_BASE as above in i915_reg.h that is the same as > PCH_GPIOA today. Or define it in a private gvt header. Do you have another > option/suggestion? > yeah, adding a base define for gvt private seems good to me, as gvt handles MMIO trap so may need to use them in different way. thanks -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
