| Message ID | 20210710012026.19705-7-vinay.belgaumkar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable GuC based power management features | expand |
On 10.07.2021 03:20, Vinay Belgaumkar wrote: > Allocate data structures for SLPC and functions for > initializing on host side. > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > Signed-off-by: Sundaresan Sujaritha <sujaritha.sundaresan@intel.com> > --- > drivers/gpu/drm/i915/gt/uc/intel_guc.c | 11 +++++++ > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 36 ++++++++++++++++++++- > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 20 ++++++++++++ > 3 files changed, 66 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c > index 9d61b2d54de4..82863a9bc8e8 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c > @@ -336,6 +336,12 @@ int intel_guc_init(struct intel_guc *guc) > goto err_ct; > } > > + if (intel_guc_slpc_is_used(guc)) { > + ret = intel_guc_slpc_init(&guc->slpc); > + if (ret) > + goto err_submission; > + } > + > /* now that everything is perma-pinned, initialize the parameters */ > guc_init_params(guc); > > @@ -346,6 +352,8 @@ int intel_guc_init(struct intel_guc *guc) > > return 0; > > +err_submission: > + intel_guc_submission_fini(guc); > err_ct: > intel_guc_ct_fini(&guc->ct); > err_ads: > @@ -368,6 +376,9 @@ void intel_guc_fini(struct intel_guc *guc) > > i915_ggtt_disable_guc(gt->ggtt); > > + if (intel_guc_slpc_is_used(guc)) > + intel_guc_slpc_fini(&guc->slpc); > + > if (intel_guc_submission_is_used(guc)) > intel_guc_submission_fini(guc); > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > index c1f569d2300d..94e2f19951aa 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > @@ -4,11 +4,41 @@ > * Copyright © 2020 Intel Corporation > */ > > +#include <asm/msr-index.h> hmm, what exactly is needed from this header ? > + > +#include "gt/intel_gt.h" > +#include "gt/intel_rps.h" > + > +#include "i915_drv.h" > #include "intel_guc_slpc.h" > +#include "intel_pm.h" > + > +static inline struct intel_guc *slpc_to_guc(struct intel_guc_slpc *slpc) > +{ > + return container_of(slpc, struct intel_guc, slpc); > +} > + > +static int slpc_shared_data_init(struct intel_guc_slpc *slpc) > +{ > + struct intel_guc *guc = slpc_to_guc(slpc); > + int err; > + u32 size = PAGE_ALIGN(sizeof(struct slpc_shared_data)); move err decl here > + > + err = intel_guc_allocate_and_map_vma(guc, size, &slpc->vma, &slpc->vaddr); > + if (unlikely(err)) { > + DRM_ERROR("Failed to allocate slpc struct (err=%d)\n", err); s/slpc/SLPC and use drm_err instead and you may also want to print error as %pe > + i915_vma_unpin_and_release(&slpc->vma, I915_VMA_RELEASE_MAP); do you really need this ? > + return err; > + } > + > + return err; > +} > > int intel_guc_slpc_init(struct intel_guc_slpc *slpc) > { > - return 0; > + GEM_BUG_ON(slpc->vma); > + > + return slpc_shared_data_init(slpc); > } > > /* > @@ -31,4 +61,8 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) > > void intel_guc_slpc_fini(struct intel_guc_slpc *slpc) > { > + if (!slpc->vma) > + return; > + > + i915_vma_unpin_and_release(&slpc->vma, I915_VMA_RELEASE_MAP); > } > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h > index 98036459a1a3..a2643b904165 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h > @@ -3,12 +3,32 @@ > * > * Copyright © 2020 Intel Corporation > */ > + should be fixed in earlier patch > #ifndef _INTEL_GUC_SLPC_H_ > #define _INTEL_GUC_SLPC_H_ > > +#include <linux/mutex.h> > #include "intel_guc_slpc_fwif.h" > > struct intel_guc_slpc { > + /*Protects access to vma and SLPC actions */ hmm, missing mutex ;) > + struct i915_vma *vma; > + void *vaddr; no need to be void, define it as ptr to slpc_shared_data > + > + /* platform frequency limits */ > + u32 min_freq; > + u32 rp0_freq; > + u32 rp1_freq; > + > + /* frequency softlimits */ > + u32 min_freq_softlimit; > + u32 max_freq_softlimit; > + > + struct { > + u32 param_id; > + u32 param_value; > + u32 param_override; > + } debug; can you add all these extra fields in patches which will need them? Michal > }; > > int intel_guc_slpc_init(struct intel_guc_slpc *slpc); >
On 7/10/2021 9:05 AM, Michal Wajdeczko wrote: > > > On 10.07.2021 03:20, Vinay Belgaumkar wrote: >> Allocate data structures for SLPC and functions for >> initializing on host side. >> >> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> >> Signed-off-by: Sundaresan Sujaritha <sujaritha.sundaresan@intel.com> >> --- >> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 11 +++++++ >> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 36 ++++++++++++++++++++- >> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 20 ++++++++++++ >> 3 files changed, 66 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c >> index 9d61b2d54de4..82863a9bc8e8 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c >> @@ -336,6 +336,12 @@ int intel_guc_init(struct intel_guc *guc) >> goto err_ct; >> } >> >> + if (intel_guc_slpc_is_used(guc)) { >> + ret = intel_guc_slpc_init(&guc->slpc); >> + if (ret) >> + goto err_submission; >> + } >> + >> /* now that everything is perma-pinned, initialize the parameters */ >> guc_init_params(guc); >> >> @@ -346,6 +352,8 @@ int intel_guc_init(struct intel_guc *guc) >> >> return 0; >> >> +err_submission: >> + intel_guc_submission_fini(guc); >> err_ct: >> intel_guc_ct_fini(&guc->ct); >> err_ads: >> @@ -368,6 +376,9 @@ void intel_guc_fini(struct intel_guc *guc) >> >> i915_ggtt_disable_guc(gt->ggtt); >> >> + if (intel_guc_slpc_is_used(guc)) >> + intel_guc_slpc_fini(&guc->slpc); >> + >> if (intel_guc_submission_is_used(guc)) >> intel_guc_submission_fini(guc); >> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c >> index c1f569d2300d..94e2f19951aa 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c >> @@ -4,11 +4,41 @@ >> * Copyright © 2020 Intel Corporation >> */ >> >> +#include <asm/msr-index.h> > > hmm, what exactly is needed from this header ? Was being used in a previous version for MSR reads, removed. > >> + >> +#include "gt/intel_gt.h" >> +#include "gt/intel_rps.h" >> + >> +#include "i915_drv.h" >> #include "intel_guc_slpc.h" >> +#include "intel_pm.h" >> + >> +static inline struct intel_guc *slpc_to_guc(struct intel_guc_slpc *slpc) >> +{ >> + return container_of(slpc, struct intel_guc, slpc); >> +} >> + >> +static int slpc_shared_data_init(struct intel_guc_slpc *slpc) >> +{ >> + struct intel_guc *guc = slpc_to_guc(slpc); >> + int err; >> + u32 size = PAGE_ALIGN(sizeof(struct slpc_shared_data)); > > move err decl here > >> + >> + err = intel_guc_allocate_and_map_vma(guc, size, &slpc->vma, &slpc->vaddr); >> + if (unlikely(err)) { >> + DRM_ERROR("Failed to allocate slpc struct (err=%d)\n", err); > > s/slpc/SLPC > > and use drm_err instead > and you may also want to print error as %pe added. > >> + i915_vma_unpin_and_release(&slpc->vma, I915_VMA_RELEASE_MAP); > > do you really need this ? removed. > >> + return err; >> + } >> + >> + return err; >> +} >> >> int intel_guc_slpc_init(struct intel_guc_slpc *slpc) >> { >> - return 0; >> + GEM_BUG_ON(slpc->vma); >> + >> + return slpc_shared_data_init(slpc); >> } >> >> /* >> @@ -31,4 +61,8 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) >> >> void intel_guc_slpc_fini(struct intel_guc_slpc *slpc) >> { >> + if (!slpc->vma) >> + return; >> + >> + i915_vma_unpin_and_release(&slpc->vma, I915_VMA_RELEASE_MAP); >> } >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h >> index 98036459a1a3..a2643b904165 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h >> @@ -3,12 +3,32 @@ >> * >> * Copyright © 2020 Intel Corporation >> */ >> + > > should be fixed in earlier patch > >> #ifndef _INTEL_GUC_SLPC_H_ >> #define _INTEL_GUC_SLPC_H_ >> >> +#include <linux/mutex.h> >> #include "intel_guc_slpc_fwif.h" >> >> struct intel_guc_slpc { >> + /*Protects access to vma and SLPC actions */ > > hmm, missing mutex ;) Removed. > >> + struct i915_vma *vma; >> + void *vaddr; > > no need to be void, define it as ptr to slpc_shared_data > >> + >> + /* platform frequency limits */ >> + u32 min_freq; >> + u32 rp0_freq; >> + u32 rp1_freq; >> + >> + /* frequency softlimits */ >> + u32 min_freq_softlimit; >> + u32 max_freq_softlimit; >> + >> + struct { >> + u32 param_id; >> + u32 param_value; >> + u32 param_override; >> + } debug; > > can you add all these extra fields in patches which will need them? > > Michal Done. Thanks, Vinay. > >> }; >> >> int intel_guc_slpc_init(struct intel_guc_slpc *slpc); >> >
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c index 9d61b2d54de4..82863a9bc8e8 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c @@ -336,6 +336,12 @@ int intel_guc_init(struct intel_guc *guc) goto err_ct; } + if (intel_guc_slpc_is_used(guc)) { + ret = intel_guc_slpc_init(&guc->slpc); + if (ret) + goto err_submission; + } + /* now that everything is perma-pinned, initialize the parameters */ guc_init_params(guc); @@ -346,6 +352,8 @@ int intel_guc_init(struct intel_guc *guc) return 0; +err_submission: + intel_guc_submission_fini(guc); err_ct: intel_guc_ct_fini(&guc->ct); err_ads: @@ -368,6 +376,9 @@ void intel_guc_fini(struct intel_guc *guc) i915_ggtt_disable_guc(gt->ggtt); + if (intel_guc_slpc_is_used(guc)) + intel_guc_slpc_fini(&guc->slpc); + if (intel_guc_submission_is_used(guc)) intel_guc_submission_fini(guc); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c index c1f569d2300d..94e2f19951aa 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c @@ -4,11 +4,41 @@ * Copyright © 2020 Intel Corporation */ +#include <asm/msr-index.h> + +#include "gt/intel_gt.h" +#include "gt/intel_rps.h" + +#include "i915_drv.h" #include "intel_guc_slpc.h" +#include "intel_pm.h" + +static inline struct intel_guc *slpc_to_guc(struct intel_guc_slpc *slpc) +{ + return container_of(slpc, struct intel_guc, slpc); +} + +static int slpc_shared_data_init(struct intel_guc_slpc *slpc) +{ + struct intel_guc *guc = slpc_to_guc(slpc); + int err; + u32 size = PAGE_ALIGN(sizeof(struct slpc_shared_data)); + + err = intel_guc_allocate_and_map_vma(guc, size, &slpc->vma, &slpc->vaddr); + if (unlikely(err)) { + DRM_ERROR("Failed to allocate slpc struct (err=%d)\n", err); + i915_vma_unpin_and_release(&slpc->vma, I915_VMA_RELEASE_MAP); + return err; + } + + return err; +} int intel_guc_slpc_init(struct intel_guc_slpc *slpc) { - return 0; + GEM_BUG_ON(slpc->vma); + + return slpc_shared_data_init(slpc); } /* @@ -31,4 +61,8 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) void intel_guc_slpc_fini(struct intel_guc_slpc *slpc) { + if (!slpc->vma) + return; + + i915_vma_unpin_and_release(&slpc->vma, I915_VMA_RELEASE_MAP); } diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h index 98036459a1a3..a2643b904165 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h @@ -3,12 +3,32 @@ * * Copyright © 2020 Intel Corporation */ + #ifndef _INTEL_GUC_SLPC_H_ #define _INTEL_GUC_SLPC_H_ +#include <linux/mutex.h> #include "intel_guc_slpc_fwif.h" struct intel_guc_slpc { + /*Protects access to vma and SLPC actions */ + struct i915_vma *vma; + void *vaddr; + + /* platform frequency limits */ + u32 min_freq; + u32 rp0_freq; + u32 rp1_freq; + + /* frequency softlimits */ + u32 min_freq_softlimit; + u32 max_freq_softlimit; + + struct { + u32 param_id; + u32 param_value; + u32 param_override; + } debug; }; int intel_guc_slpc_init(struct intel_guc_slpc *slpc);