diff mbox series

[v2,3/9] drm/i915/pxp: Add MTL hw-plumbing enabling for KCR operation

Message ID 20230111214226.907536-4-alan.previn.teres.alexis@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/pxp: Add MTL PXP Support | expand

Commit Message

Alan Previn Jan. 11, 2023, 9:42 p.m. UTC 
Add MTL hw-plumbing enabling for KCR operation under PXP
which includes:

1. Updating 'pick-gt' to get the media tile for
   KCR interrupt handling
2. Adding MTL's KCR registers for PXP operation
   (init, status-checking, etc.).

While doing #2, lets create a separate registers header file for PXP
to be consistent with other i915 global subsystems.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_irq.c       |  3 +-
 drivers/gpu/drm/i915/pxp/intel_pxp.c         | 35 ++++++++++++--------
 drivers/gpu/drm/i915/pxp/intel_pxp_regs.h    | 26 +++++++++++++++
 drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 29 +++++++++++-----
 4 files changed, 70 insertions(+), 23 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_regs.h

Comments

Daniele Ceraolo Spurio Jan. 19, 2023, 12:09 a.m. UTC | #1 
On 1/11/2023 1:42 PM, Alan Previn wrote:
> Add MTL hw-plumbing enabling for KCR operation under PXP
> which includes:
>
> 1. Updating 'pick-gt' to get the media tile for
>     KCR interrupt handling
> 2. Adding MTL's KCR registers for PXP operation
>     (init, status-checking, etc.).
>
> While doing #2, lets create a separate registers header file for PXP
> to be consistent with other i915 global subsystems.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_gt_irq.c       |  3 +-
>   drivers/gpu/drm/i915/pxp/intel_pxp.c         | 35 ++++++++++++--------
>   drivers/gpu/drm/i915/pxp/intel_pxp_regs.h    | 26 +++++++++++++++
>   drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 29 +++++++++++-----
>   4 files changed, 70 insertions(+), 23 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_regs.h
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> index 8fac2660e16b..957fa11373fc 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> @@ -100,7 +100,8 @@ static struct intel_gt *pick_gt(struct intel_gt *gt, u8 class, u8 instance)
>   	case VIDEO_ENHANCEMENT_CLASS:
>   		return media_gt;
>   	case OTHER_CLASS:
> -		if (instance == OTHER_GSC_INSTANCE && HAS_ENGINE(media_gt, GSC0))
> +		if ((instance == OTHER_GSC_INSTANCE || instance == OTHER_KCR_INSTANCE) &&
> +		    HAS_ENGINE(media_gt, GSC0))
>   			return media_gt;
>   		fallthrough;
>   	default:
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> index be52bf92e847..809b49f59594 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> @@ -14,6 +14,7 @@
>   #include "intel_pxp.h"
>   #include "intel_pxp_gsccs.h"
>   #include "intel_pxp_irq.h"
> +#include "intel_pxp_regs.h"
>   #include "intel_pxp_session.h"
>   #include "intel_pxp_tee.h"
>   #include "intel_pxp_types.h"
> @@ -61,21 +62,30 @@ bool intel_pxp_is_active(const struct intel_pxp *pxp)
>   	return IS_ENABLED(CONFIG_DRM_I915_PXP) && pxp && pxp->arb_is_valid;
>   }
>   
> -/* KCR register definitions */
> -#define KCR_INIT _MMIO(0x320f0)
> -/* Setting KCR Init bit is required after system boot */
> -#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES REG_BIT(14)
> +static i915_reg_t get_kcr_reg(const struct intel_pxp *pxp)
> +{
> +	if (pxp->gsccs_priv)

IMO here a better check would be:

if (pxp->ctrl_gt->type == GT_MEDIA)

It's equivalent, but IMO from a readability perspective it highlights 
the fact that this is a change due to us moving to the media GT model 
and has nothing to do with the GSC CS itself.

> +		return MTL_KCR_INIT;
> +	return GEN12_KCR_INIT;
> +}
>   
> -static void kcr_pxp_enable(struct intel_gt *gt)
> +static void kcr_pxp_set_status(const struct intel_pxp *pxp, bool enable)
>   {
> -	intel_uncore_write(gt->uncore, KCR_INIT,
> -			   _MASKED_BIT_ENABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES));
> +	i915_reg_t reg = get_kcr_reg(pxp);
> +	u32 val = enable ? _MASKED_BIT_ENABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES) :
> +		  _MASKED_BIT_DISABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES);
> +
> +	intel_uncore_write(pxp->ctrl_gt->uncore, reg, val);
>   }
>   
> -static void kcr_pxp_disable(struct intel_gt *gt)
> +static void kcr_pxp_enable(const struct intel_pxp *pxp)
>   {
> -	intel_uncore_write(gt->uncore, KCR_INIT,
> -			   _MASKED_BIT_DISABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES));
> +	kcr_pxp_set_status(pxp, true);
> +}
> +
> +static void kcr_pxp_disable(const struct intel_pxp *pxp)
> +{
> +	kcr_pxp_set_status(pxp, false);
>   }
>   
>   static int create_vcs_context(struct intel_pxp *pxp)
> @@ -323,14 +333,13 @@ int intel_pxp_start(struct intel_pxp *pxp)
>   
>   void intel_pxp_init_hw(struct intel_pxp *pxp)
>   {
> -	kcr_pxp_enable(pxp->ctrl_gt);
> +	kcr_pxp_enable(pxp);
>   	intel_pxp_irq_enable(pxp);
>   }
>   
>   void intel_pxp_fini_hw(struct intel_pxp *pxp)
>   {
> -	kcr_pxp_disable(pxp->ctrl_gt);
> -
> +	kcr_pxp_disable(pxp);
>   	intel_pxp_irq_disable(pxp);
>   }
>   
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_regs.h b/drivers/gpu/drm/i915/pxp/intel_pxp_regs.h
> new file mode 100644
> index 000000000000..dd4131903d4e
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_regs.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright(c) 2023, Intel Corporation. All rights reserved.
> + */
> +
> +#ifndef __INTEL_PXP_REGS_H__
> +#define __INTEL_PXP_REGS_H__
> +
> +#include "i915_reg_defs.h"
> +
> +/* KCR enable/disable control */
> +#define GEN12_KCR_INIT _MMIO(0x320f0)
> +#define MTL_KCR_INIT _MMIO(0x3860f0)
> +
> +/* Setting KCR Init bit is required after system boot */
> +#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES REG_BIT(14)
> +
> +/* KCR hwdrm session in play status 0-31 */
> +#define GEN12_KCR_SIP _MMIO(0x32260)
> +#define MTL_KCR_SIP _MMIO(0x386260)
> +
> +/* PXP global terminate register for session termination */
> +#define GEN12_KCR_GLOBAL_TERMINATE _MMIO(0x320f8)
> +#define MTL_KCR_GLOBAL_TERMINATE _MMIO(0x3860f8)

Non blocking suggestion:
it looks like all KCR regs are at the same relative offsets, just from a 
different base (which makes, sense, because the HW just got moved to the 
media tile as-is). So we could possibly have something like what we do 
for the engines:

#define GEN12_KCR_BASE 0x32000
#define MTL_KCR_BASE 0x386000

#define KCR_INIT(base) _MMIO(base + 0xf0)
#define KCR_GLOBAL_TERMINATE(base) _MMIO(base + 0xf8)
#define KCR_SIP(base) _MMIO(base + 0x260)

Then if we store the correct base in the pxp structure we can just pass 
it in the define when we need to access a register and remove the if 
conditions at each access.

Daniele

> +
> +#endif /* __INTEL_PXP_REGS_H__ */
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> index 080aa2209c5b..7bb06e67b155 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> @@ -11,23 +11,25 @@
>   #include "intel_pxp_session.h"
>   #include "intel_pxp_tee.h"
>   #include "intel_pxp_types.h"
> +#include "intel_pxp_regs.h"
>   
>   #define ARB_SESSION I915_PROTECTED_CONTENT_DEFAULT_SESSION /* shorter define */
>   
> -#define GEN12_KCR_SIP _MMIO(0x32260) /* KCR hwdrm session in play 0-31 */
> -
> -/* PXP global terminate register for session termination */
> -#define PXP_GLOBAL_TERMINATE _MMIO(0x320f8)
> -
>   static bool intel_pxp_session_is_in_play(struct intel_pxp *pxp, u32 id)
>   {
>   	struct intel_uncore *uncore = pxp->ctrl_gt->uncore;
>   	intel_wakeref_t wakeref;
> +	i915_reg_t reg;
>   	u32 sip = 0;
>   
>   	/* if we're suspended the session is considered off */
> -	with_intel_runtime_pm_if_in_use(uncore->rpm, wakeref)
> -		sip = intel_uncore_read(uncore, GEN12_KCR_SIP);
> +	with_intel_runtime_pm_if_in_use(uncore->rpm, wakeref) {
> +		if (pxp->gsccs_priv)
> +			reg = MTL_KCR_SIP;
> +		else
> +			reg = GEN12_KCR_SIP;
> +		sip = intel_uncore_read(uncore, reg);
> +	}
>   
>   	return sip & BIT(id);
>   }
> @@ -37,6 +39,7 @@ static int pxp_wait_for_session_state(struct intel_pxp *pxp, u32 id, bool in_pla
>   	struct intel_uncore *uncore = pxp->ctrl_gt->uncore;
>   	intel_wakeref_t wakeref;
>   	u32 mask = BIT(id);
> +	i915_reg_t reg;
>   	int ret;
>   
>   	/* if we're suspended the session is considered off */
> @@ -44,8 +47,13 @@ static int pxp_wait_for_session_state(struct intel_pxp *pxp, u32 id, bool in_pla
>   	if (!wakeref)
>   		return in_play ? -ENODEV : 0;
>   
> +	if (pxp->gsccs_priv)
> +		reg = MTL_KCR_SIP;
> +	else
> +		reg = GEN12_KCR_SIP;
> +
>   	ret = intel_wait_for_register(uncore,
> -				      GEN12_KCR_SIP,
> +				      reg,
>   				      mask,
>   				      in_play ? mask : 0,
>   				      100);
> @@ -112,7 +120,10 @@ static int pxp_terminate_arb_session_and_global(struct intel_pxp *pxp)
>   		return ret;
>   	}
>   
> -	intel_uncore_write(gt->uncore, PXP_GLOBAL_TERMINATE, 1);
> +	if (pxp->gsccs_priv)
> +		intel_uncore_write(gt->uncore, MTL_KCR_GLOBAL_TERMINATE, 1);
> +	else
> +		intel_uncore_write(gt->uncore, GEN12_KCR_GLOBAL_TERMINATE, 1);
>   
>   	return ret;
>   }
Alan Previn Jan. 20, 2023, 8:49 p.m. UTC | #2 
Thanks for reviewing.

On Wed, 2023-01-18 at 16:09 -0800, Ceraolo Spurio, Daniele wrote:
> > 
> > 
Alan: [snip]
> > > > -/* KCR register definitions */
> > > > -#define KCR_INIT _MMIO(0x320f0)
> > > > -/* Setting KCR Init bit is required after system boot */
> > > > -#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES REG_BIT(14)
> > > > +static i915_reg_t get_kcr_reg(const struct intel_pxp *pxp)
> > > > +{
> > > > +       if (pxp->gsccs_priv)
> > 
> > IMO here a better check would be:
> > 
> > if (pxp->ctrl_gt->type == GT_MEDIA)
> > 
> > It's equivalent, but IMO from a readability perspective it highlights 
> > the fact that this is a change due to us moving to the media GT model 
> > and has nothing to do with the GSC CS itself.
> > 
Alan: Yes agreed - in alignment with to your next comment, this readiblity
improvement shall therefore go into pxp_init (when we initialize value for kcr_base offset)

Alan: [snip]

> > > > +#ifndef __INTEL_PXP_REGS_H__
> > > > +#define __INTEL_PXP_REGS_H__
> > > > +
> > > > +#include "i915_reg_defs.h"
> > > > +
> > > > +/* KCR enable/disable control */
> > > > +#define GEN12_KCR_INIT _MMIO(0x320f0)
> > > > +#define MTL_KCR_INIT _MMIO(0x3860f0)
> > > > +
> > > > +/* Setting KCR Init bit is required after system boot */
> > > > +#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES REG_BIT(14)
> > > > +
> > > > +/* KCR hwdrm session in play status 0-31 */
> > > > +#define GEN12_KCR_SIP _MMIO(0x32260)
> > > > +#define MTL_KCR_SIP _MMIO(0x386260)
> > > > +
> > > > +/* PXP global terminate register for session termination */
> > > > +#define GEN12_KCR_GLOBAL_TERMINATE _MMIO(0x320f8)
> > > > +#define MTL_KCR_GLOBAL_TERMINATE _MMIO(0x3860f8)
> > 
> > Non blocking suggestion:
> > it looks like all KCR regs are at the same relative offsets, just from a 
> > different base (which makes, sense, because the HW just got moved to the 
> > media tile as-is). So we could possibly have something like what we do 
> > for the engines:
> > 
> > #define GEN12_KCR_BASE 0x32000
> > #define MTL_KCR_BASE 0x386000
> > 
> > #define KCR_INIT(base) _MMIO(base + 0xf0)
> > #define KCR_GLOBAL_TERMINATE(base) _MMIO(base + 0xf8)
> > #define KCR_SIP(base) _MMIO(base + 0x260)
> > 
> > Then if we store the correct base in the pxp structure we can just pass 
> > it in the define when we need to access a register and remove the if 
> > conditions at each access.
> > 
Alan: sounds good - will do this.

Alan: [snip]
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index 8fac2660e16b..957fa11373fc 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -100,7 +100,8 @@  static struct intel_gt *pick_gt(struct intel_gt *gt, u8 class, u8 instance)
 	case VIDEO_ENHANCEMENT_CLASS:
 		return media_gt;
 	case OTHER_CLASS:
-		if (instance == OTHER_GSC_INSTANCE && HAS_ENGINE(media_gt, GSC0))
+		if ((instance == OTHER_GSC_INSTANCE || instance == OTHER_KCR_INSTANCE) &&
+		    HAS_ENGINE(media_gt, GSC0))
 			return media_gt;
 		fallthrough;
 	default:
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
index be52bf92e847..809b49f59594 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
@@ -14,6 +14,7 @@ 
 #include "intel_pxp.h"
 #include "intel_pxp_gsccs.h"
 #include "intel_pxp_irq.h"
+#include "intel_pxp_regs.h"
 #include "intel_pxp_session.h"
 #include "intel_pxp_tee.h"
 #include "intel_pxp_types.h"
@@ -61,21 +62,30 @@  bool intel_pxp_is_active(const struct intel_pxp *pxp)
 	return IS_ENABLED(CONFIG_DRM_I915_PXP) && pxp && pxp->arb_is_valid;
 }
 
