| Message ID | 1469036435-18918-11-git-send-email-carlos.santa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 20, 2016 at 10:40 AM, Carlos Santa <carlos.santa@intel.com> 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 | 3 ++- > drivers/gpu/drm/i915/i915_pci.c | 5 +++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a326a88..75131a0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -775,6 +775,7 @@ struct intel_csr { > func(has_guc) sep \ > func(has_guc_ucode) sep \ > func(has_guc_sched) sep \ > + func(has_rc6) sep \ > func(has_resource_streamer) sep \ > func(has_pipe_cxsr) sep \ > func(has_hotplug) sep \ > @@ -2856,7 +2857,7 @@ struct drm_i915_cmd_table { > #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) (INTEL_INFO(dev)->has_runtime_pm) > -#define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) > +#define HAS_RC6(dev) (INTEL_INFO(dev)->has_rc6) > #define HAS_RC6p(dev) (IS_GEN6(dev) || IS_IVYBRIDGE(dev)) > > #define HAS_CSR(dev) (INTEL_INFO(dev)->has_csr) > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index f59ad4b..e10fb5c 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -200,6 +200,7 @@ static const struct intel_device_info intel_ironlake_m_info = { > .has_fbc = 1, \ > .has_runtime_pm = 1, \ > .has_core_ring_freq = 1, \ > + .has_rc6 = 1, \ > .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ > .has_llc = 1, \ > GEN_DEFAULT_PIPEOFFSETS, \ > @@ -219,6 +220,7 @@ static const struct intel_device_info intel_sandybridge_m_info = { > .need_gfx_hws = 1, .has_hotplug = 1, \ > .has_fbc = 1, \ > .has_core_ring_freq = 1, \ > + .has_rc6 = 1, \ another case where the GEN7 features based on GEN6 needs to come first. > .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ > .has_llc = 1, \ > GEN_DEFAULT_PIPEOFFSETS, \ > @@ -245,6 +247,7 @@ static const struct intel_device_info intel_ivybridge_q_info = { > .gen = 7, .num_pipes = 2, \ > .has_psr = 1, \ > .has_runtime_pm = 1, \ > + .has_rc6 = 1, \ > .need_gfx_hws = 1, .has_hotplug = 1, \ > .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ > .display_mmio_offset = VLV_DISPLAY_BASE, \ > @@ -320,6 +323,7 @@ static const struct intel_device_info intel_cherryview_info = { > .has_psr = 1, > .has_runtime_pm = 1, > .has_resource_streamer = 1, > + .has_rc6 = 1, > .display_mmio_offset = VLV_DISPLAY_BASE, > GEN_CHV_PIPEOFFSETS, > CURSOR_OFFSETS, > @@ -358,6 +362,7 @@ static const struct intel_device_info intel_broxton_info = { > .has_runtime_pm = 1, > .has_pooled_eu = 0, > .has_resource_streamer = 1, > + .has_rc6 = 1, > GEN_DEFAULT_PIPEOFFSETS, > IVB_CURSOR_OFFSETS, > BDW_COLORS, > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 20/07/16 18:40, 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 | 3 ++- > drivers/gpu/drm/i915/i915_pci.c | 5 +++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a326a88..75131a0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -775,6 +775,7 @@ struct intel_csr { > func(has_guc) sep \ > func(has_guc_ucode) sep \ > func(has_guc_sched) sep \ > + func(has_rc6) sep \ > func(has_resource_streamer) sep \ > func(has_pipe_cxsr) sep \ > func(has_hotplug) sep \ > @@ -2856,7 +2857,7 @@ struct drm_i915_cmd_table { > #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) (INTEL_INFO(dev)->has_runtime_pm) > -#define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) > +#define HAS_RC6(dev) (INTEL_INFO(dev)->has_rc6) > #define HAS_RC6p(dev) (IS_GEN6(dev) || IS_IVYBRIDGE(dev)) > > #define HAS_CSR(dev) (INTEL_INFO(dev)->has_csr) > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index f59ad4b..e10fb5c 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -200,6 +200,7 @@ static const struct intel_device_info intel_ironlake_m_info = { > .has_fbc = 1, \ > .has_runtime_pm = 1, \ > .has_core_ring_freq = 1, \ > + .has_rc6 = 1, \ This platform claims to be Gen5 so no RC6. I have a slight reservation on all this because sometimes it is very useful to see that "INTEL_INFO(dev)->gen >= 6". When sprinkled around like here it becomes harder to figure out which feature is supported by which platforms. Once I tried something like this work (mind you I was concentrating only on HAS_ and IS_ macros which contain multiple conditionals - it was an excercise in reducing multiple conditionals at runtime), I decided to keep the macro but renamed it to have a leading underscore. And I did the assignment to device_info in an appropriate place, for example: device_info->has_rc6 = _HAS_RC6(dev_priv); For completeness: #define _HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) #define HAS_RC6(dev) (INTEL_INFO(dev)->has_rc6) I was not too happy with that approach either, but I think the downside I mentioned above is real. Regards, Tvrtko > .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ > .has_llc = 1, \ > GEN_DEFAULT_PIPEOFFSETS, \ > @@ -219,6 +220,7 @@ static const struct intel_device_info intel_sandybridge_m_info = { > .need_gfx_hws = 1, .has_hotplug = 1, \ > .has_fbc = 1, \ > .has_core_ring_freq = 1, \ > + .has_rc6 = 1, \ > .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ > .has_llc = 1, \ > GEN_DEFAULT_PIPEOFFSETS, \ > @@ -245,6 +247,7 @@ static const struct intel_device_info intel_ivybridge_q_info = { > .gen = 7, .num_pipes = 2, \ > .has_psr = 1, \ > .has_runtime_pm = 1, \ > + .has_rc6 = 1, \ > .need_gfx_hws = 1, .has_hotplug = 1, \ > .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ > .display_mmio_offset = VLV_DISPLAY_BASE, \ > @@ -320,6 +323,7 @@ static const struct intel_device_info intel_cherryview_info = { > .has_psr = 1, > .has_runtime_pm = 1, > .has_resource_streamer = 1, > + .has_rc6 = 1, > .display_mmio_offset = VLV_DISPLAY_BASE, > GEN_CHV_PIPEOFFSETS, > CURSOR_OFFSETS, > @@ -358,6 +362,7 @@ static const struct intel_device_info intel_broxton_info = { > .has_runtime_pm = 1, > .has_pooled_eu = 0, > .has_resource_streamer = 1, > + .has_rc6 = 1, > GEN_DEFAULT_PIPEOFFSETS, > IVB_CURSOR_OFFSETS, > BDW_COLORS, >
On Thu, Jul 21, 2016 at 3:50 AM, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > > On 20/07/16 18:40, 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 | 3 ++- >> drivers/gpu/drm/i915/i915_pci.c | 5 +++++ >> 2 files changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index a326a88..75131a0 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -775,6 +775,7 @@ struct intel_csr { >> func(has_guc) sep \ >> func(has_guc_ucode) sep \ >> func(has_guc_sched) sep \ >> + func(has_rc6) sep \ >> func(has_resource_streamer) sep \ >> func(has_pipe_cxsr) sep \ >> func(has_hotplug) sep \ >> @@ -2856,7 +2857,7 @@ struct drm_i915_cmd_table { >> #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) (INTEL_INFO(dev)->has_runtime_pm) >> -#define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) >> +#define HAS_RC6(dev) (INTEL_INFO(dev)->has_rc6) >> #define HAS_RC6p(dev) (IS_GEN6(dev) || IS_IVYBRIDGE(dev)) >> >> #define HAS_CSR(dev) (INTEL_INFO(dev)->has_csr) >> diff --git a/drivers/gpu/drm/i915/i915_pci.c >> b/drivers/gpu/drm/i915/i915_pci.c >> index f59ad4b..e10fb5c 100644 >> --- a/drivers/gpu/drm/i915/i915_pci.c >> +++ b/drivers/gpu/drm/i915/i915_pci.c >> @@ -200,6 +200,7 @@ static const struct intel_device_info >> intel_ironlake_m_info = { >> .has_fbc = 1, \ >> .has_runtime_pm = 1, \ >> .has_core_ring_freq = 1, \ >> + .has_rc6 = 1, \ > > > This platform claims to be Gen5 so no RC6. > > I have a slight reservation on all this because sometimes it is very useful > to see that "INTEL_INFO(dev)->gen >= 6". When sprinkled around like here it > becomes harder to figure out which feature is supported by which platforms. Well, right now we have 3 issues that this series address: 1 - unify/standardize the way we introduce a feature in a giving platform. 2 - have a view to all features available on a giving platform. 3 - Make platform enabling with legacy features easy the only downside is indeed for some cases it is getting hard now to answer: "which platforms support this specific feature?" But anyways they are all organized in only one file on structs and macros that are easy to navigate and understand. > > Once I tried something like this work (mind you I was concentrating only on > HAS_ and IS_ macros which contain multiple conditionals - it was an > excercise in reducing multiple conditionals at runtime), I decided to keep > the macro but renamed it to have a leading underscore. And I did the > assignment to device_info in an appropriate place, for example: > > device_info->has_rc6 = _HAS_RC6(dev_priv); But if you do this you loose the > > For completeness: > > #define _HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) Maybe a comment here would solve this downside: /* RC6 is supported on all platforms starting on gen6. */ #define HAS_RC6(dev) (INTEL_INFO(dev)->has_rc6) or something like this... Thanks, Rodrigo. > > I was not too happy with that approach either, but I think the downside I > mentioned above is real. > > Regards, > > Tvrtko > > >> .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ >> .has_llc = 1, \ >> GEN_DEFAULT_PIPEOFFSETS, \ >> @@ -219,6 +220,7 @@ static const struct intel_device_info >> intel_sandybridge_m_info = { >> .need_gfx_hws = 1, .has_hotplug = 1, \ >> .has_fbc = 1, \ >> .has_core_ring_freq = 1, \ >> + .has_rc6 = 1, \ >> .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ >> .has_llc = 1, \ >> GEN_DEFAULT_PIPEOFFSETS, \ >> @@ -245,6 +247,7 @@ static const struct intel_device_info >> intel_ivybridge_q_info = { >> .gen = 7, .num_pipes = 2, \ >> .has_psr = 1, \ >> .has_runtime_pm = 1, \ >> + .has_rc6 = 1, \ >> .need_gfx_hws = 1, .has_hotplug = 1, \ >> .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ >> .display_mmio_offset = VLV_DISPLAY_BASE, \ >> @@ -320,6 +323,7 @@ static const struct intel_device_info >> intel_cherryview_info = { >> .has_psr = 1, >> .has_runtime_pm = 1, >> .has_resource_streamer = 1, >> + .has_rc6 = 1, >> .display_mmio_offset = VLV_DISPLAY_BASE, >> GEN_CHV_PIPEOFFSETS, >> CURSOR_OFFSETS, >> @@ -358,6 +362,7 @@ static const struct intel_device_info >> intel_broxton_info = { >> .has_runtime_pm = 1, >> .has_pooled_eu = 0, >> .has_resource_streamer = 1, >> + .has_rc6 = 1, >> GEN_DEFAULT_PIPEOFFSETS, >> IVB_CURSOR_OFFSETS, >> BDW_COLORS, >> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a326a88..75131a0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -775,6 +775,7 @@ struct intel_csr { func(has_guc) sep \ func(has_guc_ucode) sep \ func(has_guc_sched) sep \ + func(has_rc6) sep \ func(has_resource_streamer) sep \ func(has_pipe_cxsr) sep \ func(has_hotplug) sep \ @@ -2856,7 +2857,7 @@ struct drm_i915_cmd_table { #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) (INTEL_INFO(dev)->has_runtime_pm) -#define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6) +#define HAS_RC6(dev) (INTEL_INFO(dev)->has_rc6) #define HAS_RC6p(dev) (IS_GEN6(dev) || IS_IVYBRIDGE(dev)) #define HAS_CSR(dev) (INTEL_INFO(dev)->has_csr) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index f59ad4b..e10fb5c 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -200,6 +200,7 @@ static const struct intel_device_info intel_ironlake_m_info = { .has_fbc = 1, \ .has_runtime_pm = 1, \ .has_core_ring_freq = 1, \ + .has_rc6 = 1, \ .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ .has_llc = 1, \ GEN_DEFAULT_PIPEOFFSETS, \ @@ -219,6 +220,7 @@ static const struct intel_device_info intel_sandybridge_m_info = { .need_gfx_hws = 1, .has_hotplug = 1, \ .has_fbc = 1, \ .has_core_ring_freq = 1, \ + .has_rc6 = 1, \ .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ .has_llc = 1, \ GEN_DEFAULT_PIPEOFFSETS, \ @@ -245,6 +247,7 @@ static const struct intel_device_info intel_ivybridge_q_info = { .gen = 7, .num_pipes = 2, \ .has_psr = 1, \ .has_runtime_pm = 1, \ + .has_rc6 = 1, \ .need_gfx_hws = 1, .has_hotplug = 1, \ .ring_mask = RENDER_RING | BSD_RING | BLT_RING, \ .display_mmio_offset = VLV_DISPLAY_BASE, \ @@ -320,6 +323,7 @@ static const struct intel_device_info intel_cherryview_info = { .has_psr = 1, .has_runtime_pm = 1, .has_resource_streamer = 1, + .has_rc6 = 1, .display_mmio_offset = VLV_DISPLAY_BASE, GEN_CHV_PIPEOFFSETS, CURSOR_OFFSETS, @@ -358,6 +362,7 @@ static const struct intel_device_info intel_broxton_info = { .has_runtime_pm = 1, .has_pooled_eu = 0, .has_resource_streamer = 1, + .has_rc6 = 1, GEN_DEFAULT_PIPEOFFSETS, IVB_CURSOR_OFFSETS, BDW_COLORS,
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 | 3 ++- drivers/gpu/drm/i915/i915_pci.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-)