Message ID | 20210910153627.1060858-5-daniele.ceraolospurio@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/i915: Introduce Intel PXP | expand |
On Fri, 10 Sep 2021, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote: > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h > new file mode 100644 > index 000000000000..e87550fb9821 > --- /dev/null > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h > @@ -0,0 +1,35 @@ > +/* SPDX-License-Identifier: MIT */ > +/* > + * Copyright(c) 2020, Intel Corporation. All rights reserved. > + */ > + > +#ifndef __INTEL_PXP_H__ > +#define __INTEL_PXP_H__ > + > +#include "gt/intel_gt_types.h" I've been trying to promote the idea that we don't include headers from headers, unless really necessary. It helps with build times by reducing rebuilds due to changes, but more importantly, it helps with coming up with abstractions that don't need to look at the guts of other components. The above include line pulls in 67 other includes. And it has to look at the same files a *lot* more times to know not to include them again. Maybe we need to start being more aggressive about hiding the abstractions behind the interfaces and headers. Static inlines are nothing but micro-optimizations that leak abstractions. Do we need these? > +#include "intel_pxp_types.h" > + > +static inline struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp) > +{ > + return container_of(pxp, struct intel_gt, pxp); > +} I think it's questionable to claim the parameter is const, when you can do: const struct intel_pxp *const_pxp = something; struct intel_pxp *pxp = &pxp_to_gt(const_pxp)->pxp; BR, Jani. > + > +static inline bool intel_pxp_is_enabled(const struct intel_pxp *pxp) > +{ > + return pxp->ce; > +} > + > +#ifdef CONFIG_DRM_I915_PXP > +void intel_pxp_init(struct intel_pxp *pxp); > +void intel_pxp_fini(struct intel_pxp *pxp); > +#else > +static inline void intel_pxp_init(struct intel_pxp *pxp) > +{ > +} > + > +static inline void intel_pxp_fini(struct intel_pxp *pxp) > +{ > +} > +#endif > + > +#endif /* __INTEL_PXP_H__ */ > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h > new file mode 100644 > index 000000000000..bd12c520e60a > --- /dev/null > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: MIT */ > +/* > + * Copyright(c) 2020, Intel Corporation. All rights reserved. > + */ > + > +#ifndef __INTEL_PXP_TYPES_H__ > +#define __INTEL_PXP_TYPES_H__ > + > +struct intel_context; > + > +struct intel_pxp { > + struct intel_context *ce; > +}; > + > +#endif /* __INTEL_PXP_TYPES_H__ */
On Wed, Sep 15, 2021 at 04:53:35PM +0300, Jani Nikula wrote: > On Fri, 10 Sep 2021, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote: > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h > > new file mode 100644 > > index 000000000000..e87550fb9821 > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h > > @@ -0,0 +1,35 @@ > > +/* SPDX-License-Identifier: MIT */ > > +/* > > + * Copyright(c) 2020, Intel Corporation. All rights reserved. > > + */ > > + > > +#ifndef __INTEL_PXP_H__ > > +#define __INTEL_PXP_H__ > > + > > +#include "gt/intel_gt_types.h" > > I've been trying to promote the idea that we don't include headers from > headers, unless really necessary. It helps with build times by reducing > rebuilds due to changes, but more importantly, it helps with coming up > with abstractions that don't need to look at the guts of other > components. > > The above include line pulls in 67 other includes. And it has to look at > the same files a *lot* more times to know not to include them again. > > Maybe we need to start being more aggressive about hiding the > abstractions behind the interfaces and headers. Static inlines are > nothing but micro-optimizations that leak abstractions. Do we need > these? Yeap, we have a few cases where this is already happening... Should we start using the container_of more directly and avoid the a_to_b() helpers? Should we create the a_to_b() helpers only inside .c files like we have in a few other cases? In this pxp case here it looks like using the container of directly is everywhere is better... is this your recommendation? > > > +#include "intel_pxp_types.h" > > + > > +static inline struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp) > > +{ > > + return container_of(pxp, struct intel_gt, pxp); > > +} > > I think it's questionable to claim the parameter is const, when you can > do: > > const struct intel_pxp *const_pxp = something; > struct intel_pxp *pxp = &pxp_to_gt(const_pxp)->pxp; > > BR, > Jani. > > > + > > +static inline bool intel_pxp_is_enabled(const struct intel_pxp *pxp) > > +{ > > + return pxp->ce; > > +} > > + > > +#ifdef CONFIG_DRM_I915_PXP > > +void intel_pxp_init(struct intel_pxp *pxp); > > +void intel_pxp_fini(struct intel_pxp *pxp); > > +#else > > +static inline void intel_pxp_init(struct intel_pxp *pxp) > > +{ > > +} > > + > > +static inline void intel_pxp_fini(struct intel_pxp *pxp) > > +{ > > +} > > +#endif > > + > > +#endif /* __INTEL_PXP_H__ */ > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h > > new file mode 100644 > > index 000000000000..bd12c520e60a > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h > > @@ -0,0 +1,15 @@ > > +/* SPDX-License-Identifier: MIT */ > > +/* > > + * Copyright(c) 2020, Intel Corporation. All rights reserved. > > + */ > > + > > +#ifndef __INTEL_PXP_TYPES_H__ > > +#define __INTEL_PXP_TYPES_H__ > > + > > +struct intel_context; > > + > > +struct intel_pxp { > > + struct intel_context *ce; > > +}; > > + > > +#endif /* __INTEL_PXP_TYPES_H__ */ > > -- > Jani Nikula, Intel Open Source Graphics Center
On Wed, 15 Sep 2021, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > On Wed, Sep 15, 2021 at 04:53:35PM +0300, Jani Nikula wrote: >> On Fri, 10 Sep 2021, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote: >> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h >> > new file mode 100644 >> > index 000000000000..e87550fb9821 >> > --- /dev/null >> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h >> > @@ -0,0 +1,35 @@ >> > +/* SPDX-License-Identifier: MIT */ >> > +/* >> > + * Copyright(c) 2020, Intel Corporation. All rights reserved. >> > + */ >> > + >> > +#ifndef __INTEL_PXP_H__ >> > +#define __INTEL_PXP_H__ >> > + >> > +#include "gt/intel_gt_types.h" >> >> I've been trying to promote the idea that we don't include headers from >> headers, unless really necessary. It helps with build times by reducing >> rebuilds due to changes, but more importantly, it helps with coming up >> with abstractions that don't need to look at the guts of other >> components. >> >> The above include line pulls in 67 other includes. And it has to look at >> the same files a *lot* more times to know not to include them again. >> >> Maybe we need to start being more aggressive about hiding the >> abstractions behind the interfaces and headers. Static inlines are >> nothing but micro-optimizations that leak abstractions. Do we need >> these? > > Yeap, we have a few cases where this is already happening... > > Should we start using the container_of more directly and avoid the a_to_b() > helpers? > > Should we create the a_to_b() helpers only inside .c files like we have > in a few other cases? > > In this pxp case here it looks like using the container of directly is > everywhere is better... is this your recommendation? Either that, or make it a non-inline function that's actually abstracted. Yes, it leads to a function call, but I'm really starting to wonder about the costs of a function call vs. maintainability across the board. Static inlines considered harmful. BR, Jani. > >> >> > +#include "intel_pxp_types.h" >> > + >> > +static inline struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp) >> > +{ >> > + return container_of(pxp, struct intel_gt, pxp); >> > +} >> >> I think it's questionable to claim the parameter is const, when you can >> do: >> >> const struct intel_pxp *const_pxp = something; >> struct intel_pxp *pxp = &pxp_to_gt(const_pxp)->pxp; >> >> BR, >> Jani. >> >> > + >> > +static inline bool intel_pxp_is_enabled(const struct intel_pxp *pxp) >> > +{ >> > + return pxp->ce; >> > +} >> > + >> > +#ifdef CONFIG_DRM_I915_PXP >> > +void intel_pxp_init(struct intel_pxp *pxp); >> > +void intel_pxp_fini(struct intel_pxp *pxp); >> > +#else >> > +static inline void intel_pxp_init(struct intel_pxp *pxp) >> > +{ >> > +} >> > + >> > +static inline void intel_pxp_fini(struct intel_pxp *pxp) >> > +{ >> > +} >> > +#endif >> > + >> > +#endif /* __INTEL_PXP_H__ */ >> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h >> > new file mode 100644 >> > index 000000000000..bd12c520e60a >> > --- /dev/null >> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h >> > @@ -0,0 +1,15 @@ >> > +/* SPDX-License-Identifier: MIT */ >> > +/* >> > + * Copyright(c) 2020, Intel Corporation. All rights reserved. >> > + */ >> > + >> > +#ifndef __INTEL_PXP_TYPES_H__ >> > +#define __INTEL_PXP_TYPES_H__ >> > + >> > +struct intel_context; >> > + >> > +struct intel_pxp { >> > + struct intel_context *ce; >> > +}; >> > + >> > +#endif /* __INTEL_PXP_TYPES_H__ */ >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
On Thu, Sep 16, 2021 at 02:06:56PM +0300, Jani Nikula wrote: > On Wed, 15 Sep 2021, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > > On Wed, Sep 15, 2021 at 04:53:35PM +0300, Jani Nikula wrote: > >> On Fri, 10 Sep 2021, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote: > >> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h > >> > new file mode 100644 > >> > index 000000000000..e87550fb9821 > >> > --- /dev/null > >> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h > >> > @@ -0,0 +1,35 @@ > >> > +/* SPDX-License-Identifier: MIT */ > >> > +/* > >> > + * Copyright(c) 2020, Intel Corporation. All rights reserved. > >> > + */ > >> > + > >> > +#ifndef __INTEL_PXP_H__ > >> > +#define __INTEL_PXP_H__ > >> > + > >> > +#include "gt/intel_gt_types.h" > >> > >> I've been trying to promote the idea that we don't include headers from > >> headers, unless really necessary. It helps with build times by reducing > >> rebuilds due to changes, but more importantly, it helps with coming up > >> with abstractions that don't need to look at the guts of other > >> components. > >> > >> The above include line pulls in 67 other includes. And it has to look at > >> the same files a *lot* more times to know not to include them again. > >> > >> Maybe we need to start being more aggressive about hiding the > >> abstractions behind the interfaces and headers. Static inlines are > >> nothing but micro-optimizations that leak abstractions. Do we need > >> these? > > > > Yeap, we have a few cases where this is already happening... > > > > Should we start using the container_of more directly and avoid the a_to_b() > > helpers? > > > > Should we create the a_to_b() helpers only inside .c files like we have > > in a few other cases? > > > > In this pxp case here it looks like using the container of directly is > > everywhere is better... is this your recommendation? > > Either that, or make it a non-inline function that's actually > abstracted. Yes, it leads to a function call, but I'm really starting to > wonder about the costs of a function call vs. maintainability across the > board. Alan, could you please address this? With that addressed and CI happy we will merge to the already created topic/pxp and do the proper pull requests. if your change only touches this patch don't need to resend the whole series but just in-reply-to. But if changes everything to container-of please resend it entirely. Thanks, Rodrigo. > > Static inlines considered harmful. > > > BR, > Jani. > > > > > >> > >> > +#include "intel_pxp_types.h" > >> > + > >> > +static inline struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp) > >> > +{ > >> > + return container_of(pxp, struct intel_gt, pxp); > >> > +} > >> > >> I think it's questionable to claim the parameter is const, when you can > >> do: > >> > >> const struct intel_pxp *const_pxp = something; > >> struct intel_pxp *pxp = &pxp_to_gt(const_pxp)->pxp; > >> > >> BR, > >> Jani. > >> > >> > + > >> > +static inline bool intel_pxp_is_enabled(const struct intel_pxp *pxp) > >> > +{ > >> > + return pxp->ce; > >> > +} > >> > + > >> > +#ifdef CONFIG_DRM_I915_PXP > >> > +void intel_pxp_init(struct intel_pxp *pxp); > >> > +void intel_pxp_fini(struct intel_pxp *pxp); > >> > +#else > >> > +static inline void intel_pxp_init(struct intel_pxp *pxp) > >> > +{ > >> > +} > >> > + > >> > +static inline void intel_pxp_fini(struct intel_pxp *pxp) > >> > +{ > >> > +} > >> > +#endif > >> > + > >> > +#endif /* __INTEL_PXP_H__ */ > >> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h > >> > new file mode 100644 > >> > index 000000000000..bd12c520e60a > >> > --- /dev/null > >> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h > >> > @@ -0,0 +1,15 @@ > >> > +/* SPDX-License-Identifier: MIT */ > >> > +/* > >> > + * Copyright(c) 2020, Intel Corporation. All rights reserved. > >> > + */ > >> > + > >> > +#ifndef __INTEL_PXP_TYPES_H__ > >> > +#define __INTEL_PXP_TYPES_H__ > >> > + > >> > +struct intel_context; > >> > + > >> > +struct intel_pxp { > >> > + struct intel_context *ce; > >> > +}; > >> > + > >> > +#endif /* __INTEL_PXP_TYPES_H__ */ > >> > >> -- > >> Jani Nikula, Intel Open Source Graphics Center > > -- > Jani Nikula, Intel Open Source Graphics Center
On Thu, 2021-09-16 at 09:59 -0400, Rodrigo Vivi wrote: > On Thu, Sep 16, 2021 at 02:06:56PM +0300, Jani Nikula wrote: > > On Wed, 15 Sep 2021, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > > > On Wed, Sep 15, 2021 at 04:53:35PM +0300, Jani Nikula wrote: > > > > On Fri, 10 Sep 2021, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote: > > > > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h > > > > > new file mode 100644 > > > > > index 000000000000..e87550fb9821 > > > > > --- /dev/null > > > > > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h > > > > > @@ -0,0 +1,35 @@ > > > > > +/* SPDX-License-Identifier: MIT */ > > > > > +/* > > > > > + * Copyright(c) 2020, Intel Corporation. All rights reserved. > > > > > + */ > > > > > + > > > > > +#ifndef __INTEL_PXP_H__ > > > > > +#define __INTEL_PXP_H__ > > > > > + > > > > > +#include "gt/intel_gt_types.h" > > > > > > > > I've been trying to promote the idea that we don't include headers from > > > > headers, unless really necessary. It helps with build times by reducing > > > > rebuilds due to changes, but more importantly, it helps with coming up > > > > with abstractions that don't need to look at the guts of other > > > > components. > > > > > > > > The above include line pulls in 67 other includes. And it has to look at > > > > the same files a *lot* more times to know not to include them again. > > > > > > > > Maybe we need to start being more aggressive about hiding the > > > > abstractions behind the interfaces and headers. Static inlines are > > > > nothing but micro-optimizations that leak abstractions. Do we need > > > > these? > > > > > > Yeap, we have a few cases where this is already happening... > > > > > > Should we start using the container_of more directly and avoid the a_to_b() > > > helpers? > > > > > > Should we create the a_to_b() helpers only inside .c files like we have > > > in a few other cases? > > > > > > In this pxp case here it looks like using the container of directly is > > > everywhere is better... is this your recommendation? > > > > Either that, or make it a non-inline function that's actually > > abstracted. Yes, it leads to a function call, but I'm really starting to > > wonder about the costs of a function call vs. maintainability across the > > board. > > Alan, could you please address this? With that addressed and CI happy > we will merge to the already created topic/pxp and do the proper > pull requests. > > if your change only touches this patch don't need to resend the whole > series but just in-reply-to. But if changes everything to container-of > please resend it entirely. > > Thanks, > Rodrigo. > Yes - as you are aware, I have something ready to publish but it touches multiple patches (as function get introduced and modified across multiple patches). The approach i am taking is to make them function prototypes (except when CONFG_PXP is off as they are can be inline stubs). I just have more clean ups + testing to do. > > Static inlines considered harmful. > > > > > > BR, > > Jani. > > > > > > > > > +#include "intel_pxp_types.h" > > > > > + > > > > > +static inline struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp) > > > > > +{ > > > > > + return container_of(pxp, struct intel_gt, pxp); > > > > > +} > > > > > > > > I think it's questionable to claim the parameter is const, when you can > > > > do: > > > > > > > > const struct intel_pxp *const_pxp = something; > > > > struct intel_pxp *pxp = &pxp_to_gt(const_pxp)->pxp; > > > > > > > > BR, > > > > Jani. > > > > > > > > > + > > > > > +static inline bool intel_pxp_is_enabled(const struct intel_pxp *pxp) > > > > > +{ > > > > > + return pxp->ce; > > > > > +} > > > > > + > > > > > +#ifdef CONFIG_DRM_I915_PXP > > > > > +void intel_pxp_init(struct intel_pxp *pxp); > > > > > +void intel_pxp_fini(struct intel_pxp *pxp); > > > > > +#else > > > > > +static inline void intel_pxp_init(struct intel_pxp *pxp) > > > > > +{ > > > > > +} > > > > > + > > > > > +static inline void intel_pxp_fini(struct intel_pxp *pxp) > > > > > +{ > > > > > +} > > > > > +#endif > > > > > + > > > > > +#endif /* __INTEL_PXP_H__ */ > > > > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h > > > > > new file mode 100644 > > > > > index 000000000000..bd12c520e60a > > > > > --- /dev/null > > > > > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h > > > > > @@ -0,0 +1,15 @@ > > > > > +/* SPDX-License-Identifier: MIT */ > > > > > +/* > > > > > + * Copyright(c) 2020, Intel Corporation. All rights reserved. > > > > > + */ > > > > > + > > > > > +#ifndef __INTEL_PXP_TYPES_H__ > > > > > +#define __INTEL_PXP_TYPES_H__ > > > > > + > > > > > +struct intel_context; > > > > > + > > > > > +struct intel_pxp { > > > > > + struct intel_context *ce; > > > > > +}; > > > > > + > > > > > +#endif /* __INTEL_PXP_TYPES_H__ */ > > > > > > > > -- > > > > Jani Nikula, Intel Open Source Graphics Center > > > > -- > > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index c36c8a4f0716..23f5bc268962 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -281,6 +281,10 @@ i915-y += \ i915-y += i915_perf.o +# Protected execution platform (PXP) support +i915-$(CONFIG_DRM_I915_PXP) += \ + pxp/intel_pxp.o + # Post-mortem debug and GPU hang state capture i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o i915-$(CONFIG_DRM_I915_SELFTEST) += \ diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index 87579affb952..eed4634c08cd 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -175,6 +175,8 @@ intel_write_status_page(struct intel_engine_cs *engine, int reg, u32 value) #define I915_GEM_HWS_SEQNO 0x40 #define I915_GEM_HWS_SEQNO_ADDR (I915_GEM_HWS_SEQNO * sizeof(u32)) #define I915_GEM_HWS_MIGRATE (0x42 * sizeof(u32)) +#define I915_GEM_HWS_PXP 0x60 +#define I915_GEM_HWS_PXP_ADDR (I915_GEM_HWS_PXP * sizeof(u32)) #define I915_GEM_HWS_SCRATCH 0x80 #define I915_HWS_CSB_BUF0_INDEX 0x10 diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 2aeaae036a6f..da30919b7e99 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -21,6 +21,7 @@ #include "intel_uncore.h" #include "intel_pm.h" #include "shmem_utils.h" +#include "pxp/intel_pxp.h" void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) { @@ -712,6 +713,8 @@ int intel_gt_init(struct intel_gt *gt) intel_migrate_init(>->migrate, gt); + intel_pxp_init(>->pxp); + goto out_fw; err_gt: __intel_gt_disable(gt); @@ -747,6 +750,8 @@ void intel_gt_driver_unregister(struct intel_gt *gt) intel_rps_driver_unregister(>->rps); + intel_pxp_fini(>->pxp); + /* * Upon unregistering the device to prevent any new users, cancel * all in-flight requests so that we can quickly unbind the active diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index 6fdcde64c180..8001a61f42e5 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -26,6 +26,7 @@ #include "intel_rps_types.h" #include "intel_migrate_types.h" #include "intel_wakeref.h" +#include "pxp/intel_pxp_types.h" struct drm_i915_private; struct i915_ggtt; @@ -196,6 +197,8 @@ struct intel_gt { struct { u8 uc_index; } mocs; + + struct intel_pxp pxp; }; enum intel_gt_scratch_field { diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c new file mode 100644 index 000000000000..7b2053902146 --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright(c) 2020 Intel Corporation. + */ +#include "intel_pxp.h" +#include "gt/intel_context.h" +#include "i915_drv.h" + +static int create_vcs_context(struct intel_pxp *pxp) +{ + static struct lock_class_key pxp_lock; + struct intel_gt *gt = pxp_to_gt(pxp); + struct intel_engine_cs *engine; + struct intel_context *ce; + + /* + * Find the first VCS engine present. We're guaranteed there is one + * if we're in this function due to the check in has_pxp + */ + for (engine = gt->engine_class[VIDEO_DECODE_CLASS][0]; !engine; engine++); + GEM_BUG_ON(!engine || engine->class != VIDEO_DECODE_CLASS); + + ce = intel_engine_create_pinned_context(engine, engine->gt->vm, SZ_4K, + I915_GEM_HWS_PXP_ADDR, + &pxp_lock, "pxp_context"); + if (IS_ERR(ce)) { + drm_err(>->i915->drm, "failed to create VCS ctx for PXP\n"); + return PTR_ERR(ce); + } + + pxp->ce = ce; + + return 0; +} + +static void destroy_vcs_context(struct intel_pxp *pxp) +{ + intel_engine_destroy_pinned_context(fetch_and_zero(&pxp->ce)); +} + +void intel_pxp_init(struct intel_pxp *pxp) +{ + struct intel_gt *gt = pxp_to_gt(pxp); + int ret; + + if (!HAS_PXP(gt->i915)) + return; + + ret = create_vcs_context(pxp); + if (ret) + return; + + drm_info(>->i915->drm, "Protected Xe Path (PXP) protected content support initialized\n"); +} + +void intel_pxp_fini(struct intel_pxp *pxp) +{ + if (!intel_pxp_is_enabled(pxp)) + return; + + destroy_vcs_context(pxp); +} diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h new file mode 100644 index 000000000000..e87550fb9821 --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h @@ -0,0 +1,35 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright(c) 2020, Intel Corporation. All rights reserved. + */ + +#ifndef __INTEL_PXP_H__ +#define __INTEL_PXP_H__ + +#include "gt/intel_gt_types.h" +#include "intel_pxp_types.h" + +static inline struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp) +{ + return container_of(pxp, struct intel_gt, pxp); +} + +static inline bool intel_pxp_is_enabled(const struct intel_pxp *pxp) +{ + return pxp->ce; +} + +#ifdef CONFIG_DRM_I915_PXP +void intel_pxp_init(struct intel_pxp *pxp); +void intel_pxp_fini(struct intel_pxp *pxp); +#else +static inline void intel_pxp_init(struct intel_pxp *pxp) +{ +} + +static inline void intel_pxp_fini(struct intel_pxp *pxp) +{ +} +#endif + +#endif /* __INTEL_PXP_H__ */ diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h new file mode 100644 index 000000000000..bd12c520e60a --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright(c) 2020, Intel Corporation. All rights reserved. + */ + +#ifndef __INTEL_PXP_TYPES_H__ +#define __INTEL_PXP_TYPES_H__ + +struct intel_context; + +struct intel_pxp { + struct intel_context *ce; +}; + +#endif /* __INTEL_PXP_TYPES_H__ */