-/* KCR register definitions */
-#define KCR_INIT _MMIO(0x320f0)
-/* Setting KCR Init bit is required after system boot */
-#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES REG_BIT(14)
+static i915_reg_t get_kcr_reg(const struct intel_pxp *pxp)
+{
+	if (pxp->gsccs_priv)
+		return MTL_KCR_INIT;
+	return GEN12_KCR_INIT;
+}
 
-static void kcr_pxp_enable(struct intel_gt *gt)
+static void kcr_pxp_set_status(const struct intel_pxp *pxp, bool enable)
 {
-	intel_uncore_write(gt->uncore, KCR_INIT,
-			   _MASKED_BIT_ENABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES));
+	i915_reg_t reg = get_kcr_reg(pxp);
+	u32 val = enable ? _MASKED_BIT_ENABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES) :
+		  _MASKED_BIT_DISABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES);
+
+	intel_uncore_write(pxp->ctrl_gt->uncore, reg, val);
 }
 
-static void kcr_pxp_disable(struct intel_gt *gt)
+static void kcr_pxp_enable(const struct intel_pxp *pxp)
 {
-	intel_uncore_write(gt->uncore, KCR_INIT,
-			   _MASKED_BIT_DISABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES));
+	kcr_pxp_set_status(pxp, true);
+}
+
+static void kcr_pxp_disable(const struct intel_pxp *pxp)
+{
+	kcr_pxp_set_status(pxp, false);
 }
 
 static int create_vcs_context(struct intel_pxp *pxp)
