Re: [PATCH 4/6] drm/i915/gvt: Change for_each_pipe to use pipe_valid API

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

On 12/19/2025 6:04 PM, Jani Nikula wrote:
> On Fri, 19 Dec 2025, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> Add a new API to check if a given pipe is valid using
>> DISPLAY_RUNTIME_INFO() for GVT.
>>
>> Update GVT to use this API instead of accessing
>> `DISPLAY_RUNTIME_INFO->pipe_mask` directly in the `for_each_pipe` macro.
>>
>> Since `for_each_pipe` is defined in i915/display/intel_display.h, which
>> also contains other macros used by gvt/display.c, we cannot drop the
>> intel_display.h header yet. This causes a build error because
>> `for_each_pipe` is included from both i915/display/intel_display.h and
>> gvt/display_helpers.h.
>>
>> To resolve this, rename the GVT macro to `gvt_for_each_pipe` and make it
>> call the new API. This avoids exposing display internals and prepares for
>> display modularization.
> I'm fine with the separate name for this, to underline the difference.
>
> However, I think in the long run gvt should include fewer things from
> display/. This may mean refactoring display headers a bit to limit the
> exposure. Like, there's current intel_display_types.h and
> intel_display.h includes, and we shouldn't have them, but it's beyond
> this series.

I agree.. I have realized this while dealing with the problem with 
for_each_pipe.

display/intel_display_core.h was one such file, now that is removed.

Anyways, will get some things started on intel_gvt_mmio_table.c stuff next.


Thanks for the reviews!

Regards,

Ankit


