Re: [Intel-xe] [PATCH 5/6] drm/xe: Build soc files directly

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 31 Mar 2023, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
> On 2023-03-31 13:49, Jani Nikula wrote:
>> On Fri, 31 Mar 2023, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
>>> Instead of making a copy that is hard to keep up to date, build the
>>> source directly.
>>>
>>> Add a rule for soc_* files, and use those to build i915.
>>>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/soc/intel_dram.c         |  30 +-
>>>   drivers/gpu/drm/xe/Makefile                   |   8 +-
>>>   .../gpu/drm/xe/compat-i915-headers/i915_drv.h |   2 +-
>>>   .../soc}/intel_dram.h                         |   1 -
>>>   .../soc}/intel_pch.h                          |   0
>>>   drivers/gpu/drm/xe/display/ext/intel_dram.c   | 495 ------------------
>>>   drivers/gpu/drm/xe/display/ext/intel_pch.c    | 157 ------
>>>   drivers/gpu/drm/xe/xe_device_types.h          |   2 +-
>>>   drivers/gpu/drm/xe/xe_display.c               |   2 +-
>>>   9 files changed, 28 insertions(+), 669 deletions(-)
>>>   rename drivers/gpu/drm/xe/{display/ext => compat-i915-headers/soc}/intel_dram.h (80%)
>>>   rename drivers/gpu/drm/xe/{display/ext => compat-i915-headers/soc}/intel_pch.h (100%)
>>>   delete mode 100644 drivers/gpu/drm/xe/display/ext/intel_dram.c
>>>   delete mode 100644 drivers/gpu/drm/xe/display/ext/intel_pch.c
>>>
>>> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
>>> index 9649051ed8ed..13fe18a9372f 100644
>>> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
>>> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
>>> @@ -9,8 +9,12 @@
>>>   #include "i915_reg.h"
>>>   #include "intel_dram.h"
>>>   #include "intel_mchbar_regs.h"
>>> -#include "intel_pcode.h"
>>> +#ifdef I915
>>>   #include "vlv_sideband.h"
>>> +#include "display/intel_de.h"
>>> +#else
>>> +#include "xe_de.h"
>>> +#endif
>>>   
>>>   struct dram_dimm_info {
>>>   	u16 size;
>>> @@ -43,6 +47,8 @@ static const char *intel_dram_type_str(enum intel_dram_type type)
>>>   
>>>   #undef DRAM_TYPE_STR
>>>   
>>> +#ifdef I915
>>> +
>>>   static void pnv_detect_mem_freq(struct drm_i915_private *dev_priv)
>>>   {
>>>   	u32 tmp;
>>> @@ -191,6 +197,9 @@ static void detect_mem_freq(struct drm_i915_private *i915)
>>>   	if (i915->mem_freq)
>>>   		drm_dbg(&i915->drm, "DDR speed: %d MHz\n", i915->mem_freq);
>>>   }
>>> +#else
>>> +#define detect_mem_freq(i915) do { } while (0)
>>> +#endif
>>>   
>>>   static int intel_dimm_num_devices(const struct dram_dimm_info *dimm)
>>>   {
>>> @@ -339,14 +348,12 @@ skl_dram_get_channels_info(struct drm_i915_private *i915)
>>>   	u32 val;
>>>   	int ret;
>>>   
>>> -	val = intel_uncore_read(&i915->uncore,
>>> -				SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
>>> +	val = intel_de_read(i915, SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
>> This isn't great, because those aren't DE registers. So this won't fly
>> upstream.
>
> Could we do this for xe instead?
>
> #define intel_uncore_read(ignore, reg) xe_mmio_read32(i915, reg) ?

The obvious downside is that it relies on the implict variable i915,
which could also be named something else, and it has taken a long, long
time trying to fix those in i915.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center