@@ -323,14 +333,13 @@  int intel_pxp_start(struct intel_pxp *pxp)
 
 void intel_pxp_init_hw(struct intel_pxp *pxp)
 {
-	kcr_pxp_enable(pxp->ctrl_gt);
+	kcr_pxp_enable(pxp);
 	intel_pxp_irq_enable(pxp);
 }
 
 void intel_pxp_fini_hw(struct intel_pxp *pxp)
 {
-	kcr_pxp_disable(pxp->ctrl_gt);
-
+	kcr_pxp_disable(pxp);
 	intel_pxp_irq_disable(pxp);
 }
 
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_regs.h b/drivers/gpu/drm/i915/pxp/intel_pxp_regs.h
new file mode 100644
index 000000000000..dd4131903d4e
--- /dev/null
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_regs.h
@@ -0,0 +1,26 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright(c) 2023, Intel Corporation. All rights reserved.
+ */
+
+#ifndef __INTEL_PXP_REGS_H__
+#define __INTEL_PXP_REGS_H__
+
+#include "i915_reg_defs.h"
+
+/* KCR enable/disable control */
+#define GEN12_KCR_INIT _MMIO(0x320f0)
+#define MTL_KCR_INIT _MMIO(0x3860f0)
+
+/* Setting KCR Init bit is required after system boot */
+#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES REG_BIT(14)
+
+/* KCR hwdrm session in play status 0-31 */
+#define GEN12_KCR_SIP _MMIO(0x32260)
+#define MTL_KCR_SIP _MMIO(0x386260)
+
+/* PXP global terminate register for session termination */
+#define GEN12_KCR_GLOBAL_TERMINATE _MMIO(0x320f8)
+#define MTL_KCR_GLOBAL_TERMINATE _MMIO(0x3860f8)
+
+#endif /* __INTEL_PXP_REGS_H__ */
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
index 080aa2209c5b..7bb06e67b155 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
@@ -11,23 +11,25 @@ 
 #include "intel_pxp_session.h"
 #include "intel_pxp_tee.h"
 #include "intel_pxp_types.h"
+#include "intel_pxp_regs.h"
 
 #define ARB_SESSION I915_PROTECTED_CONTENT_DEFAULT_SESSION /* shorter define */
 
-#define GEN12_KCR_SIP _MMIO(0x32260) /* KCR hwdrm session in play 0-31 */
-
-/* PXP global terminate register for session termination */
-#define PXP_GLOBAL_TERMINATE _MMIO(0x320f8)
-
 static bool intel_pxp_session_is_in_play(struct intel_pxp *pxp, u32 id)
 {
 	struct intel_uncore *uncore = pxp->ctrl_gt->uncore;
 	intel_wakeref_t wakeref;
+	i915_reg_t reg;
 	u32 sip = 0;
 
 	/* if we're suspended the session is considered off */
-	with_intel_runtime_pm_if_in_use(uncore->rpm, wakeref)
-		sip = intel_uncore_read(uncore, GEN12_KCR_SIP);
+	with_intel_runtime_pm_if_in_use(uncore->rpm, wakeref) {
+		if (pxp->gsccs_priv)
+			reg = MTL_KCR_SIP;
+		else
+			reg = GEN12_KCR_SIP;
+		sip = intel_uncore_read(uncore, reg);
+	}
 
 	return sip & BIT(id);
 }
@@ -37,6 +39,7 @@  static int pxp_wait_for_session_state(struct intel_pxp *pxp, u32 id, bool in_pla
 	struct intel_uncore *uncore = pxp->ctrl_gt->uncore;
 	intel_wakeref_t wakeref;
 	u32 mask = BIT(id);
+	i915_reg_t reg;
 	int ret;
 
 	/* if we're suspended the session is considered off */
@@ -44,8 +47,13 @@  static int pxp_wait_for_session_state(struct intel_pxp *pxp, u32 id, bool in_pla
 	if (!wakeref)
 		return in_play ? -ENODEV : 0;
 
+	if (pxp->gsccs_priv)
+		reg = MTL_KCR_SIP;
+	else
+		reg = GEN12_KCR_SIP;
+
 	ret = intel_wait_for_register(uncore,
-				      GEN12_KCR_SIP,
+				      reg,
 				      mask,
 				      in_play ? mask : 0,
 				      100);
@@ -112,7 +120,10 @@  static int pxp_terminate_arb_session_and_global(struct intel_pxp *pxp)
 		return ret;
 	}
 
-	intel_uncore_write(gt->uncore, PXP_GLOBAL_TERMINATE, 1);
+	if (pxp->gsccs_priv)
+		intel_uncore_write(gt->uncore, MTL_KCR_GLOBAL_TERMINATE, 1);
+	else
+		intel_uncore_write(gt->uncore, GEN12_KCR_GLOBAL_TERMINATE, 1);
 
 	return ret;
 }