On Mon, 09 May 2022, "Souza, Jose" <jose.souza@xxxxxxxxx> wrote:
> On Mon, 2022-05-09 at 17:05 +0300, Jani Nikula wrote:
>> On Mon, 09 May 2022, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
>> > On 05/05/2022 20:35, José Roberto de Souza wrote:
>> > > No need to have this parameter in intel_device_info struct
>> > > as all platforms with display version 9 or newer, haswell or broadwell
>> > > supports it.
>> > >
>> > > 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.
>> > >
>> > > Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
>> > > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
>> > > ---
>> > > 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 5538564bc1d25..600d8cee272da 100644
>> > > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > > @@ -1298,7 +1298,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>> > > #define HAS_DP20(dev_priv) (IS_DG2(dev_priv))
>> > >
>> > > #define HAS_CDCLK_CRAWL(dev_priv) (INTEL_INFO(dev_priv)->display.has_cdclk_crawl)
>> > > -#define HAS_DDI(dev_priv) (INTEL_INFO(dev_priv)->display.has_ddi)
>> > > +#define HAS_DDI(dev_priv) (DISPLAY_VER(dev_priv) >= 9 || \
>> > > + IS_BROADWELL(dev_priv) || \
>> > > + IS_HASWELL(dev_priv))
>> >
>> > This one is a bit borderline, not sure it passes Jani's criteria of
>> > simplicity, which I thought was a good one. And from the OCD angle it
>> > kind of sucks to expand the conditionals to all call sites (when it's
>> > even called from i915_irq.c, justifiably or not I don't know).
>> >
>> > What is the high level motivation for this work?
>>
>> Yeah, just don't merge when there's open discussion. Get the acks.
>
> Sorry, I thought that for this ones we were good to go.
>
>>
>> > Also, why is this in drm-intel-gt-next? :)
>>
>> Without the smiley, actually.
>>
>> *ALL* refactoring like this *MUST* go through drm-intel-next.
>
> My understating was that if it was touching GT it should go to
> drm-intel-gt-next.
More like, if you change gt/gem features or functionality or refactor
them in any significant way, they go via drm-intel-gt-next. Stuff that
affects i915 core or display most likely should go via drm-intel-next.
BR,
Jani.
>
>>
>> This is now a source for conflicts for at least 4-6 weeks until we can
>> merge drm-intel-gt-next -> drm-next -> drm-intel-next.
>>
>>
>> BR,
>> Jani.
>>
>>
>> >
>> > Regards,
>> >
>> > Tvrtko
>> >
>> >
>> > > #define HAS_FPGA_DBG_UNCLAIMED(dev_priv) (INTEL_INFO(dev_priv)->display.has_fpga_dbg)
>> > > #define HAS_PSR(dev_priv) (INTEL_INFO(dev_priv)->display.has_psr)
>> > > #define HAS_PSR_HW_TRACKING(dev_priv) \
>> > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> > > index 2dc0284629d30..a0693d9ff9cee 100644
>> > > --- a/drivers/gpu/drm/i915/i915_pci.c
>> > > +++ b/drivers/gpu/drm/i915/i915_pci.c
>> > > @@ -535,7 +535,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_ddi = 1, \
>> > > .display.has_fpga_dbg = 1, \
>> > > .display.has_dp_mst = 1, \
>> > > .has_rc6p = 0 /* RC6p removed-by HSW */, \
>> > > @@ -683,7 +682,6 @@ static const struct intel_device_info skl_gt4_info = {
>> > > BIT(TRANSCODER_C) | BIT(TRANSCODER_EDP) | \
>> > > BIT(TRANSCODER_DSI_A) | BIT(TRANSCODER_DSI_C), \
>> > > .has_64bit_reloc = 1, \
>> > > - .display.has_ddi = 1, \
>> > > .display.has_fpga_dbg = 1, \
>> > > .display.fbc_mask = BIT(INTEL_FBC_A), \
>> > > .display.has_hdcp = 1, \
>> > > @@ -932,7 +930,6 @@ static const struct intel_device_info adl_s_info = {
>> > > .dbuf.size = 4096, \
>> > > .dbuf.slice_mask = BIT(DBUF_S1) | BIT(DBUF_S2) | BIT(DBUF_S3) | \
>> > > BIT(DBUF_S4), \
>> > > - .display.has_ddi = 1, \
>> > > .display.has_dmc = 1, \
>> > > .display.has_dp_mst = 1, \
>> > > .display.has_dsb = 1, \
>> > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
>> > > index bef65e3f02c55..bc71ce48763ad 100644
>> > > --- a/drivers/gpu/drm/i915/intel_device_info.h
>> > > +++ b/drivers/gpu/drm/i915/intel_device_info.h
>> > > @@ -167,7 +167,6 @@ enum intel_ppgtt_type {
>> > > func(cursor_needs_physical); \
>> > > func(has_cdclk_crawl); \
>> > > func(has_dmc); \
>> > > - func(has_ddi); \
>> > > func(has_dp_mst); \
>> > > func(has_dsb); \
>> > > func(has_dsc); \
>>
>
--
Jani Nikula, Intel Open Source Graphics Center
