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

2016-08-11 Thread Carlos Santa 
On Tue, 2016-08-09 at 16:49 +0300, Ville Syrjälä wrote:
> On Thu, Jul 21, 2016 at 04:34:28PM +0300, Imre Deak wrote:
> > 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 
> > > > ---
> > > >  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.
> 
> I tried it, it gave zero benefit with the downside of killing HPD. So
> I've changed my mind and now I want to see the SNB runtime PM support
> removed instead. So runtime PM would be only for HSW+/VLV/CHV.

V3 of this patch series already does this.

> 
> And we still need to fix HPD while runtime suspended. But at least
> it's theoretically possible for those platforms. I don't think we can
> do it for SNB/IVB and older machines. And polling is no answer as
> it just increases the power consumption. So adding ineffective
> runtime PM with HPD polling is just wasting more power than not
> runtime suspending in the first place.
> 
> > 
> > --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 
> > > 
> > > > .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

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

2016-08-09 Thread Ville Syrjälä 
On Thu, Jul 21, 2016 at 04:34:28PM +0300, Imre Deak wrote:
> 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 
> > > ---
> > >  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.

I tried it, it gave zero benefit with the downside of killing HPD. So
I've changed my mind and now I want to see the SNB runtime PM support
removed instead. So runtime PM would be only for HSW+/VLV/CHV.

And we still need to fix HPD while runtime suspended. But at least
it's theoretically possible for those platforms. I don't think we can
do it for SNB/IVB and older machines. And polling is no answer as
it just increases the power consumption. So adding ineffective
runtime PM with HPD polling is just wasting more power than not
runtime suspending in the first place.

> 
> --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 
> > 
> > > .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

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

2016-08-01 Thread Jani Nikula 
On Wed, 20 Jul 2016, 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 
> ---
>  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))
> +#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, \
>   .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

Drive by bikeshedding, please always add the comma in the end even for
the last line so that you can add new ones without touching the
preceding lines like here.

BR,
Jani.

>  
>  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,

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2016-07-21 Thread Imre Deak 
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 
> > ---
> >  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 
> 
> > .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

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

2016-07-20 Thread Rodrigo Vivi 
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 
> ---
>  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?

> +#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 

> .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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 03/23] drm/i915: Move HAS_RUNTIME_PM definition to platform

2016-07-20 Thread Carlos Santa 
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 
---
 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))
+#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, \
.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