Re: [PATCH 04/12] drm/{i915, xe}: Avoid direct inspection of dpt_vma from outside dpt

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Aug 16, 2024 at 12:16:36AM -0500, Lucas De Marchi wrote:
> On Thu, Aug 15, 2024 at 06:07:24PM GMT, Rodrigo Vivi wrote:
> > DPT code is so dependent on i915 vma implementation and it is not
> > ported yet to Xe.
> > 
> > This patch limits inspection to DPT's VMA struct to intel_dpt
> > component only, so the Xe GGTT code can evolve.
> > 
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_dpt.c           | 4 ++++
> > drivers/gpu/drm/i915/display/intel_dpt.h           | 1 +
> > drivers/gpu/drm/i915/display/skl_universal_plane.c | 3 ++-
> > drivers/gpu/drm/xe/display/xe_fb_pin.c             | 9 +++++++--
> > 4 files changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c b/drivers/gpu/drm/i915/display/intel_dpt.c
> > index 73a1918e2537..3a6d99044828 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpt.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpt.c
> > @@ -317,3 +317,7 @@ void intel_dpt_destroy(struct i915_address_space *vm)
> > 	i915_vm_put(&dpt->vm);
> > }
> > 
> > +u64 intel_dpt_offset(struct i915_vma *dpt_vma)
> > +{
> > +	return dpt_vma->node.start;
> > +}
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpt.h b/drivers/gpu/drm/i915/display/intel_dpt.h
> > index ff18a525bfbe..7336e905142d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpt.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dpt.h
> > @@ -20,5 +20,6 @@ void intel_dpt_suspend(struct drm_i915_private *i915);
> > void intel_dpt_resume(struct drm_i915_private *i915);
> > struct i915_address_space *
> > intel_dpt_create(struct intel_framebuffer *fb);
> > +u64 intel_dpt_offset(struct i915_vma *dpt_vma);
> 
> CI doesn't show the exact line, but build is breaking here:
> 
> ./../drivers/gpu/drm/i915/display/intel_dpt.h:23:1: error: unknown type name ‘u64’
>    23 | u64 intel_dpt_offset(struct i915_vma *dpt_vma);
>       | ^~~
> make[7]: *** [../drivers/gpu/drm/i915/Makefile:419: drivers/gpu/drm/i915/display/intel_dpt.hdrtest] Error 1
> make[6]: *** [../scripts/Makefile.build:485: drivers/gpu/drm/i915] Error 2
> 
> #include <linux/types.h>
> 
> to fix it.

Thanks. I had seen the build failure and thought I was on some wrong base or something
and didn't carefully look that.

> 
> And CONFIG_DRM_XE_WERROR in your .config

The oddest part is that I do have this here.
In both of the machines where I was always building this code
and absolutely no warning of any kind! o.O

> 
> Lucas De Marchi
> 
> > 
> > #endif /* __INTEL_DPT_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index ba5a628b4757..1cf1d5c8b9dc 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -14,6 +14,7 @@
> > #include "intel_de.h"
> > #include "intel_display_irq.h"
> > #include "intel_display_types.h"
> > +#include "intel_dpt.h"
> > #include "intel_fb.h"
> > #include "intel_fbc.h"
> > #include "intel_frontbuffer.h"
> > @@ -1162,7 +1163,7 @@ static u32 skl_surf_address(const struct intel_plane_state *plane_state,
> > 		 * within the DPT is always 0.
> > 		 */
> > 		drm_WARN_ON(&i915->drm, plane_state->dpt_vma &&
> > -			    plane_state->dpt_vma->node.start);
> > +			    intel_dpt_offset(plane_state->dpt_vma));
> > 		drm_WARN_ON(&i915->drm, offset & 0x1fffff);
> > 		return offset >> 9;
> > 	} else {
> > diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > index d7db44e79eaf..42d431ff14e7 100644
> > --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> > @@ -377,8 +377,8 @@ void intel_plane_unpin_fb(struct intel_plane_state *old_plane_state)
> > }
> > 
> > /*
> > - * For Xe introduce dummy intel_dpt_create which just return NULL and
> > - * intel_dpt_destroy which does nothing.
> > + * For Xe introduce dummy intel_dpt_create which just return NULL,
> > + * intel_dpt_destroy which does nothing, and fake intel_dpt_ofsset returning 0;
> >  */
> > struct i915_address_space *intel_dpt_create(struct intel_framebuffer *fb)
> > {
> > @@ -389,3 +389,8 @@ void intel_dpt_destroy(struct i915_address_space *vm)
> > {
> > 	return;
> > }
> > +
> > +u64 intel_dpt_offset(struct i915_vma *dpt_vma)
> > +{
> > +	return 0;
> > +}
> > -- 
> > 2.46.0
> >