Quoting Souza, Jose (2022-05-09 17:19:28)
> On Mon, 2022-05-09 at 15:38 +0300, Joonas Lahtinen wrote:
> > Quoting José Roberto de Souza (2022-05-07 16:28:50)
> > > No need to have this parameter in intel_device_info struct
> > > as this feature is supported by Broadwell, Haswell all platforms with
> > > display version 9 or newer.
> > 
> > This is opposite of the direction we want to move to.
> > 
> > We want to embrace the has_xyz flags, instead of the macro trickery.
> 
> This ever growing flag definition is causing problems when defining new 
> platforms.

The ever growing macros that change between kernel versions are much
more painful than easily printable list per SKU.

Just to make it clear, a strict NACK from me for merging any patches
into this direction.

Regards, Joonas

> 
> There is too many features to check if a new platform supports each one of 
> it, what is leading to platform definition errors.
> 
> Also usually when a feature is dropped a HSD will be filed, so the person 
> taking care of that can just adjust the macro upper platform or IP bound and
> disable it for good. 
> 
> > 
> > > As a side effect of the of removal this flag, it will not be printed
> > > in dmesg during driver load anymore and developers will have to rely
> > > on to check the macro and compare with platform being used and IP
> > > versions of it.
> > 
> > This is not a very good rationale. If the platform has something, but it
> > becomes disabled in runtime, then we should add an another print after
> > the runtime sanitization has been done.
> 
> In my opinion this flags should only change in runtime if the feature is 
> fused off like is done for has_dsc and has_dmc.
> 
> > 
> > Regards, Joonas
> > 
> > > 
> > > Signed-off-by: José Roberto de Souza <[email protected]>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h          | 4 +++-
> > >  drivers/gpu/drm/i915/i915_pci.c          | 3 ---
> > >  drivers/gpu/drm/i915/intel_device_info.h | 1 -
> > >  3 files changed, 3 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 4b1025dbaab2a..4a1edf48d37b9 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1306,7 +1306,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> > >                                           IS_BROADWELL(dev_priv) || \
> > >                                           IS_HASWELL(dev_priv))
> > >  #define HAS_DP_MST(dev_priv)            (HAS_DDI(dev_priv))
> > > -#define HAS_FPGA_DBG_UNCLAIMED(dev_priv) 
> > > (INTEL_INFO(dev_priv)->display.has_fpga_dbg)
> > > +#define HAS_FPGA_DBG_UNCLAIMED(dev_priv) (DISPLAY_VER(dev_priv) >= 9 || \
> > > +                                         IS_BROADWELL(dev_priv) || \
> > > +                                         IS_HASWELL(dev_priv))
> > >  #define HAS_PSR(dev_priv)               (DISPLAY_VER(dev_priv) >= 9)
> > >  #define HAS_PSR2_SEL_FETCH(dev_priv)    (DISPLAY_VER(dev_priv) >= 12)
> > >  #define HAS_TRANSCODER(dev_priv, trans)         
> > > ((INTEL_INFO(dev_priv)->display.cpu_transcoder_mask & BIT(trans)) != 0)
> > > diff --git a/drivers/gpu/drm/i915/i915_pci.c 
> > > b/drivers/gpu/drm/i915/i915_pci.c
> > > index 5a42acb162a15..6a5b70b3ea2d7 100644
> > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > @@ -523,7 +523,6 @@ static const struct intel_device_info vlv_info = {
> > >         .platform_engine_mask = BIT(RCS0) | BIT(VCS0) | BIT(BCS0) | 
> > > BIT(VECS0), \
> > >         .display.cpu_transcoder_mask = BIT(TRANSCODER_A) | 
> > > BIT(TRANSCODER_B) | \
> > >                 BIT(TRANSCODER_C) | BIT(TRANSCODER_EDP), \
> > > -       .display.has_fpga_dbg = 1, \
> > >         HSW_PIPE_OFFSETS
> > >  
> > >  #define HSW_PLATFORM \
> > > @@ -657,7 +656,6 @@ static const struct intel_device_info skl_gt4_info = {
> > >         .display.cpu_transcoder_mask = BIT(TRANSCODER_A) | 
> > > BIT(TRANSCODER_B) | \
> > >                 BIT(TRANSCODER_C) | BIT(TRANSCODER_EDP) | \
> > >                 BIT(TRANSCODER_DSI_A) | BIT(TRANSCODER_DSI_C), \
> > > -       .display.has_fpga_dbg = 1, \
> > >         .display.fbc_mask = BIT(INTEL_FBC_A), \
> > >         .display.has_hdcp = 1, \
> > >         .display.has_dmc = 1, \
> > > @@ -894,7 +892,6 @@ static const struct intel_device_info adl_s_info = {
> > >         .display.has_dmc = 1,                                             
> > >       \
> > >         .display.has_dsc = 1,                                             
> > >       \
> > >         .display.fbc_mask = BIT(INTEL_FBC_A),                             
> > >       \
> > > -       .display.has_fpga_dbg = 1,                                        
> > >       \
> > >         .display.has_hdcp = 1,                                            
> > >       \
> > >         .display.has_hotplug = 1,                                         
> > >       \
> > >         .display.ver = 13,                                                
> > >       \
> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h 
> > > b/drivers/gpu/drm/i915/intel_device_info.h
> > > index 7581ef4a68f94..e61a334b611ac 100644
> > > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > > @@ -157,7 +157,6 @@ enum intel_ppgtt_type {
> > >         func(has_cdclk_crawl); \
> > >         func(has_dmc); \
> > >         func(has_dsc); \
> > > -       func(has_fpga_dbg); \
> > >         func(has_gmch); \
> > >         func(has_hdcp); \
> > >         func(has_hotplug); \
> > > -- 
> > > 2.36.0
> > > 
> 

Reply via email to