>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
>> v2:
>>   - Expose API to check if pipe is valid rather than the runtime info
>>     pipe mask. (Jani)
>>   - Rename the macro to `gvt_for_each_pipe` to resolve build error.
>> v3:
>>   - Use EXPORT_SYMBOL_NS_GPL(..., "I915_GVT"); (Jani)
>>   - Use enum pipe at call sites instead of casting in the macro. (Jani)
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_gvt_api.c |  9 +++++++++
>>   drivers/gpu/drm/i915/display/intel_gvt_api.h |  1 +
>>   drivers/gpu/drm/i915/gvt/display.c           | 10 +++++-----
>>   drivers/gpu/drm/i915/gvt/display_helpers.h   |  4 ++++
>>   4 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_gvt_api.c b/drivers/gpu/drm/i915/display/intel_gvt_api.c
>> index b1bfe4843135..a69e249395ae 100644
>> --- a/drivers/gpu/drm/i915/display/intel_gvt_api.c
>> +++ b/drivers/gpu/drm/i915/display/intel_gvt_api.c
>> @@ -32,3 +32,12 @@ u32 intel_display_device_mmio_base(struct intel_display *display)
>>   	return DISPLAY_MMIO_BASE(display);
>>   }
>>   EXPORT_SYMBOL_NS_GPL(intel_display_device_mmio_base, "I915_GVT");
>> +
>> +bool intel_display_device_pipe_valid(struct intel_display *display, enum pipe pipe)
>> +{
>> +	if (pipe < PIPE_A || pipe >= I915_MAX_PIPES)
>> +		return false;
>> +
>> +	return DISPLAY_RUNTIME_INFO(display)->pipe_mask & BIT(pipe);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(intel_display_device_pipe_valid, "I915_GVT");
>> diff --git a/drivers/gpu/drm/i915/display/intel_gvt_api.h b/drivers/gpu/drm/i915/display/intel_gvt_api.h
>> index e9a1122a988d..a53687f7d934 100644
>> --- a/drivers/gpu/drm/i915/display/intel_gvt_api.h
>> +++ b/drivers/gpu/drm/i915/display/intel_gvt_api.h
>> @@ -16,5 +16,6 @@ u32 intel_display_device_pipe_offset(struct intel_display *display, enum pipe pi
>>   u32 intel_display_device_trans_offset(struct intel_display *display, enum transcoder trans);
>>   u32 intel_display_device_cursor_offset(struct intel_display *display, enum pipe pipe);
>>   u32 intel_display_device_mmio_base(struct intel_display *display);
>> +bool intel_display_device_pipe_valid(struct intel_display *display, enum pipe pipe);
>>   
>>   #endif /* __INTEL_GVT_API_H__ */
>> diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
>> index 9d6b22b2e4d0..1d0c581a8ccc 100644
>> --- a/drivers/gpu/drm/i915/gvt/display.c
>> +++ b/drivers/gpu/drm/i915/gvt/display.c
>> @@ -188,7 +188,7 @@ static void emulate_monitor_status_change(struct intel_vgpu *vgpu)
>>   {
>>   	struct drm_i915_private *dev_priv = vgpu->gvt->gt->i915;
>>   	struct intel_display *display = dev_priv->display;
>> -	int pipe;
>> +	enum pipe pipe;
>>   
>>   	if (IS_BROXTON(dev_priv)) {
>>   		enum transcoder trans;
>> @@ -200,7 +200,7 @@ static void emulate_monitor_status_change(struct intel_vgpu *vgpu)
>>   			  GEN8_DE_PORT_HOTPLUG(HPD_PORT_B) |
>>   			  GEN8_DE_PORT_HOTPLUG(HPD_PORT_C));
>>   
>> -		for_each_pipe(display, pipe) {
>> +		gvt_for_each_pipe(display, pipe) {
>>   			vgpu_vreg_t(vgpu, TRANSCONF(display, pipe)) &=
>>   				~(TRANSCONF_ENABLE | TRANSCONF_STATE_ENABLE);
>>   			vgpu_vreg_t(vgpu, DSPCNTR(display, pipe)) &= ~DISP_ENABLE;
>> @@ -516,7 +516,7 @@ static void emulate_monitor_status_change(struct intel_vgpu *vgpu)
>>   		vgpu_vreg_t(vgpu, PCH_ADPA) &= ~ADPA_CRT_HOTPLUG_MONITOR_MASK;
>>   
>>   	/* Disable Primary/Sprite/Cursor plane */
>> -	for_each_pipe(display, pipe) {
>> +	gvt_for_each_pipe(display, pipe) {
>>   		vgpu_vreg_t(vgpu, DSPCNTR(display, pipe)) &= ~DISP_ENABLE;
>>   		vgpu_vreg_t(vgpu, SPRCTL(pipe)) &= ~SPRITE_ENABLE;
>>   		vgpu_vreg_t(vgpu, CURCNTR(display, pipe)) &= ~MCURSOR_MODE_MASK;
>> @@ -669,10 +669,10 @@ void intel_vgpu_emulate_vblank(struct intel_vgpu *vgpu)
>>   {
>>   	struct drm_i915_private *i915 = vgpu->gvt->gt->i915;
>>   	struct intel_display *display = i915->display;
>> -	int pipe;
>> +	enum pipe pipe;
>>   
>>   	mutex_lock(&vgpu->vgpu_lock);
>> -	for_each_pipe(display, pipe)
>> +	gvt_for_each_pipe(display, pipe)
>>   		emulate_vblank_on_pipe(vgpu, pipe);
>>   	mutex_unlock(&vgpu->vgpu_lock);
>>   }
>> diff --git a/drivers/gpu/drm/i915/gvt/display_helpers.h b/drivers/gpu/drm/i915/gvt/display_helpers.h
>> index 46c5192a79a7..7c6e15aa280a 100644
>> --- a/drivers/gpu/drm/i915/gvt/display_helpers.h
>> +++ b/drivers/gpu/drm/i915/gvt/display_helpers.h
>> @@ -42,4 +42,8 @@
>>   #define INTEL_DISPLAY_DEVICE_CURSOR_OFFSET(display, pipe) \
>>   	intel_display_device_cursor_offset((display), (pipe))
>>   
>> +#define gvt_for_each_pipe(display, __p) \
>> +	for ((__p) = PIPE_A; (__p) < I915_MAX_PIPES; (__p)++) \
>> +		for_each_if(intel_display_device_pipe_valid((display), (__p)))
>> +
>>   #endif /* __DISPLAY_HELPERS_H__ */