Re: [PATCH 02/16] drm/i915: Move intel_fb_pin_params to the parent interface

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

On Thu, 23 Apr 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> strut intel_fb_pin_params will be an important part of the fb_pin
> interface, so move the definition to the parent interface file.
>
> Or maybe we should have a separate header for this kind of stuff
> since the users of the parent interface will need the struct
> definition but not the parent interface vfunc struct definitions?

Yeah, another hard one. I've tried to keep the interface definition
clear of other type definitions, and in particular to use types that are
readily available in generic drm headers. Or opaque pointers.

I think let's see how bad it gets. We're bound to need a few structs
specific to this interface, like struct intel_fb_pin_params. If it's
just a few structs, and they don't need to include the world for their
definition, let's just keep them here. But if it starts to look like
there'll be plenty, and they need a bunch of dependencies, let's split
them out. It's not a big deal to do that later.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fb_pin.h  | 12 +-----------
>  drivers/gpu/drm/i915/display/intel_fbdev.c   |  1 +
>  include/drm/intel/display_parent_interface.h | 12 ++++++++++++
>  3 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.h b/drivers/gpu/drm/i915/display/intel_fb_pin.h
> index 805e23067004..84530f20d7d1 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb_pin.h
> +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.h
> @@ -10,21 +10,11 @@
>  
>  struct drm_gem_object;
>  struct i915_vma;
> +struct intel_fb_pin_params;
>  struct intel_plane_state;
>  struct i915_gtt_view;
>  struct iosys_map;
>  
> -struct intel_fb_pin_params {
> -	const struct i915_gtt_view *view;
> -	unsigned int alignment;
> -	unsigned int phys_alignment;
> -	unsigned int vtd_guard;
> -	bool needs_cpu_lmem_access;
> -	bool needs_low_address;
> -	bool needs_physical;
> -	bool needs_fence;
> -};
> -
>  struct i915_vma *
>  intel_fb_pin_to_ggtt(struct drm_gem_object *obj,
>  		     const struct intel_fb_pin_params *pin_params,
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index ce4082da30e8..aa2701795caa 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -38,6 +38,7 @@
>  #include <linux/vga_switcheroo.h>
>  
>  #include <drm/clients/drm_client_setup.h>
> +#include <drm/intel/display_parent_interface.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_helper.h>
> diff --git a/include/drm/intel/display_parent_interface.h b/include/drm/intel/display_parent_interface.h
> index ee2e9572bfca..b363d6a85dfe 100644
> --- a/include/drm/intel/display_parent_interface.h
> +++ b/include/drm/intel/display_parent_interface.h
> @@ -16,6 +16,7 @@ struct drm_mode_fb_cmd2;
>  struct drm_plane_state;
>  struct drm_scanout_buffer;
>  struct fb_info;
> +struct i915_gtt_view;
>  struct i915_vma;
>  struct intel_dpt;
>  struct intel_dsb_buffer;
> @@ -29,6 +30,17 @@ struct ref_tracker;
>  struct seq_file;
>  struct vm_area_struct;
>  
> +struct intel_fb_pin_params {
> +	const struct i915_gtt_view *view;
> +	unsigned int alignment;
> +	unsigned int phys_alignment;
> +	unsigned int vtd_guard;
> +	bool needs_cpu_lmem_access;
> +	bool needs_low_address;
> +	bool needs_physical;
> +	bool needs_fence;
> +};
> +
>  /* Keep struct definitions sorted */
>  
>  struct intel_display_bo_interface {

-- 
Jani Nikula, Intel