Re: [PATCH 03/23] drm/i915: Move HAS_RUNTIME_PM definition to platform

[Imre Deak <imre.deak@intel.com>]

On ke, 2016-07-20 at 13:25 -0700, Rodrigo Vivi wrote:
> On Wed, Jul 20, 2016 at 10:40 AM, Carlos Santa  wrote:
> > Moving all GPU features to the platform struct definition allows for
> >         - standard place when adding new features from new platforms
> >         - possible to see supported features when dumping struct
> >           definitions
> > 
> > Signed-off-by: Carlos Santa <carlos.santa@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h | 6 ++----
> >  drivers/gpu/drm/i915/i915_pci.c | 7 ++++++-
> >  2 files changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 6569eb7..7443b9a 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -769,6 +769,7 @@ struct intel_csr {
> >         func(is_preliminary) sep \
> >         func(has_fbc) sep \
> >         func(has_psr) sep \
> > +       func(has_runtime_pm) sep \
> >         func(has_pipe_cxsr) sep \
> >         func(has_hotplug) sep \
> >         func(cursor_needs_physical) sep \
> > @@ -2848,10 +2849,7 @@ struct drm_i915_cmd_table {
> >  #define HAS_DDI(dev)           (INTEL_INFO(dev)->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_RUNTIME_PM(dev)    (IS_GEN6(dev) || IS_HASWELL(dev) || \
> > -                                IS_BROADWELL(dev) || IS_VALLEYVIEW(dev) || \
> > -                                IS_CHERRYVIEW(dev) || IS_SKYLAKE(dev) || \
> > -                                IS_KABYLAKE(dev) || IS_BROXTON(dev))
> 
> Why don't we have runtime_pm on Ivybridge since we have on
> sandybdrige? Imre, any idea?

I don't know what are the exact differences, in any case I haven't
tried to enable RPM on IVB. I think Ville had plans for this.

--Imre

> 
> > +#define HAS_RUNTIME_PM(dev)    (INTEL_INFO(dev)->has_runtime_pm)
> >  #define HAS_RC6(dev)           (INTEL_INFO(dev)->gen >= 6)
> >  #define HAS_RC6p(dev)          (IS_GEN6(dev) || IS_IVYBRIDGE(dev))
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> > index 8b1311d..92ab3c2 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -198,6 +198,7 @@ static const struct intel_device_info intel_ironlake_m_info = {
> >         .gen = 6, .num_pipes = 2, \
> >         .need_gfx_hws = 1, .has_hotplug = 1, \
> >         .has_fbc = 1, \
> > +       .has_runtime_pm = 1, \
> 
> This patch made me notice that we should define the
> 
> GEN7_FEATURE on GEN6_FEATURES + new changes as a followup of patch
> 02/32 or in that same patch.
> However for this case we should redefine .has_runtime_pm=0 on gen7,
> what is really strange.
> 
> Anyway, this patch itself has nothing wrong and just follows what it
> was set there already.
> any change related to my comments should be addressed in separated patches.
> So fell free to also use
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> >         .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> >         .has_llc = 1, \
> >         GEN_DEFAULT_PIPEOFFSETS, \
> > @@ -241,6 +242,7 @@ static const struct intel_device_info intel_ivybridge_q_info = {
> >  #define VLV_FEATURES  \
> >         .gen = 7, .num_pipes = 2, \
> >         .has_psr = 1, \
> > +       .has_runtime_pm = 1, \
> >         .need_gfx_hws = 1, .has_hotplug = 1, \
> >         .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
> >         .display_mmio_offset = VLV_DISPLAY_BASE, \
> > @@ -263,7 +265,8 @@ static const struct intel_device_info intel_valleyview_d_info = {
> >         .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING, \
> >         .has_ddi = 1, \
> >         .has_fpga_dbg = 1, \
> > -       .has_psr = 1
> > +       .has_psr = 1, \
> > +       .has_runtime_pm = 1
> > 
> >  static const struct intel_device_info intel_haswell_d_info = {
> >         HSW_FEATURES,
> > @@ -312,6 +315,7 @@ static const struct intel_device_info intel_cherryview_info = {
> >         .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
> >         .is_cherryview = 1,
> >         .has_psr = 1,
> > +       .has_runtime_pm = 1,
> >         .display_mmio_offset = VLV_DISPLAY_BASE,
> >         GEN_CHV_PIPEOFFSETS,
> >         CURSOR_OFFSETS,
> > @@ -340,6 +344,7 @@ static const struct intel_device_info intel_broxton_info = {
> >         .has_ddi = 1,
> >         .has_fpga_dbg = 1,
> >         .has_fbc = 1,
> > +       .has_runtime_pm = 1,
> >         .has_pooled_eu = 0,
> >         GEN_DEFAULT_PIPEOFFSETS,
> >         IVB_CURSOR_OFFSETS,
> > --
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx