Re: [PATCH 3/5] drm/i915/gvt: Add header to use display offset functions in macros

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 12/15/2025 5:23 PM, Jani Nikula wrote:
> On Mon, 15 Dec 2025, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> Introduce gvt/display_helpers.h to make DISPLAY_MMIO_BASE and
>> INTEL_DISPLAY_DEVICE_*_OFFSET macros call exported display functions.
>> This lets GVT keep using existing register macros (e.g.,
>> TRANSCONF(display, pipe)) while ensuring offset calculations happen
>> through functions instead of accessing display internals.
>>
>> Include gvt/display_helpers.h after display headers to avoid
>> conflicts.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gvt/cmd_parser.c      |  2 ++
>>   drivers/gpu/drm/i915/gvt/display.c         |  2 ++
>>   drivers/gpu/drm/i915/gvt/display_helpers.h | 39 ++++++++++++++++++++++
>>   drivers/gpu/drm/i915/gvt/fb_decoder.c      |  2 ++
>>   drivers/gpu/drm/i915/gvt/handlers.c        |  2 ++
>>   5 files changed, 47 insertions(+)
>>   create mode 100644 drivers/gpu/drm/i915/gvt/display_helpers.h
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
>> index df04e4ead8ea..6b5e18fca403 100644
>> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
>> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
>> @@ -59,6 +59,8 @@
>>   #include "gem/i915_gem_pm.h"
>>   #include "gt/intel_context.h"
>>   
>> +#include "gvt/display_helpers.h"
> None of these includes need the gvt/ prefix as they're in the same
> subdirectory.
Alright, will drop the gvt/ prefix.
>
>> +
>>   #define INVALID_OP    (~0U)
>>   
>>   #define OP_LEN_MI           9
>> diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
>> index 06517d1f07a2..7a51c13b9b58 100644
>> --- a/drivers/gpu/drm/i915/gvt/display.c
>> +++ b/drivers/gpu/drm/i915/gvt/display.c
>> @@ -49,6 +49,8 @@
>>   #include "display/intel_dpio_phy.h"
>>   #include "display/intel_sprite_regs.h"
>>   
>> +#include "gvt/display_helpers.h"
>> +
>>   static int get_edp_pipe(struct intel_vgpu *vgpu)
>>   {
>>   	u32 data = vgpu_vreg(vgpu, _TRANS_DDI_FUNC_CTL_EDP);
>> diff --git a/drivers/gpu/drm/i915/gvt/display_helpers.h b/drivers/gpu/drm/i915/gvt/display_helpers.h
>> new file mode 100644
>> index 000000000000..6f68a1e8751a
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/gvt/display_helpers.h
>> @@ -0,0 +1,39 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2025 Intel Corporation
>> + */
>> +
>> +#ifndef __DISPLAY_HELPERS_H__
>> +#define __DISPLAY_HELPERS_H__
>> +
>> +#include "display/intel_gvt_api.h"
>> +
>> +enum pipe;
>> +enum trans;
>> +struct display;
>> +
>> +#ifdef DISPLAY_MMIO_BASE
> Ideally, we shouldn't need these ifdefs. I think it's better if we can
> drop any includes from gvt that would cause a conflict here, and it's
> better to get the build failure.
>
> If we can't do that in this patch already, then please drop the relevant
> includes and these ifdefs at the end of the series.

Ok, let me re-check this part.


Regards,

Ankit

>
> BR,
> Jani.
>
>> +#undef DISPLAY_MMIO_BASE
>> +#endif
>> +#define DISPLAY_MMIO_BASE(display) \
>> +	intel_display_device_mmio_base((display))
>> +
>> +#ifdef INTEL_DISPLAY_DEVICE_PIPE_OFFSET
>> +#undef INTEL_DISPLAY_DEVICE_PIPE_OFFSET
>> +#endif
>> +#define INTEL_DISPLAY_DEVICE_PIPE_OFFSET(display, pipe) \
>> +	intel_display_device_pipe_offset((display), (pipe))
>> +
>> +#ifdef INTEL_DISPLAY_DEVICE_TRANS_OFFSET
>> +#undef INTEL_DISPLAY_DEVICE_TRANS_OFFSET
>> +#endif
>> +#define INTEL_DISPLAY_DEVICE_TRANS_OFFSET(display, trans) \
>> +	intel_display_device_trans_offset((display), (trans))
>> +
>> +#ifdef INTEL_DISPLAY_DEVICE_CURSOR_OFFSET
>> +#undef INTEL_DISPLAY_DEVICE_CURSOR_OFFSET
>> +#endif
>> +#define INTEL_DISPLAY_DEVICE_CURSOR_OFFSET(display, pipe) \
>> +	intel_display_device_cursor_offset((display), (pipe))
>> +
>> +#endif /* __DISPLAY_HELPERS_H__ */
>> diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c b/drivers/gpu/drm/i915/gvt/fb_decoder.c
>> index a8079cfa8e1d..ee4213fa2cda 100644
>> --- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
>> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
>> @@ -47,6 +47,8 @@
>>   #include "display/intel_sprite_regs.h"
>>   #include "display/skl_universal_plane_regs.h"
>>   
>> +#include "gvt/display_helpers.h"
>> +
>>   #define PRIMARY_FORMAT_NUM	16
>>   struct pixel_format {
>>   	int drm_format;	/* Pixel format in DRM definition */
>> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
>> index 36ea12ade849..3e58e35ea2b9 100644
>> --- a/drivers/gpu/drm/i915/gvt/handlers.c
>> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
>> @@ -67,6 +67,8 @@
>>   #include "gt/intel_gt_regs.h"
>>   #include <linux/vmalloc.h>
>>   
>> +#include "gvt/display_helpers.h"
>> +
>>   /* XXX FIXME i915 has changed PP_XXX definition */
>>   #define PCH_PP_STATUS  _MMIO(0xc7200)
>>   #define PCH_PP_CONTROL _MMIO(0xc7204)