| Message ID | 1458738023-31292-1-git-send-email-joonas.lahtinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> writes: > [ text/plain ] > Rename and document the GGTT init functions to give a better > idea of the context where they are called from. > > i915_gem_gtt_init => i915_init_ggtt_hw > i915_gem_init_global_gtt => i915_gem_init_ggtt > i915_global_gtt_cleanup => i915_cleanup_ggtt_hw > Me likes. The cognitive burden is heavy enough for someone wandering around in this territory. This clears things up. Now if we would have glossary.txt... Thanks, -Mika > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_dma.c | 14 +++++++------- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++++++--- > drivers/gpu/drm/i915/i915_gem_gtt.h | 7 +++---- > 4 files changed, 26 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index fc8ac98..124cefd 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1180,7 +1180,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > > intel_device_info_runtime_init(dev); > > - ret = i915_gem_gtt_init(dev); > + ret = i915_init_ggtt_hw(dev); > if (ret) > return ret; > > @@ -1189,13 +1189,13 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > ret = i915_kick_out_firmware_fb(dev_priv); > if (ret) { > DRM_ERROR("failed to remove conflicting framebuffer drivers\n"); > - goto out_gtt; > + goto out_ggtt; > } > > ret = i915_kick_out_vgacon(dev_priv); > if (ret) { > DRM_ERROR("failed to remove conflicting VGA console\n"); > - goto out_gtt; > + goto out_ggtt; > } > > pci_set_master(dev->pdev); > @@ -1222,7 +1222,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > aperture_size); > if (dev_priv->ggtt.mappable == NULL) { > ret = -EIO; > - goto out_gtt; > + goto out_ggtt; > } > > dev_priv->ggtt.mtrr = arch_phys_wc_add(dev_priv->ggtt.mappable_base, > @@ -1255,8 +1255,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > > return 0; > > -out_gtt: > - i915_global_gtt_cleanup(dev); > +out_ggtt: > + i915_cleanup_ggtt_hw(dev); > > return ret; > } > @@ -1275,7 +1275,7 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv) > pm_qos_remove_request(&dev_priv->pm_qos); > arch_phys_wc_del(dev_priv->ggtt.mtrr); > io_mapping_free(dev_priv->ggtt.mappable); > - i915_global_gtt_cleanup(dev); > + i915_cleanup_ggtt_hw(dev); > } > > /** > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 8588c83..506a706 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4972,7 +4972,7 @@ int i915_gem_init(struct drm_device *dev) > if (ret) > goto out_unlock; > > - i915_gem_init_global_gtt(dev); > + i915_gem_init_ggtt(dev); > > ret = i915_gem_context_init(dev); > if (ret) > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 0715bb7..c23513b 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -2808,7 +2808,11 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev, > return 0; > } > > -void i915_gem_init_global_gtt(struct drm_device *dev) > +/** > + * i915_gem_init_ggtt - Initialize GEM for Global GTT > + * @dev: DRM device > + */ > +void i915_gem_init_ggtt(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > u64 gtt_size, mappable_size; > @@ -2819,7 +2823,11 @@ void i915_gem_init_global_gtt(struct drm_device *dev) > i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size); > } > > -void i915_global_gtt_cleanup(struct drm_device *dev) > +/** > + * i915_cleanup_ggtt_hw - Clean up GGTT hardware initialization > + * @dev: DRM device > + */ > +void i915_cleanup_ggtt_hw(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct i915_address_space *vm = &dev_priv->ggtt.base; > @@ -3157,7 +3165,11 @@ static void i915_gmch_remove(struct i915_address_space *vm) > intel_gmch_remove(); > } > > -int i915_gem_gtt_init(struct drm_device *dev) > +/** > + * i915_init_ggtt_hw - Initialize GGTT hardware > + * @dev: DRM device > + */ > +int i915_init_ggtt_hw(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct i915_ggtt *ggtt = &dev_priv->ggtt; > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > index d804be0..95bf9a0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -513,10 +513,9 @@ i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n) > px_dma(ppgtt->base.scratch_pd); > } > > -int i915_gem_gtt_init(struct drm_device *dev); > -void i915_gem_init_global_gtt(struct drm_device *dev); > -void i915_global_gtt_cleanup(struct drm_device *dev); > - > +int i915_init_ggtt_hw(struct drm_device *dev); > +void i915_gem_init_ggtt(struct drm_device *dev); > +void i915_cleanup_ggtt_hw(struct drm_device *dev); > > int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); > int i915_ppgtt_init_hw(struct drm_device *dev); > -- > 2.5.5
On Wed, Mar 23, 2016 at 03:00:22PM +0200, Joonas Lahtinen wrote: > Rename and document the GGTT init functions to give a better > idea of the context where they are called from. > > i915_gem_gtt_init => i915_init_ggtt_hw Seems to me i915_ggtt_init_hw would match existing practices better. > i915_gem_init_global_gtt => i915_gem_init_ggtt > i915_global_gtt_cleanup => i915_cleanup_ggtt_hw > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_dma.c | 14 +++++++------- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++++++--- > drivers/gpu/drm/i915/i915_gem_gtt.h | 7 +++---- > 4 files changed, 26 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index fc8ac98..124cefd 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1180,7 +1180,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > > intel_device_info_runtime_init(dev); > > - ret = i915_gem_gtt_init(dev); > + ret = i915_init_ggtt_hw(dev); > if (ret) > return ret; > > @@ -1189,13 +1189,13 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > ret = i915_kick_out_firmware_fb(dev_priv); > if (ret) { > DRM_ERROR("failed to remove conflicting framebuffer drivers\n"); > - goto out_gtt; > + goto out_ggtt; > } > > ret = i915_kick_out_vgacon(dev_priv); > if (ret) { > DRM_ERROR("failed to remove conflicting VGA console\n"); > - goto out_gtt; > + goto out_ggtt; > } > > pci_set_master(dev->pdev); > @@ -1222,7 +1222,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > aperture_size); > if (dev_priv->ggtt.mappable == NULL) { > ret = -EIO; > - goto out_gtt; > + goto out_ggtt; > } > > dev_priv->ggtt.mtrr = arch_phys_wc_add(dev_priv->ggtt.mappable_base, > @@ -1255,8 +1255,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > > return 0; > > -out_gtt: > - i915_global_gtt_cleanup(dev); > +out_ggtt: > + i915_cleanup_ggtt_hw(dev); > > return ret; > } > @@ -1275,7 +1275,7 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv) > pm_qos_remove_request(&dev_priv->pm_qos); > arch_phys_wc_del(dev_priv->ggtt.mtrr); > io_mapping_free(dev_priv->ggtt.mappable); > - i915_global_gtt_cleanup(dev); > + i915_cleanup_ggtt_hw(dev); > } > > /** > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 8588c83..506a706 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4972,7 +4972,7 @@ int i915_gem_init(struct drm_device *dev) > if (ret) > goto out_unlock; > > - i915_gem_init_global_gtt(dev); > + i915_gem_init_ggtt(dev); > > ret = i915_gem_context_init(dev); > if (ret) > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 0715bb7..c23513b 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -2808,7 +2808,11 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev, > return 0; > } > > -void i915_gem_init_global_gtt(struct drm_device *dev) > +/** > + * i915_gem_init_ggtt - Initialize GEM for Global GTT > + * @dev: DRM device > + */ > +void i915_gem_init_ggtt(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > u64 gtt_size, mappable_size; > @@ -2819,7 +2823,11 @@ void i915_gem_init_global_gtt(struct drm_device *dev) > i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size); > } > > -void i915_global_gtt_cleanup(struct drm_device *dev) > +/** > + * i915_cleanup_ggtt_hw - Clean up GGTT hardware initialization > + * @dev: DRM device > + */ > +void i915_cleanup_ggtt_hw(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct i915_address_space *vm = &dev_priv->ggtt.base; > @@ -3157,7 +3165,11 @@ static void i915_gmch_remove(struct i915_address_space *vm) > intel_gmch_remove(); > } > > -int i915_gem_gtt_init(struct drm_device *dev) > +/** > + * i915_init_ggtt_hw - Initialize GGTT hardware > + * @dev: DRM device > + */ > +int i915_init_ggtt_hw(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct i915_ggtt *ggtt = &dev_priv->ggtt; > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > index d804be0..95bf9a0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -513,10 +513,9 @@ i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n) > px_dma(ppgtt->base.scratch_pd); > } > > -int i915_gem_gtt_init(struct drm_device *dev); > -void i915_gem_init_global_gtt(struct drm_device *dev); > -void i915_global_gtt_cleanup(struct drm_device *dev); > - > +int i915_init_ggtt_hw(struct drm_device *dev); > +void i915_gem_init_ggtt(struct drm_device *dev); > +void i915_cleanup_ggtt_hw(struct drm_device *dev); > > int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); > int i915_ppgtt_init_hw(struct drm_device *dev); > -- > 2.5.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä <ville.syrjala@linux.intel.com> writes: > [ text/plain ] > On Wed, Mar 23, 2016 at 03:00:22PM +0200, Joonas Lahtinen wrote: >> Rename and document the GGTT init functions to give a better >> idea of the context where they are called from. >> >> i915_gem_gtt_init => i915_init_ggtt_hw > > Seems to me i915_ggtt_init_hw would match existing practices better. > There is also some gravity towards putting the verb first. In gem side atleast. -Mika >> i915_gem_init_global_gtt => i915_gem_init_ggtt >> i915_global_gtt_cleanup => i915_cleanup_ggtt_hw >> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_dma.c | 14 +++++++------- >> drivers/gpu/drm/i915/i915_gem.c | 2 +- >> drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++++++--- >> drivers/gpu/drm/i915/i915_gem_gtt.h | 7 +++---- >> 4 files changed, 26 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c >> index fc8ac98..124cefd 100644 >> --- a/drivers/gpu/drm/i915/i915_dma.c >> +++ b/drivers/gpu/drm/i915/i915_dma.c >> @@ -1180,7 +1180,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) >> >> intel_device_info_runtime_init(dev); >> >> - ret = i915_gem_gtt_init(dev); >> + ret = i915_init_ggtt_hw(dev); >> if (ret) >> return ret; >> >> @@ -1189,13 +1189,13 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) >> ret = i915_kick_out_firmware_fb(dev_priv); >> if (ret) { >> DRM_ERROR("failed to remove conflicting framebuffer drivers\n"); >> - goto out_gtt; >> + goto out_ggtt; >> } >> >> ret = i915_kick_out_vgacon(dev_priv); >> if (ret) { >> DRM_ERROR("failed to remove conflicting VGA console\n"); >> - goto out_gtt; >> + goto out_ggtt; >> } >> >> pci_set_master(dev->pdev); >> @@ -1222,7 +1222,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) >> aperture_size); >> if (dev_priv->ggtt.mappable == NULL) { >> ret = -EIO; >> - goto out_gtt; >> + goto out_ggtt; >> } >> >> dev_priv->ggtt.mtrr = arch_phys_wc_add(dev_priv->ggtt.mappable_base, >> @@ -1255,8 +1255,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) >> >> return 0; >> >> -out_gtt: >> - i915_global_gtt_cleanup(dev); >> +out_ggtt: >> + i915_cleanup_ggtt_hw(dev); >> >> return ret; >> } >> @@ -1275,7 +1275,7 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv) >> pm_qos_remove_request(&dev_priv->pm_qos); >> arch_phys_wc_del(dev_priv->ggtt.mtrr); >> io_mapping_free(dev_priv->ggtt.mappable); >> - i915_global_gtt_cleanup(dev); >> + i915_cleanup_ggtt_hw(dev); >> } >> >> /** >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index 8588c83..506a706 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -4972,7 +4972,7 @@ int i915_gem_init(struct drm_device *dev) >> if (ret) >> goto out_unlock; >> >> - i915_gem_init_global_gtt(dev); >> + i915_gem_init_ggtt(dev); >> >> ret = i915_gem_context_init(dev); >> if (ret) >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >> index 0715bb7..c23513b 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >> @@ -2808,7 +2808,11 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev, >> return 0; >> } >> >> -void i915_gem_init_global_gtt(struct drm_device *dev) >> +/** >> + * i915_gem_init_ggtt - Initialize GEM for Global GTT >> + * @dev: DRM device >> + */ >> +void i915_gem_init_ggtt(struct drm_device *dev) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> u64 gtt_size, mappable_size; >> @@ -2819,7 +2823,11 @@ void i915_gem_init_global_gtt(struct drm_device *dev) >> i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size); >> } >> >> -void i915_global_gtt_cleanup(struct drm_device *dev) >> +/** >> + * i915_cleanup_ggtt_hw - Clean up GGTT hardware initialization >> + * @dev: DRM device >> + */ >> +void i915_cleanup_ggtt_hw(struct drm_device *dev) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct i915_address_space *vm = &dev_priv->ggtt.base; >> @@ -3157,7 +3165,11 @@ static void i915_gmch_remove(struct i915_address_space *vm) >> intel_gmch_remove(); >> } >> >> -int i915_gem_gtt_init(struct drm_device *dev) >> +/** >> + * i915_init_ggtt_hw - Initialize GGTT hardware >> + * @dev: DRM device >> + */ >> +int i915_init_ggtt_hw(struct drm_device *dev) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct i915_ggtt *ggtt = &dev_priv->ggtt; >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h >> index d804be0..95bf9a0 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h >> @@ -513,10 +513,9 @@ i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n) >> px_dma(ppgtt->base.scratch_pd); >> } >> >> -int i915_gem_gtt_init(struct drm_device *dev); >> -void i915_gem_init_global_gtt(struct drm_device *dev); >> -void i915_global_gtt_cleanup(struct drm_device *dev); >> - >> +int i915_init_ggtt_hw(struct drm_device *dev); >> +void i915_gem_init_ggtt(struct drm_device *dev); >> +void i915_cleanup_ggtt_hw(struct drm_device *dev); >> >> int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); >> int i915_ppgtt_init_hw(struct drm_device *dev); >> -- >> 2.5.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Mar 23, 2016 at 05:54:09PM +0200, Mika Kuoppala wrote: > Ville Syrjälä <ville.syrjala@linux.intel.com> writes: > > > [ text/plain ] > > On Wed, Mar 23, 2016 at 03:00:22PM +0200, Joonas Lahtinen wrote: > >> Rename and document the GGTT init functions to give a better > >> idea of the context where they are called from. > >> > >> i915_gem_gtt_init => i915_init_ggtt_hw > > > > Seems to me i915_ggtt_init_hw would match existing practices better. > > > > There is also some gravity towards putting the verb first. In gem > side atleast. At least in this case ggtt_init_hw would match ppgtt_init_hw, which seems like a nice thing. > > -Mika > > > >> i915_gem_init_global_gtt => i915_gem_init_ggtt > >> i915_global_gtt_cleanup => i915_cleanup_ggtt_hw > >> > >> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > >> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > >> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_dma.c | 14 +++++++------- > >> drivers/gpu/drm/i915/i915_gem.c | 2 +- > >> drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++++++--- > >> drivers/gpu/drm/i915/i915_gem_gtt.h | 7 +++---- > >> 4 files changed, 26 insertions(+), 15 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > >> index fc8ac98..124cefd 100644 > >> --- a/drivers/gpu/drm/i915/i915_dma.c > >> +++ b/drivers/gpu/drm/i915/i915_dma.c > >> @@ -1180,7 +1180,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > >> > >> intel_device_info_runtime_init(dev); > >> > >> - ret = i915_gem_gtt_init(dev); > >> + ret = i915_init_ggtt_hw(dev); > >> if (ret) > >> return ret; > >> > >> @@ -1189,13 +1189,13 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > >> ret = i915_kick_out_firmware_fb(dev_priv); > >> if (ret) { > >> DRM_ERROR("failed to remove conflicting framebuffer drivers\n"); > >> - goto out_gtt; > >> + goto out_ggtt; > >> } > >> > >> ret = i915_kick_out_vgacon(dev_priv); > >> if (ret) { > >> DRM_ERROR("failed to remove conflicting VGA console\n"); > >> - goto out_gtt; > >> + goto out_ggtt; > >> } > >> > >> pci_set_master(dev->pdev); > >> @@ -1222,7 +1222,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > >> aperture_size); > >> if (dev_priv->ggtt.mappable == NULL) { > >> ret = -EIO; > >> - goto out_gtt; > >> + goto out_ggtt; > >> } > >> > >> dev_priv->ggtt.mtrr = arch_phys_wc_add(dev_priv->ggtt.mappable_base, > >> @@ -1255,8 +1255,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > >> > >> return 0; > >> > >> -out_gtt: > >> - i915_global_gtt_cleanup(dev); > >> +out_ggtt: > >> + i915_cleanup_ggtt_hw(dev); > >> > >> return ret; > >> } > >> @@ -1275,7 +1275,7 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv) > >> pm_qos_remove_request(&dev_priv->pm_qos); > >> arch_phys_wc_del(dev_priv->ggtt.mtrr); > >> io_mapping_free(dev_priv->ggtt.mappable); > >> - i915_global_gtt_cleanup(dev); > >> + i915_cleanup_ggtt_hw(dev); > >> } > >> > >> /** > >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >> index 8588c83..506a706 100644 > >> --- a/drivers/gpu/drm/i915/i915_gem.c > >> +++ b/drivers/gpu/drm/i915/i915_gem.c > >> @@ -4972,7 +4972,7 @@ int i915_gem_init(struct drm_device *dev) > >> if (ret) > >> goto out_unlock; > >> > >> - i915_gem_init_global_gtt(dev); > >> + i915_gem_init_ggtt(dev); > >> > >> ret = i915_gem_context_init(dev); > >> if (ret) > >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > >> index 0715bb7..c23513b 100644 > >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > >> @@ -2808,7 +2808,11 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev, > >> return 0; > >> } > >> > >> -void i915_gem_init_global_gtt(struct drm_device *dev) > >> +/** > >> + * i915_gem_init_ggtt - Initialize GEM for Global GTT > >> + * @dev: DRM device > >> + */ > >> +void i915_gem_init_ggtt(struct drm_device *dev) > >> { > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> u64 gtt_size, mappable_size; > >> @@ -2819,7 +2823,11 @@ void i915_gem_init_global_gtt(struct drm_device *dev) > >> i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size); > >> } > >> > >> -void i915_global_gtt_cleanup(struct drm_device *dev) > >> +/** > >> + * i915_cleanup_ggtt_hw - Clean up GGTT hardware initialization > >> + * @dev: DRM device > >> + */ > >> +void i915_cleanup_ggtt_hw(struct drm_device *dev) > >> { > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> struct i915_address_space *vm = &dev_priv->ggtt.base; > >> @@ -3157,7 +3165,11 @@ static void i915_gmch_remove(struct i915_address_space *vm) > >> intel_gmch_remove(); > >> } > >> > >> -int i915_gem_gtt_init(struct drm_device *dev) > >> +/** > >> + * i915_init_ggtt_hw - Initialize GGTT hardware > >> + * @dev: DRM device > >> + */ > >> +int i915_init_ggtt_hw(struct drm_device *dev) > >> { > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> struct i915_ggtt *ggtt = &dev_priv->ggtt; > >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > >> index d804be0..95bf9a0 100644 > >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > >> @@ -513,10 +513,9 @@ i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n) > >> px_dma(ppgtt->base.scratch_pd); > >> } > >> > >> -int i915_gem_gtt_init(struct drm_device *dev); > >> -void i915_gem_init_global_gtt(struct drm_device *dev); > >> -void i915_global_gtt_cleanup(struct drm_device *dev); > >> - > >> +int i915_init_ggtt_hw(struct drm_device *dev); > >> +void i915_gem_init_ggtt(struct drm_device *dev); > >> +void i915_cleanup_ggtt_hw(struct drm_device *dev); > >> > >> int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); > >> int i915_ppgtt_init_hw(struct drm_device *dev); > >> -- > >> 2.5.5 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Mar 23, 2016 at 06:02:34PM +0200, Ville Syrjälä wrote: > On Wed, Mar 23, 2016 at 05:54:09PM +0200, Mika Kuoppala wrote: > > Ville Syrjälä <ville.syrjala@linux.intel.com> writes: > > > > > [ text/plain ] > > > On Wed, Mar 23, 2016 at 03:00:22PM +0200, Joonas Lahtinen wrote: > > >> Rename and document the GGTT init functions to give a better > > >> idea of the context where they are called from. > > >> > > >> i915_gem_gtt_init => i915_init_ggtt_hw > > > > > > Seems to me i915_ggtt_init_hw would match existing practices better. > > > > > > > There is also some gravity towards putting the verb first. In gem > > side atleast. > > At least in this case ggtt_init_hw would match ppgtt_init_hw, which > seems like a nice thing. It would also help with the plan to tag the init_hw phases consistently. -Chris
On ke, 2016-03-23 at 18:02 +0200, Ville Syrjälä wrote: > On Wed, Mar 23, 2016 at 05:54:09PM +0200, Mika Kuoppala wrote: > > > > Ville Syrjälä <ville.syrjala@linux.intel.com> writes: > > > > > > > > [ text/plain ] > > > On Wed, Mar 23, 2016 at 03:00:22PM +0200, Joonas Lahtinen wrote: > > > > > > > > Rename and document the GGTT init functions to give a better > > > > idea of the context where they are called from. > > > > > > > > i915_gem_gtt_init => i915_init_ggtt_hw > > > Seems to me i915_ggtt_init_hw would match existing practices better. > > > > > There is also some gravity towards putting the verb first. In gem > > side atleast. > At least in this case ggtt_init_hw would match ppgtt_init_hw, which > seems like a nice thing. > Right, I have changed the order quite a few times already. If it's i915_init_* (like i915_init_userptr), will be easier to grep. Adding Chris here as we discussed this yesterday. His idea is that logic should be action_feature and object_verb, init_some_thingamagic, vs object_destroy. Whatever we decide on, we should drop a small note at kerneldoc. Regards, Joonas
On ke, 2016-03-23 at 17:54 +0200, Mika Kuoppala wrote: > Ville Syrjälä <ville.syrjala@linux.intel.com> writes: > > > > > [ text/plain ] > > On Wed, Mar 23, 2016 at 03:00:22PM +0200, Joonas Lahtinen wrote: > > > > > > Rename and document the GGTT init functions to give a better > > > idea of the context where they are called from. > > > > > > i915_gem_gtt_init => i915_init_ggtt_hw > > Seems to me i915_ggtt_init_hw would match existing practices better. > > > There is also some gravity towards putting the verb first. In gem > side atleast. > When one excludes i915_gem_init_* functions there are not so many left (0). So I will change it as suggested by Ville (and Daniel too). Regards, Joonas > -Mika > > > > > > > > > > i915_gem_init_global_gtt => i915_gem_init_ggtt > > > i915_global_gtt_cleanup => i915_cleanup_ggtt_hw > > > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_dma.c | 14 +++++++------- > > > drivers/gpu/drm/i915/i915_gem.c | 2 +- > > > drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++++++--- > > > drivers/gpu/drm/i915/i915_gem_gtt.h | 7 +++---- > > > 4 files changed, 26 insertions(+), 15 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > > index fc8ac98..124cefd 100644 > > > --- a/drivers/gpu/drm/i915/i915_dma.c > > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > > @@ -1180,7 +1180,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > > > > > > intel_device_info_runtime_init(dev); > > > > > > - ret = i915_gem_gtt_init(dev); > > > + ret = i915_init_ggtt_hw(dev); > > > if (ret) > > > return ret; > > > > > > @@ -1189,13 +1189,13 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > > > ret = i915_kick_out_firmware_fb(dev_priv); > > > if (ret) { > > > DRM_ERROR("failed to remove conflicting framebuffer drivers\n"); > > > - goto out_gtt; > > > + goto out_ggtt; > > > } > > > > > > ret = i915_kick_out_vgacon(dev_priv); > > > if (ret) { > > > DRM_ERROR("failed to remove conflicting VGA console\n"); > > > - goto out_gtt; > > > + goto out_ggtt; > > > } > > > > > > pci_set_master(dev->pdev); > > > @@ -1222,7 +1222,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > > > aperture_size); > > > if (dev_priv->ggtt.mappable == NULL) { > > > ret = -EIO; > > > - goto out_gtt; > > > + goto out_ggtt; > > > } > > > > > > dev_priv->ggtt.mtrr = arch_phys_wc_add(dev_priv->ggtt.mappable_base, > > > @@ -1255,8 +1255,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) > > > > > > return 0; > > > > > > -out_gtt: > > > - i915_global_gtt_cleanup(dev); > > > +out_ggtt: > > > + i915_cleanup_ggtt_hw(dev); > > > > > > return ret; > > > } > > > @@ -1275,7 +1275,7 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv) > > > pm_qos_remove_request(&dev_priv->pm_qos); > > > arch_phys_wc_del(dev_priv->ggtt.mtrr); > > > io_mapping_free(dev_priv->ggtt.mappable); > > > - i915_global_gtt_cleanup(dev); > > > + i915_cleanup_ggtt_hw(dev); > > > } > > > > > > /** > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > > index 8588c83..506a706 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem.c > > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > > @@ -4972,7 +4972,7 @@ int i915_gem_init(struct drm_device *dev) > > > if (ret) > > > goto out_unlock; > > > > > > - i915_gem_init_global_gtt(dev); > > > + i915_gem_init_ggtt(dev); > > > > > > ret = i915_gem_context_init(dev); > > > if (ret) > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > > index 0715bb7..c23513b 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > > @@ -2808,7 +2808,11 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev, > > > return 0; > > > } > > > > > > -void i915_gem_init_global_gtt(struct drm_device *dev) > > > +/** > > > + * i915_gem_init_ggtt - Initialize GEM for Global GTT > > > + * @dev: DRM device > > > + */ > > > +void i915_gem_init_ggtt(struct drm_device *dev) > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > u64 gtt_size, mappable_size; > > > @@ -2819,7 +2823,11 @@ void i915_gem_init_global_gtt(struct drm_device *dev) > > > i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size); > > > } > > > > > > -void i915_global_gtt_cleanup(struct drm_device *dev) > > > +/** > > > + * i915_cleanup_ggtt_hw - Clean up GGTT hardware initialization > > > + * @dev: DRM device > > > + */ > > > +void i915_cleanup_ggtt_hw(struct drm_device *dev) > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > struct i915_address_space *vm = &dev_priv->ggtt.base; > > > @@ -3157,7 +3165,11 @@ static void i915_gmch_remove(struct i915_address_space *vm) > > > intel_gmch_remove(); > > > } > > > > > > -int i915_gem_gtt_init(struct drm_device *dev) > > > +/** > > > + * i915_init_ggtt_hw - Initialize GGTT hardware > > > + * @dev: DRM device > > > + */ > > > +int i915_init_ggtt_hw(struct drm_device *dev) > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > struct i915_ggtt *ggtt = &dev_priv->ggtt; > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > > > index d804be0..95bf9a0 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > > > @@ -513,10 +513,9 @@ i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n) > > > px_dma(ppgtt->base.scratch_pd); > > > } > > > > > > -int i915_gem_gtt_init(struct drm_device *dev); > > > -void i915_gem_init_global_gtt(struct drm_device *dev); > > > -void i915_global_gtt_cleanup(struct drm_device *dev); > > > - > > > +int i915_init_ggtt_hw(struct drm_device *dev); > > > +void i915_gem_init_ggtt(struct drm_device *dev); > > > +void i915_cleanup_ggtt_hw(struct drm_device *dev); > > > > > > int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); > > > int i915_ppgtt_init_hw(struct drm_device *dev); > > > -- > > > 2.5.5 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 24/03/16 07:40, Joonas Lahtinen wrote: > On ke, 2016-03-23 at 18:02 +0200, Ville Syrjälä wrote: >> On Wed, Mar 23, 2016 at 05:54:09PM +0200, Mika Kuoppala wrote: >>> >>> Ville Syrjälä <ville.syrjala@linux.intel.com> writes: >>> >>>> >>>> [ text/plain ] >>>> On Wed, Mar 23, 2016 at 03:00:22PM +0200, Joonas Lahtinen wrote: >>>>> >>>>> Rename and document the GGTT init functions to give a better >>>>> idea of the context where they are called from. >>>>> >>>>> i915_gem_gtt_init => i915_init_ggtt_hw >>>> Seems to me i915_ggtt_init_hw would match existing practices better. >>>> >>> There is also some gravity towards putting the verb first. In gem >>> side atleast. >> At least in this case ggtt_init_hw would match ppgtt_init_hw, which >> seems like a nice thing. >> > > Right, I have changed the order quite a few times already. If it's > i915_init_* (like i915_init_userptr), will be easier to grep. > > Adding Chris here as we discussed this yesterday. His idea is that > logic should be action_feature and object_verb, init_some_thingamagic, > vs object_destroy. Reasonable enough, as long as we can tell what's a feature and what's an object. A totally RPN scheme would be even clearer, since we would then treat features as objects (and actions are verbs), yielding: i915_userptr_init() i915_engine_setup() i915_object_destroy() and the like. That would require: i915_gem_init_global_gtt => i915_gem_ggtt_init i915_gem_gtt_init => i915_ggtt_hw_init i915_global_gtt_cleanup => i915_ggtt_hw_cleanup and i915_pggtt_init() i915_pggtt_hw_init() and perhaps i915_context_allocate() i915_hw_ctx_init() i915_hw_ctx_pin_and_map() i915_context_free() What do people think counts as "features" in Chris' scheme? > Whatever we decide on, we should drop a small note at kerneldoc. > > Regards, Joonas And perhaps we should have a list of preferred verbs with sensible meaning, e.g. "allocate" or "create", "init" or "setup", "free" or "release" or "destroy", etc? .Dave.
On Thu, Mar 24, 2016 at 04:01:53PM +0000, Dave Gordon wrote: > On 24/03/16 07:40, Joonas Lahtinen wrote: > >On ke, 2016-03-23 at 18:02 +0200, Ville Syrjälä wrote: > >>On Wed, Mar 23, 2016 at 05:54:09PM +0200, Mika Kuoppala wrote: > >>> > >>>Ville Syrjälä <ville.syrjala@linux.intel.com> writes: > >>> > >>>> > >>>>[ text/plain ] > >>>>On Wed, Mar 23, 2016 at 03:00:22PM +0200, Joonas Lahtinen wrote: > >>>>> > >>>>>Rename and document the GGTT init functions to give a better > >>>>>idea of the context where they are called from. > >>>>> > >>>>>i915_gem_gtt_init => i915_init_ggtt_hw > >>>>Seems to me i915_ggtt_init_hw would match existing practices better. > >>>> > >>>There is also some gravity towards putting the verb first. In gem > >>>side atleast. > >>At least in this case ggtt_init_hw would match ppgtt_init_hw, which > >>seems like a nice thing. > >> > > > >Right, I have changed the order quite a few times already. If it's > >i915_init_* (like i915_init_userptr), will be easier to grep. > > > >Adding Chris here as we discussed this yesterday. His idea is that > >logic should be action_feature and object_verb, init_some_thingamagic, > >vs object_destroy. I said object_verb[_phase]. On reflection I mean object_verb_adverb > Reasonable enough, as long as we can tell what's a feature and > what's an object. A totally RPN scheme would be even clearer, since > we would then treat features as objects (and actions are verbs), > yielding: > > i915_userptr_init() > i915_engine_setup() > i915_object_destroy() If those objects exist, yes. e.g userptr doesn't, but intel_engine does. Hence i915_gem_init_userptr() and it should have been intel_render_engine_init (oh well). > and the like. That would require: > > i915_gem_init_global_gtt => i915_gem_ggtt_init > i915_gem_gtt_init => i915_ggtt_hw_init > i915_global_gtt_cleanup => i915_ggtt_hw_cleanup Not quite. init_hw is the verb (or rather verb_adverb). Into that if an object doesn't exist such as stolen or userptr, then that pattern doesn't hold and we use the parent as the actor (subject) and move stolen after the verb (so it describes the verb, because to call it the object would be confusing!). Along those lines it is why I like i915_init_hw, i915_init_mmio etc over i915_hw_init, i915_mmio_init as then it clear that the verb is init and we are describing what phase of init we are in (and that we are operating at the driver level). > and > > i915_pggtt_init() > i915_pggtt_hw_init() > > and perhaps > > i915_context_allocate() > i915_hw_ctx_init() > i915_hw_ctx_pin_and_map() > i915_context_free() > > What do people think counts as "features" in Chris' scheme? Not features, objects, which is quite easy to define as anything that has a class^W struct. -Chris
On to, 2016-03-24 at 17:02 +0000, Patchwork wrote: > == Series Details == > > Series: series starting with [v2] drm/i915: Rename GGTT init functions (rev3) > URL : https://patchwork.freedesktop.org/series/4790/ > State : warning > > == Summary == > > Series 4790v3 Series without cover letter > http://patchwork.freedesktop.org/api/1.0/series/4790/revisions/3/mbox/ > > Test gem_exec_basic: > Subgroup gtt-blt: > pass -> DMESG-WARN (bsw-nuc-2) [drm:intel_runtime_suspend [i915]] *ERROR* Unclaimed access detected prior to suspending Problem in BSW/BYT runtime PM, reported at: https://bugs.freedesktop.org/show_bug.cgi?id=94164 > Test pm_rpm: > Subgroup basic-rte: > pass -> DMESG-WARN (bsw-nuc-2) "====================================================== [ INFO: possible circular locking dependency detected ] 4.5.0-gfxbench+ #1 Not tainted ------------------------------------------------------- gem_exec_basic/5896 is trying to acquire lock: (&dev->struct_mutex){+.+.+.}, at: [<ffffffff81514ea1>] drm_gem_mmap+0x1a1/0x270 but task is already holding lock: (&mm->mmap_sem){++++++}, at: [<ffffffff8117d8c4>] vm_mmap_pgoff+0x44/0xa0 which lock already depends on the new lock." Lockdep issue (completely unrelated to renaming functions :P), this whole chain causing multiple splats is being worked on. Opened a bug to track this specific warn: https://bugs.freedesktop.org/show_bug.cgi?id=94759 Regards, Joonas > > bdw-nuci7 total:192 pass:179 dwarn:0 dfail:0 fail:1 skip:12 > bdw-ultra total:192 pass:170 dwarn:0 dfail:0 fail:1 skip:21 > bsw-nuc-2 total:192 pass:153 dwarn:2 dfail:0 fail:0 skip:37 > byt-nuc total:192 pass:156 dwarn:1 dfail:0 fail:0 skip:35 > hsw-brixbox total:192 pass:170 dwarn:0 dfail:0 fail:0 skip:22 > hsw-gt2 total:192 pass:175 dwarn:0 dfail:0 fail:0 skip:17 > ivb-t430s total:192 pass:167 dwarn:0 dfail:0 fail:0 skip:25 > skl-i7k-2 total:192 pass:169 dwarn:0 dfail:0 fail:0 skip:23 > skl-nuci5 total:192 pass:181 dwarn:0 dfail:0 fail:0 skip:11 > snb-dellxps total:192 pass:158 dwarn:0 dfail:0 fail:0 skip:34 > snb-x220t total:192 pass:158 dwarn:0 dfail:0 fail:1 skip:33 > > Results at /archive/results/CI_IGT_test/Patchwork_1712/ > > f5d413cccefa1f93d64c34f357151d42add63a84 drm-intel-nightly: 2016y-03m-24d-14h-34m-29s UTC integration manifest > 9a1fb4808f9bc1219d05bf79c4a90079c246e7ed drm/i915: Refer to GGTT VM consistently > 4351646d82c63100310f5d4e437c2f8a4435c9a7 drm/i915: Rename GGTT init functions >
On to, 2016-03-31 at 13:44 +0000, Patchwork wrote: > == Series Details == > > Series: series starting with [v2] drm/i915: Rename GGTT init functions (rev5) > URL : https://patchwork.freedesktop.org/series/4790/ > State : warning > > == Summary == > > Series 4790v5 Series without cover letter > http://patchwork.freedesktop.org/api/1.0/series/4790/revisions/5/mbox/ > > Test gem_mmap_gtt: > Subgroup basic-read-no-prefault: > pass -> DMESG-WARN (bsw-nuc-2) > Test kms_pipe_crc_basic: > Subgroup suspend-read-crc-pipe-c: > dmesg-warn -> PASS (bsw-nuc-2) This is the kernfs lockdep warn (awaiting comments from upstream maintainers): https://bugs.freedesktop.org/show_bug.cgi?id=94759 So I'm merging this other patch too as it was supposed to be NOOP too (just renames). Regards, Joonas > > bdw-nuci7 total:196 pass:184 dwarn:0 dfail:0 fail:0 skip:12 > bdw-ultra total:196 pass:175 dwarn:0 dfail:0 fail:0 skip:21 > bsw-nuc-2 total:196 pass:158 dwarn:1 dfail:0 fail:0 skip:37 > hsw-brixbox total:196 pass:174 dwarn:0 dfail:0 fail:0 skip:22 > hsw-gt2 total:21 pass:19 dwarn:0 dfail:0 fail:0 skip:1 > skl-i7k-2 total:196 pass:173 dwarn:0 dfail:0 fail:0 skip:23 > skl-nuci5 total:196 pass:185 dwarn:0 dfail:0 fail:0 skip:11 > snb-dellxps total:27 pass:23 dwarn:0 dfail:0 fail:0 skip:3 > > Results at /archive/results/CI_IGT_test/Patchwork_1757/ > > 03c0f854e93263563f559d2bc8e47fb51adae697 drm-intel-nightly: 2016y-03m-31d-10h-50m-15s UTC integration manifest > 5061b7bad84ec0d6047783dc1a23f1e592e7d5d8 drm/i915: Refer to GGTT {,VM} consistently >
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index fc8ac98..124cefd 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1180,7 +1180,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) intel_device_info_runtime_init(dev); - ret = i915_gem_gtt_init(dev); + ret = i915_init_ggtt_hw(dev); if (ret) return ret; @@ -1189,13 +1189,13 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) ret = i915_kick_out_firmware_fb(dev_priv); if (ret) { DRM_ERROR("failed to remove conflicting framebuffer drivers\n"); - goto out_gtt; + goto out_ggtt; } ret = i915_kick_out_vgacon(dev_priv); if (ret) { DRM_ERROR("failed to remove conflicting VGA console\n"); - goto out_gtt; + goto out_ggtt; } pci_set_master(dev->pdev); @@ -1222,7 +1222,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) aperture_size); if (dev_priv->ggtt.mappable == NULL) { ret = -EIO; - goto out_gtt; + goto out_ggtt; } dev_priv->ggtt.mtrr = arch_phys_wc_add(dev_priv->ggtt.mappable_base, @@ -1255,8 +1255,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) return 0; -out_gtt: - i915_global_gtt_cleanup(dev); +out_ggtt: + i915_cleanup_ggtt_hw(dev); return ret; } @@ -1275,7 +1275,7 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv) pm_qos_remove_request(&dev_priv->pm_qos); arch_phys_wc_del(dev_priv->ggtt.mtrr); io_mapping_free(dev_priv->ggtt.mappable); - i915_global_gtt_cleanup(dev); + i915_cleanup_ggtt_hw(dev); } /** diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8588c83..506a706 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4972,7 +4972,7 @@ int i915_gem_init(struct drm_device *dev) if (ret) goto out_unlock; - i915_gem_init_global_gtt(dev); + i915_gem_init_ggtt(dev); ret = i915_gem_context_init(dev); if (ret) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 0715bb7..c23513b 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2808,7 +2808,11 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev, return 0; } -void i915_gem_init_global_gtt(struct drm_device *dev) +/** + * i915_gem_init_ggtt - Initialize GEM for Global GTT + * @dev: DRM device + */ +void i915_gem_init_ggtt(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; u64 gtt_size, mappable_size; @@ -2819,7 +2823,11 @@ void i915_gem_init_global_gtt(struct drm_device *dev) i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size); } -void i915_global_gtt_cleanup(struct drm_device *dev) +/** + * i915_cleanup_ggtt_hw - Clean up GGTT hardware initialization + * @dev: DRM device + */ +void i915_cleanup_ggtt_hw(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct i915_address_space *vm = &dev_priv->ggtt.base; @@ -3157,7 +3165,11 @@ static void i915_gmch_remove(struct i915_address_space *vm) intel_gmch_remove(); } -int i915_gem_gtt_init(struct drm_device *dev) +/** + * i915_init_ggtt_hw - Initialize GGTT hardware + * @dev: DRM device + */ +int i915_init_ggtt_hw(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct i915_ggtt *ggtt = &dev_priv->ggtt; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index d804be0..95bf9a0 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -513,10 +513,9 @@ i915_page_dir_dma_addr(const struct i915_hw_ppgtt *ppgtt, const unsigned n) px_dma(ppgtt->base.scratch_pd); } -int i915_gem_gtt_init(struct drm_device *dev); -void i915_gem_init_global_gtt(struct drm_device *dev); -void i915_global_gtt_cleanup(struct drm_device *dev); - +int i915_init_ggtt_hw(struct drm_device *dev); +void i915_gem_init_ggtt(struct drm_device *dev); +void i915_cleanup_ggtt_hw(struct drm_device *dev); int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); int i915_ppgtt_init_hw(struct drm_device *dev);