On Fri, Nov 04, 2016 at 04:03:55PM +0000, Tvrtko Ursulin wrote:
>
> On 04/11/2016 15:32, Ville Syrjälä wrote:
> > On Fri, Nov 04, 2016 at 02:42:45PM +0000, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >>
> >> A small selection of macros which can only accept dev_priv from
> >> now on and a resulting trickle of fixups.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.h | 27 ++++++++++++---------------
> >> drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
> >> drivers/gpu/drm/i915/i915_irq.c | 6 +++---
> >> drivers/gpu/drm/i915/intel_crt.c | 8 ++++----
> >> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> >> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> >> drivers/gpu/drm/i915/intel_hotplug.c | 2 +-
> >> drivers/gpu/drm/i915/intel_psr.c | 2 +-
> >> 8 files changed, 25 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 45a30f730216..6060e41d25e5 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -2901,28 +2901,25 @@ struct drm_i915_cmd_table {
> >> #define HAS_128_BYTE_Y_TILING(dev_priv) (!IS_GEN2(dev_priv) && \
> >> !(IS_I915G(dev_priv) || \
> >> IS_I915GM(dev_priv)))
> >> -#define SUPPORTS_TV(dev) (INTEL_INFO(dev)->supports_tv)
> >> -#define I915_HAS_HOTPLUG(dev) (INTEL_INFO(dev)->has_hotplug)
> >> -
> >> -#define HAS_FW_BLC(dev_priv) (INTEL_GEN(dev_priv) > 2)
> >> -#define HAS_PIPE_CXSR(dev) (INTEL_INFO(dev)->has_pipe_cxsr)
> >> -#define HAS_FBC(dev) (INTEL_INFO(dev)->has_fbc)
> >> +#define SUPPORTS_TV(dev_priv) ((dev_priv)->info.supports_tv)
> >> +#define I915_HAS_HOTPLUG(dev_priv) ((dev_priv)->info.has_hotplug)
> >>
> >> +#define HAS_FW_BLC(dev_priv) (INTEL_GEN(dev_priv) > 2)
> >> +#define HAS_PIPE_CXSR(dev_priv) ((dev_priv)->info.has_pipe_cxsr)
> >> +#define HAS_FBC(dev_priv) ((dev_priv)->info.has_fbc)
> >> #define HAS_IPS(dev_priv) (IS_HSW_ULT(dev_priv) || IS_BROADWELL(dev_priv))
> >> -
> >> -#define HAS_DP_MST(dev) (INTEL_INFO(dev)->has_dp_mst)
> >> -
> >> +#define HAS_DP_MST(dev_priv) ((dev_priv)->info.has_dp_mst)
> >> #define HAS_DDI(dev_priv) ((dev_priv)->info.has_ddi)
> >> -#define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
> >> -#define HAS_PSR(dev) (INTEL_INFO(dev)->has_psr)
> >> -#define HAS_RC6(dev) (INTEL_INFO(dev)->has_rc6)
> >> -#define HAS_RC6p(dev) (INTEL_INFO(dev)->has_rc6p)
> >> -
> >> -#define HAS_CSR(dev) (INTEL_INFO(dev)->has_csr)
> >> +#define HAS_PSR(dev_priv) ((dev_priv)->info.has_psr)
> >> +#define HAS_RC6(dev_priv) ((dev_priv)->info.has_rc6)
> >> +#define HAS_RC6p(dev_priv) ((dev_priv)->info.has_rc6p)
> >> +#define HAS_CSR(dev_priv) ((dev_priv)->info.has_csr)
> >>
> >> #define HAS_RUNTIME_PM(dev_priv) ((dev_priv)->info.has_runtime_pm)
> >> #define HAS_64BIT_RELOC(dev_priv) ((dev_priv)->info.has_64bit_reloc)
> >>
> >> +#define HAS_FPGA_DBG_UNCLAIMED(dev_priv) ((dev_priv)->info.has_fpga_dbg)
> >
> > What's confusing me is this reordering of these macros. Was there a
> > particular reason for doing that?
>
> Just because of its long name, so I pulled it out and separated so the
> alignment is nicer in the blocks above it.
The original grouping looked more based on functionality, so made a bit
more sense to me.
>
> > Outside that it all looks pretty reasonable. Could got a bit further
> > with passing around dev_priv in some cases, but I guess we can leave
> > that to future work.
>
> Yes, I mention that in the cover letter.
>
> > One random idea that did pop into my head was this:
> >
> > static inline const struct ... *
> > intel_info(struct drm_i915_private *dev_priv)
> > {
> > return &dev_priv->info;
> > }
> > #define HAS_WHATEVER(dev_priv) (intel_info(dev_priv)->whatever)
> >
> > for some extra type safety. Any thoughts?
>
> Sounds like a good idea to me. And it would be really easy to do,
> localized to i915_drv.h, and then when the last INTEL_INFO(dev) gets
> converted we can make it use the inline as well.
>
> Regards,
>
> Tvrtko
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
