| Message ID | 1390616265-4329-3-git-send-email-benjamin.widawsky@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 24, 2014 at 06:17:43PM -0800, Ben Widawsky wrote: > There are cases where we want to know if there is a full, or aliased > ppgtt. Having to always to the || is annoying. This shorthand will keep > the code a bit cleaner/easier to read. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index e851a82..6f68515 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1844,6 +1844,7 @@ struct drm_i915_file_private { > #define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) && !IS_BROADWELL(dev)) > #define USES_ALIASING_PPGTT(dev) intel_enable_ppgtt(dev, false) > #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true) > +#define USES_PPGTT(dev) (USES_ALIASING_PPGTT(dev) || USES_FULL_PPGTT(dev)) Just do an s/ALIASING/HW/ and we get to the same point with overall clearer code. -Daniel > > #define HAS_OVERLAY(dev) (INTEL_INFO(dev)->has_overlay) > #define OVERLAY_NEEDS_PHYSICAL(dev) (INTEL_INFO(dev)->overlay_needs_physical) > -- > 1.8.5.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Sat, Jan 25, 2014 at 09:41:22PM +0100, Daniel Vetter wrote: > On Fri, Jan 24, 2014 at 06:17:43PM -0800, Ben Widawsky wrote: > > There are cases where we want to know if there is a full, or aliased > > ppgtt. Having to always to the || is annoying. This shorthand will keep > > the code a bit cleaner/easier to read. > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index e851a82..6f68515 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1844,6 +1844,7 @@ struct drm_i915_file_private { > > #define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) && !IS_BROADWELL(dev)) > > #define USES_ALIASING_PPGTT(dev) intel_enable_ppgtt(dev, false) > > #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true) > > +#define USES_PPGTT(dev) (USES_ALIASING_PPGTT(dev) || USES_FULL_PPGTT(dev)) > > Just do an s/ALIASING/HW/ and we get to the same point with overall > clearer code. > -Daniel I had plans to make USES_ALIASING actually mean it's using aliasing, and NOT full PPGTT. However, at this point, they are indeed logically equivalent. Let me go over the code again and see if we actually want an USES_ALIASING_PPGTT(), and if not, I'll do it your way. First, let's figure out if the series will get merged at all. > > > > > #define HAS_OVERLAY(dev) (INTEL_INFO(dev)->has_overlay) > > #define OVERLAY_NEEDS_PHYSICAL(dev) (INTEL_INFO(dev)->overlay_needs_physical) > > -- > > 1.8.5.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Sun, Jan 26, 2014 at 6:45 AM, Ben Widawsky <benjamin.widawsky@intel.com> wrote: >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> > index e851a82..6f68515 100644 >> > --- a/drivers/gpu/drm/i915/i915_drv.h >> > +++ b/drivers/gpu/drm/i915/i915_drv.h >> > @@ -1844,6 +1844,7 @@ struct drm_i915_file_private { >> > #define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) && !IS_BROADWELL(dev)) >> > #define USES_ALIASING_PPGTT(dev) intel_enable_ppgtt(dev, false) >> > #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true) >> > +#define USES_PPGTT(dev) (USES_ALIASING_PPGTT(dev) || USES_FULL_PPGTT(dev)) >> >> Just do an s/ALIASING/HW/ and we get to the same point with overall >> clearer code. >> -Daniel > > I had plans to make USES_ALIASING actually mean it's using aliasing, and > NOT full PPGTT. However, at this point, they are indeed logically > equivalent. Let me go over the code again and see if we actually want an > USES_ALIASING_PPGTT(), and if not, I'll do it your way. First, let's > figure out if the series will get merged at all. Well I've had a giant wtf moment when looking through your ppgtt patches before the holidays due to this, until I've noticed that USES_ALIASING_PPGTT also holds for full ppgtt. So I'm voting very much for a cleanup here, the current code is confusing as hell in some areas, at least for incompetent me ;-) Cheers, Daniel
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e851a82..6f68515 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1844,6 +1844,7 @@ struct drm_i915_file_private { #define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) && !IS_BROADWELL(dev)) #define USES_ALIASING_PPGTT(dev) intel_enable_ppgtt(dev, false) #define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true) +#define USES_PPGTT(dev) (USES_ALIASING_PPGTT(dev) || USES_FULL_PPGTT(dev)) #define HAS_OVERLAY(dev) (INTEL_INFO(dev)->has_overlay) #define OVERLAY_NEEDS_PHYSICAL(dev) (INTEL_INFO(dev)->overlay_needs_physical)
There are cases where we want to know if there is a full, or aliased ppgtt. Having to always to the || is annoying. This shorthand will keep the code a bit cleaner/easier to read. Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.h | 1 + 1 file changed, 1 insertion(+)