diff mbox series

[v8,02/69] drm/i915: Pin timeline map after first timeline pin, v3.

Message ID 20210311134249.588632-3-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Remove obj->mm.lock! | expand

Commit Message

Maarten Lankhorst March 11, 2021, 1:41 p.m. UTC 
We're starting to require the reservation lock for pinning,
so wait until we have that.

Update the selftests to handle this correctly, and ensure pin is
called in live_hwsp_rollover_user() and mock_hwsp_freelist().

Changes since v1:
- Fix NULL + XX arithmatic, use casts. (kbuild)
Changes since v2:
- Clear entire cacheline when pinning.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reported-by: kernel test robot <lkp@intel.com>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_timeline.c    | 40 +++++++++----
 drivers/gpu/drm/i915/gt/intel_timeline.h    |  2 +
 drivers/gpu/drm/i915/gt/mock_engine.c       | 22 ++++++-
 drivers/gpu/drm/i915/gt/selftest_timeline.c | 63 +++++++++++----------
 drivers/gpu/drm/i915/i915_selftest.h        |  2 +
 5 files changed, 84 insertions(+), 45 deletions(-)

Comments

Jason Ekstrand March 11, 2021, 9:44 p.m. UTC | #1 
On Thu, Mar 11, 2021 at 7:49 AM Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
>
> We're starting to require the reservation lock for pinning,
> so wait until we have that.
>
> Update the selftests to handle this correctly, and ensure pin is
> called in live_hwsp_rollover_user() and mock_hwsp_freelist().
>
> Changes since v1:
> - Fix NULL + XX arithmatic, use casts. (kbuild)
> Changes since v2:
> - Clear entire cacheline when pinning.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_timeline.c    | 40 +++++++++----
>  drivers/gpu/drm/i915/gt/intel_timeline.h    |  2 +
>  drivers/gpu/drm/i915/gt/mock_engine.c       | 22 ++++++-
>  drivers/gpu/drm/i915/gt/selftest_timeline.c | 63 +++++++++++----------
>  drivers/gpu/drm/i915/i915_selftest.h        |  2 +
>  5 files changed, 84 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
> index efe2030cfe5e..032e1d1b4c5e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_timeline.c
> +++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
> @@ -52,14 +52,29 @@ static int __timeline_active(struct i915_active *active)
>         return 0;
>  }
>
> +I915_SELFTEST_EXPORT int
> +intel_timeline_pin_map(struct intel_timeline *timeline)
> +{
> +       struct drm_i915_gem_object *obj = timeline->hwsp_ggtt->obj;
> +       u32 ofs = offset_in_page(timeline->hwsp_offset);
> +       void *vaddr;
> +
> +       vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
> +       if (IS_ERR(vaddr))
> +               return PTR_ERR(vaddr);
> +
> +       timeline->hwsp_map = vaddr;
> +       timeline->hwsp_seqno = memset(vaddr + ofs, 0, CACHELINE_BYTES);

What guarantees that hwsp_offset is cacheline-aligned?  From what I
saw in patch 1, it's incremented by 8 so only every 8th one is
actually CL-aligned.

> +       clflush(vaddr + ofs);
> +
> +       return 0;
> +}
> +
>  static int intel_timeline_init(struct intel_timeline *timeline,
>                                struct intel_gt *gt,
>                                struct i915_vma *hwsp,
>                                unsigned int offset)
>  {
> -       void *vaddr;
> -       u32 *seqno;
> -
>         kref_init(&timeline->kref);
>         atomic_set(&timeline->pin_count, 0);
>
> @@ -76,14 +91,8 @@ static int intel_timeline_init(struct intel_timeline *timeline,
>                 timeline->hwsp_ggtt = hwsp;
>         }
>
> -       vaddr = i915_gem_object_pin_map(hwsp->obj, I915_MAP_WB);
> -       if (IS_ERR(vaddr))
> -               return PTR_ERR(vaddr);
> -
> -       timeline->hwsp_map = vaddr;
> -       seqno = vaddr + timeline->hwsp_offset;
> -       WRITE_ONCE(*seqno, 0);
> -       timeline->hwsp_seqno = seqno;
> +       timeline->hwsp_map = NULL;
> +       timeline->hwsp_seqno = (void *)(long)timeline->hwsp_offset;

Maybe uintptr_t instead of long?  I think they're always the same on
Linux but uintptr_t seems like the more "correct" type for this sort
of thing.


>
>         GEM_BUG_ON(timeline->hwsp_offset >= hwsp->size);
>
> @@ -113,7 +122,8 @@ static void intel_timeline_fini(struct rcu_head *rcu)
>         struct intel_timeline *timeline =
>                 container_of(rcu, struct intel_timeline, rcu);
>
> -       i915_gem_object_unpin_map(timeline->hwsp_ggtt->obj);
> +       if (timeline->hwsp_map)
> +               i915_gem_object_unpin_map(timeline->hwsp_ggtt->obj);
>
>         i915_vma_put(timeline->hwsp_ggtt);
>         i915_active_fini(&timeline->active);
> @@ -173,6 +183,12 @@ int intel_timeline_pin(struct intel_timeline *tl, struct i915_gem_ww_ctx *ww)
>         if (atomic_add_unless(&tl->pin_count, 1, 0))
>                 return 0;
>
> +       if (!tl->hwsp_map) {
> +               err = intel_timeline_pin_map(tl);
> +               if (err)
> +                       return err;
> +       }
> +
>         err = i915_ggtt_pin(tl->hwsp_ggtt, ww, 0, PIN_HIGH);
>         if (err)
>                 return err;
> diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.h b/drivers/gpu/drm/i915/gt/intel_timeline.h
> index b1f81d947f8d..57308c4d664a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_timeline.h
> +++ b/drivers/gpu/drm/i915/gt/intel_timeline.h
> @@ -98,4 +98,6 @@ intel_timeline_is_last(const struct intel_timeline *tl,
>         return list_is_last_rcu(&rq->link, &tl->requests);
>  }
>
> +I915_SELFTEST_DECLARE(int intel_timeline_pin_map(struct intel_timeline *tl));
> +
>  #endif
> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
> index 5662f7c2f719..42fd86658ee7 100644
> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
> @@ -13,9 +13,20 @@
>  #include "mock_engine.h"
>  #include "selftests/mock_request.h"
>
> -static void mock_timeline_pin(struct intel_timeline *tl)
> +static int mock_timeline_pin(struct intel_timeline *tl)
>  {
> +       int err;
> +
> +       if (WARN_ON(!i915_gem_object_trylock(tl->hwsp_ggtt->obj)))
> +               return -EBUSY;
> +
> +       err = intel_timeline_pin_map(tl);
> +       i915_gem_object_unlock(tl->hwsp_ggtt->obj);
> +       if (err)
> +               return err;
> +
>         atomic_inc(&tl->pin_count);
> +       return 0;
>  }
>
>  static void mock_timeline_unpin(struct intel_timeline *tl)
> @@ -133,6 +144,8 @@ static void mock_context_destroy(struct kref *ref)
>
>  static int mock_context_alloc(struct intel_context *ce)
>  {
> +       int err;
> +
>         ce->ring = mock_ring(ce->engine);
>         if (!ce->ring)
>                 return -ENOMEM;
> @@ -143,7 +156,12 @@ static int mock_context_alloc(struct intel_context *ce)
>                 return PTR_ERR(ce->timeline);
>         }
>
> -       mock_timeline_pin(ce->timeline);
> +       err = mock_timeline_pin(ce->timeline);
> +       if (err) {
> +               intel_timeline_put(ce->timeline);
> +               ce->timeline = NULL;
> +               return err;
> +       }
>
>         return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> index a4c78062e92b..31b492eb2982 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> @@ -34,7 +34,7 @@ static unsigned long hwsp_cacheline(struct intel_timeline *tl)
>  {
>         unsigned long address = (unsigned long)page_address(hwsp_page(tl));
>
> -       return (address + tl->hwsp_offset) / CACHELINE_BYTES;
> +       return (address + offset_in_page(tl->hwsp_offset)) / CACHELINE_BYTES;

Does this belong in the previous commit?  I've got no clue what I'm
talking about here but it looks like it goes with the hwsp_offset
wrapping changes in 01/69.

--Jason

>  }
>
>  #define CACHELINES_PER_PAGE (PAGE_SIZE / CACHELINE_BYTES)
> @@ -58,6 +58,7 @@ static void __mock_hwsp_record(struct mock_hwsp_freelist *state,
>         tl = xchg(&state->history[idx], tl);
>         if (tl) {
>                 radix_tree_delete(&state->cachelines, hwsp_cacheline(tl));
> +               intel_timeline_unpin(tl);
>                 intel_timeline_put(tl);
>         }
>  }
> @@ -77,6 +78,12 @@ static int __mock_hwsp_timeline(struct mock_hwsp_freelist *state,
>                 if (IS_ERR(tl))
>                         return PTR_ERR(tl);
>
> +               err = intel_timeline_pin(tl, NULL);
> +               if (err) {
> +                       intel_timeline_put(tl);
> +                       return err;
> +               }
> +
>                 cacheline = hwsp_cacheline(tl);
>                 err = radix_tree_insert(&state->cachelines, cacheline, tl);
>                 if (err) {
> @@ -84,6 +91,7 @@ static int __mock_hwsp_timeline(struct mock_hwsp_freelist *state,
>                                 pr_err("HWSP cacheline %lu already used; duplicate allocation!\n",
>                                        cacheline);
>                         }
> +                       intel_timeline_unpin(tl);
>                         intel_timeline_put(tl);
>                         return err;
>                 }
> @@ -451,7 +459,7 @@ static int emit_ggtt_store_dw(struct i915_request *rq, u32 addr, u32 value)
>  }
>
>  static struct i915_request *
> -tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
> +checked_tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>  {
>         struct i915_request *rq;
>         int err;
> @@ -462,6 +470,13 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>                 goto out;
>         }
>
> +       if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
> +               pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
> +                      *tl->hwsp_seqno, tl->seqno);
> +               intel_timeline_unpin(tl);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
>         rq = intel_engine_create_kernel_request(engine);
>         if (IS_ERR(rq))
>                 goto out_unpin;
> @@ -483,25 +498,6 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>         return rq;
>  }
>
> -static struct intel_timeline *
> -checked_intel_timeline_create(struct intel_gt *gt)
> -{
> -       struct intel_timeline *tl;
> -
> -       tl = intel_timeline_create(gt);
> -       if (IS_ERR(tl))
> -               return tl;
> -
> -       if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
> -               pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
> -                      *tl->hwsp_seqno, tl->seqno);
> -               intel_timeline_put(tl);
> -               return ERR_PTR(-EINVAL);
> -       }
> -
> -       return tl;
> -}
> -
>  static int live_hwsp_engine(void *arg)
>  {
>  #define NUM_TIMELINES 4096
> @@ -534,13 +530,13 @@ static int live_hwsp_engine(void *arg)
>                         struct intel_timeline *tl;
>                         struct i915_request *rq;
>
> -                       tl = checked_intel_timeline_create(gt);
> +                       tl = intel_timeline_create(gt);
>                         if (IS_ERR(tl)) {
>                                 err = PTR_ERR(tl);
>                                 break;
>                         }
>
> -                       rq = tl_write(tl, engine, count);
> +                       rq = checked_tl_write(tl, engine, count);
>                         if (IS_ERR(rq)) {
>                                 intel_timeline_put(tl);
>                                 err = PTR_ERR(rq);
> @@ -607,14 +603,14 @@ static int live_hwsp_alternate(void *arg)
>                         if (!intel_engine_can_store_dword(engine))
>                                 continue;
>
> -                       tl = checked_intel_timeline_create(gt);
> +                       tl = intel_timeline_create(gt);
>                         if (IS_ERR(tl)) {
>                                 err = PTR_ERR(tl);
>                                 goto out;
>                         }
>
>                         intel_engine_pm_get(engine);
> -                       rq = tl_write(tl, engine, count);
> +                       rq = checked_tl_write(tl, engine, count);
>                         intel_engine_pm_put(engine);
>                         if (IS_ERR(rq)) {
>                                 intel_timeline_put(tl);
> @@ -1257,6 +1253,10 @@ static int live_hwsp_rollover_user(void *arg)
>                 if (!tl->has_initial_breadcrumb)
>                         goto out;
>
> +               err = intel_context_pin(ce);
> +               if (err)
> +                       goto out;
> +
>                 tl->seqno = -4u;
>                 WRITE_ONCE(*(u32 *)tl->hwsp_seqno, tl->seqno);
>
> @@ -1266,7 +1266,7 @@ static int live_hwsp_rollover_user(void *arg)
>                         this = intel_context_create_request(ce);
>                         if (IS_ERR(this)) {
>                                 err = PTR_ERR(this);
> -                               goto out;
> +                               goto out_unpin;
>                         }
>
>                         pr_debug("%s: create fence.seqnp:%d\n",
> @@ -1285,17 +1285,18 @@ static int live_hwsp_rollover_user(void *arg)
>                 if (i915_request_wait(rq[2], 0, HZ / 5) < 0) {
>                         pr_err("Wait for timeline wrap timed out!\n");
>                         err = -EIO;
> -                       goto out;
> +                       goto out_unpin;
>                 }
>
>                 for (i = 0; i < ARRAY_SIZE(rq); i++) {
>                         if (!i915_request_completed(rq[i])) {
>                                 pr_err("Pre-wrap request not completed!\n");
>                                 err = -EINVAL;
> -                               goto out;
> +                               goto out_unpin;
>                         }
>                 }
> -
> +out_unpin:
> +               intel_context_unpin(ce);
>  out:
>                 for (i = 0; i < ARRAY_SIZE(rq); i++)
>                         i915_request_put(rq[i]);
> @@ -1337,13 +1338,13 @@ static int live_hwsp_recycle(void *arg)
>                         struct intel_timeline *tl;
>                         struct i915_request *rq;
>
> -                       tl = checked_intel_timeline_create(gt);
> +                       tl = intel_timeline_create(gt);
>                         if (IS_ERR(tl)) {
>                                 err = PTR_ERR(tl);
>                                 break;
>                         }
>
> -                       rq = tl_write(tl, engine, count);
> +                       rq = checked_tl_write(tl, engine, count);
>                         if (IS_ERR(rq)) {
>                                 intel_timeline_put(tl);
>                                 err = PTR_ERR(rq);
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> index d53d207ab6eb..f54de0499be7 100644
> --- a/drivers/gpu/drm/i915/i915_selftest.h
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -107,6 +107,7 @@ int __i915_subtests(const char *caller,
>
>  #define I915_SELFTEST_DECLARE(x) x
>  #define I915_SELFTEST_ONLY(x) unlikely(x)
> +#define I915_SELFTEST_EXPORT
>
>  #else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */
>
> @@ -116,6 +117,7 @@ static inline int i915_perf_selftests(struct pci_dev *pdev) { return 0; }
>
>  #define I915_SELFTEST_DECLARE(x)
>  #define I915_SELFTEST_ONLY(x) 0
> +#define I915_SELFTEST_EXPORT static
>
>  #endif
>
> --
> 2.30.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst March 15, 2021, 12:34 p.m. UTC | #2 
Op 2021-03-11 om 22:44 schreef Jason Ekstrand:
> On Thu, Mar 11, 2021 at 7:49 AM Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>> We're starting to require the reservation lock for pinning,
>> so wait until we have that.
>>
>> Update the selftests to handle this correctly, and ensure pin is
>> called in live_hwsp_rollover_user() and mock_hwsp_freelist().
>>
>> Changes since v1:
>> - Fix NULL + XX arithmatic, use casts. (kbuild)
>> Changes since v2:
>> - Clear entire cacheline when pinning.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/intel_timeline.c    | 40 +++++++++----
>>  drivers/gpu/drm/i915/gt/intel_timeline.h    |  2 +
>>  drivers/gpu/drm/i915/gt/mock_engine.c       | 22 ++++++-
>>  drivers/gpu/drm/i915/gt/selftest_timeline.c | 63 +++++++++++----------
>>  drivers/gpu/drm/i915/i915_selftest.h        |  2 +
>>  5 files changed, 84 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
>> index efe2030cfe5e..032e1d1b4c5e 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_timeline.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
>> @@ -52,14 +52,29 @@ static int __timeline_active(struct i915_active *active)
>>         return 0;
>>  }
>>
>> +I915_SELFTEST_EXPORT int
>> +intel_timeline_pin_map(struct intel_timeline *timeline)
>> +{
>> +       struct drm_i915_gem_object *obj = timeline->hwsp_ggtt->obj;
>> +       u32 ofs = offset_in_page(timeline->hwsp_offset);
>> +       void *vaddr;
>> +
>> +       vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
>> +       if (IS_ERR(vaddr))
>> +               return PTR_ERR(vaddr);
>> +
>> +       timeline->hwsp_map = vaddr;
>> +       timeline->hwsp_seqno = memset(vaddr + ofs, 0, CACHELINE_BYTES);
> What guarantees that hwsp_offset is cacheline-aligned?  From what I
> saw in patch 1, it's incremented by 8 so only every 8th one is
> actually CL-aligned.

Yeah the name of the #define is wrong here. It was originally cacheline aligned because

the page was originally shared between different contexts. This is no longer the case,

but the name was kept. I think TIMELINE_SEQNO_ALIGN would be a better fit, I will respin.

>
>> +       clflush(vaddr + ofs);
>> +
>> +       return 0;
>> +}
>> +
>>  static int intel_timeline_init(struct intel_timeline *timeline,
>>                                struct intel_gt *gt,
>>                                struct i915_vma *hwsp,
>>                                unsigned int offset)
>>  {
>> -       void *vaddr;
>> -       u32 *seqno;
>> -
>>         kref_init(&timeline->kref);
>>         atomic_set(&timeline->pin_count, 0);
>>
>> @@ -76,14 +91,8 @@ static int intel_timeline_init(struct intel_timeline *timeline,
>>                 timeline->hwsp_ggtt = hwsp;
>>         }
>>
>> -       vaddr = i915_gem_object_pin_map(hwsp->obj, I915_MAP_WB);
>> -       if (IS_ERR(vaddr))
>> -               return PTR_ERR(vaddr);
>> -
>> -       timeline->hwsp_map = vaddr;
>> -       seqno = vaddr + timeline->hwsp_offset;
>> -       WRITE_ONCE(*seqno, 0);
>> -       timeline->hwsp_seqno = seqno;
>> +       timeline->hwsp_map = NULL;
>> +       timeline->hwsp_seqno = (void *)(long)timeline->hwsp_offset;
> Maybe uintptr_t instead of long?  I think they're always the same on
> Linux but uintptr_t seems like the more "correct" type for this sort
> of thing.

I did a non-scientific compare, void *)(uintptr_t vs void *)(long in the kernel, the latter was used 4x as often.

In general long is a pointer sized type for the kernel. I think for userspace it would be more correct, though.


>>         GEM_BUG_ON(timeline->hwsp_offset >= hwsp->size);
>>
>> @@ -113,7 +122,8 @@ static void intel_timeline_fini(struct rcu_head *rcu)
>>         struct intel_timeline *timeline =
>>                 container_of(rcu, struct intel_timeline, rcu);
>>
>> -       i915_gem_object_unpin_map(timeline->hwsp_ggtt->obj);
>> +       if (timeline->hwsp_map)
>> +               i915_gem_object_unpin_map(timeline->hwsp_ggtt->obj);
>>
>>         i915_vma_put(timeline->hwsp_ggtt);
>>         i915_active_fini(&timeline->active);
>> @@ -173,6 +183,12 @@ int intel_timeline_pin(struct intel_timeline *tl, struct i915_gem_ww_ctx *ww)
>>         if (atomic_add_unless(&tl->pin_count, 1, 0))
>>                 return 0;
>>
>> +       if (!tl->hwsp_map) {
>> +               err = intel_timeline_pin_map(tl);
>> +               if (err)
>> +                       return err;
>> +       }
>> +
>>         err = i915_ggtt_pin(tl->hwsp_ggtt, ww, 0, PIN_HIGH);
>>         if (err)
>>                 return err;
>> diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.h b/drivers/gpu/drm/i915/gt/intel_timeline.h
>> index b1f81d947f8d..57308c4d664a 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_timeline.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_timeline.h
>> @@ -98,4 +98,6 @@ intel_timeline_is_last(const struct intel_timeline *tl,
>>         return list_is_last_rcu(&rq->link, &tl->requests);
>>  }
>>
>> +I915_SELFTEST_DECLARE(int intel_timeline_pin_map(struct intel_timeline *tl));
>> +
>>  #endif
>> diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
>> index 5662f7c2f719..42fd86658ee7 100644
>> --- a/drivers/gpu/drm/i915/gt/mock_engine.c
>> +++ b/drivers/gpu/drm/i915/gt/mock_engine.c
>> @@ -13,9 +13,20 @@
>>  #include "mock_engine.h"
>>  #include "selftests/mock_request.h"
>>
>> -static void mock_timeline_pin(struct intel_timeline *tl)
>> +static int mock_timeline_pin(struct intel_timeline *tl)
>>  {
>> +       int err;
>> +
>> +       if (WARN_ON(!i915_gem_object_trylock(tl->hwsp_ggtt->obj)))
>> +               return -EBUSY;
>> +
>> +       err = intel_timeline_pin_map(tl);
>> +       i915_gem_object_unlock(tl->hwsp_ggtt->obj);
>> +       if (err)
>> +               return err;
>> +
>>         atomic_inc(&tl->pin_count);
>> +       return 0;
>>  }
>>
>>  static void mock_timeline_unpin(struct intel_timeline *tl)
>> @@ -133,6 +144,8 @@ static void mock_context_destroy(struct kref *ref)
>>
>>  static int mock_context_alloc(struct intel_context *ce)
>>  {
>> +       int err;
>> +
>>         ce->ring = mock_ring(ce->engine);
>>         if (!ce->ring)
>>                 return -ENOMEM;
>> @@ -143,7 +156,12 @@ static int mock_context_alloc(struct intel_context *ce)
>>                 return PTR_ERR(ce->timeline);
>>         }
>>
>> -       mock_timeline_pin(ce->timeline);
>> +       err = mock_timeline_pin(ce->timeline);
>> +       if (err) {
>> +               intel_timeline_put(ce->timeline);
>> +               ce->timeline = NULL;
>> +               return err;
>> +       }
>>
>>         return 0;
>>  }
>> diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
>> index a4c78062e92b..31b492eb2982 100644
>> --- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
>> +++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
>> @@ -34,7 +34,7 @@ static unsigned long hwsp_cacheline(struct intel_timeline *tl)
>>  {
>>         unsigned long address = (unsigned long)page_address(hwsp_page(tl));
>>
>> -       return (address + tl->hwsp_offset) / CACHELINE_BYTES;
>> +       return (address + offset_in_page(tl->hwsp_offset)) / CACHELINE_BYTES;
> Does this belong in the previous commit?  I've got no clue what I'm
> talking about here but it looks like it goes with the hwsp_offset
> wrapping changes in 01/69.
Yeah likely. Will move.
>
> --Jason
>
>>  }
>>
>>  #define CACHELINES_PER_PAGE (PAGE_SIZE / CACHELINE_BYTES)
>> @@ -58,6 +58,7 @@ static void __mock_hwsp_record(struct mock_hwsp_freelist *state,
>>         tl = xchg(&state->history[idx], tl);
>>         if (tl) {
>>                 radix_tree_delete(&state->cachelines, hwsp_cacheline(tl));
>> +               intel_timeline_unpin(tl);
>>                 intel_timeline_put(tl);
>>         }
>>  }
>> @@ -77,6 +78,12 @@ static int __mock_hwsp_timeline(struct mock_hwsp_freelist *state,
>>                 if (IS_ERR(tl))
>>                         return PTR_ERR(tl);
>>
>> +               err = intel_timeline_pin(tl, NULL);
>> +               if (err) {
>> +                       intel_timeline_put(tl);
>> +                       return err;
>> +               }
>> +
>>                 cacheline = hwsp_cacheline(tl);
>>                 err = radix_tree_insert(&state->cachelines, cacheline, tl);
>>                 if (err) {
>> @@ -84,6 +91,7 @@ static int __mock_hwsp_timeline(struct mock_hwsp_freelist *state,
>>                                 pr_err("HWSP cacheline %lu already used; duplicate allocation!\n",
>>                                        cacheline);
>>                         }
>> +                       intel_timeline_unpin(tl);
>>                         intel_timeline_put(tl);
>>                         return err;
>>                 }
>> @@ -451,7 +459,7 @@ static int emit_ggtt_store_dw(struct i915_request *rq, u32 addr, u32 value)
>>  }
>>
>>  static struct i915_request *
>> -tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>> +checked_tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>>  {
>>         struct i915_request *rq;
>>         int err;
>> @@ -462,6 +470,13 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>>                 goto out;
>>         }
>>
>> +       if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
>> +               pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
>> +                      *tl->hwsp_seqno, tl->seqno);
>> +               intel_timeline_unpin(tl);
>> +               return ERR_PTR(-EINVAL);
>> +       }
>> +
>>         rq = intel_engine_create_kernel_request(engine);
>>         if (IS_ERR(rq))
>>                 goto out_unpin;
>> @@ -483,25 +498,6 @@ tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
>>         return rq;
>>  }
>>
>> -static struct intel_timeline *
>> -checked_intel_timeline_create(struct intel_gt *gt)
>> -{
>> -       struct intel_timeline *tl;
>> -
>> -       tl = intel_timeline_create(gt);
>> -       if (IS_ERR(tl))
>> -               return tl;
>> -
>> -       if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
>> -               pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
>> -                      *tl->hwsp_seqno, tl->seqno);
>> -               intel_timeline_put(tl);
>> -               return ERR_PTR(-EINVAL);
>> -       }
>> -
>> -       return tl;
>> -}
>> -
>>  static int live_hwsp_engine(void *arg)
>>  {
>>  #define NUM_TIMELINES 4096
>> @@ -534,13 +530,13 @@ static int live_hwsp_engine(void *arg)
>>                         struct intel_timeline *tl;
>>                         struct i915_request *rq;
>>
>> -                       tl = checked_intel_timeline_create(gt);
>> +                       tl = intel_timeline_create(gt);
>>                         if (IS_ERR(tl)) {
>>                                 err = PTR_ERR(tl);
>>                                 break;
>>                         }
>>
>> -                       rq = tl_write(tl, engine, count);
>> +                       rq = checked_tl_write(tl, engine, count);
>>                         if (IS_ERR(rq)) {
>>                                 intel_timeline_put(tl);
>>                                 err = PTR_ERR(rq);
>> @@ -607,14 +603,14 @@ static int live_hwsp_alternate(void *arg)
>>                         if (!intel_engine_can_store_dword(engine))
>>                                 continue;
>>
>> -                       tl = checked_intel_timeline_create(gt);
>> +                       tl = intel_timeline_create(gt);
>>                         if (IS_ERR(tl)) {
>>                                 err = PTR_ERR(tl);
>>                                 goto out;
>>                         }
>>
>>                         intel_engine_pm_get(engine);
>> -                       rq = tl_write(tl, engine, count);
>> +                       rq = checked_tl_write(tl, engine, count);
>>                         intel_engine_pm_put(engine);
>>                         if (IS_ERR(rq)) {
>>                                 intel_timeline_put(tl);
>> @@ -1257,6 +1253,10 @@ static int live_hwsp_rollover_user(void *arg)
>>                 if (!tl->has_initial_breadcrumb)
>>                         goto out;
>>
>> +               err = intel_context_pin(ce);
>> +               if (err)
>> +                       goto out;
>> +
>>                 tl->seqno = -4u;
>>                 WRITE_ONCE(*(u32 *)tl->hwsp_seqno, tl->seqno);
>>
>> @@ -1266,7 +1266,7 @@ static int live_hwsp_rollover_user(void *arg)
>>                         this = intel_context_create_request(ce);
>>                         if (IS_ERR(this)) {
>>                                 err = PTR_ERR(this);
>> -                               goto out;
>> +                               goto out_unpin;
>>                         }
>>
>>                         pr_debug("%s: create fence.seqnp:%d\n",
>> @@ -1285,17 +1285,18 @@ static int live_hwsp_rollover_user(void *arg)
>>                 if (i915_request_wait(rq[2], 0, HZ / 5) < 0) {
>>                         pr_err("Wait for timeline wrap timed out!\n");
>>                         err = -EIO;
>> -                       goto out;
>> +                       goto out_unpin;
>>                 }
>>
>>                 for (i = 0; i < ARRAY_SIZE(rq); i++) {
>>                         if (!i915_request_completed(rq[i])) {
>>                                 pr_err("Pre-wrap request not completed!\n");
>>                                 err = -EINVAL;
>> -                               goto out;
>> +                               goto out_unpin;
>>                         }
>>                 }
>> -
>> +out_unpin:
>> +               intel_context_unpin(ce);
>>  out:
>>                 for (i = 0; i < ARRAY_SIZE(rq); i++)
>>                         i915_request_put(rq[i]);
>> @@ -1337,13 +1338,13 @@ static int live_hwsp_recycle(void *arg)
>>                         struct intel_timeline *tl;
>>                         struct i915_request *rq;
>>
>> -                       tl = checked_intel_timeline_create(gt);
>> +                       tl = intel_timeline_create(gt);
>>                         if (IS_ERR(tl)) {
>>                                 err = PTR_ERR(tl);
>>                                 break;
>>                         }
>>
>> -                       rq = tl_write(tl, engine, count);
>> +                       rq = checked_tl_write(tl, engine, count);
>>                         if (IS_ERR(rq)) {
>>                                 intel_timeline_put(tl);
>>                                 err = PTR_ERR(rq);
>> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
>> index d53d207ab6eb..f54de0499be7 100644
>> --- a/drivers/gpu/drm/i915/i915_selftest.h
>> +++ b/drivers/gpu/drm/i915/i915_selftest.h
>> @@ -107,6 +107,7 @@ int __i915_subtests(const char *caller,
>>
>>  #define I915_SELFTEST_DECLARE(x) x
>>  #define I915_SELFTEST_ONLY(x) unlikely(x)
>> +#define I915_SELFTEST_EXPORT
>>
>>  #else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */
>>
>> @@ -116,6 +117,7 @@ static inline int i915_perf_selftests(struct pci_dev *pdev) { return 0; }
>>
>>  #define I915_SELFTEST_DECLARE(x)
>>  #define I915_SELFTEST_ONLY(x) 0
>> +#define I915_SELFTEST_EXPORT static
>>
>>  #endif
>>
>> --
>> 2.30.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c b/drivers/gpu/drm/i915/gt/intel_timeline.c
index efe2030cfe5e..032e1d1b4c5e 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -52,14 +52,29 @@  static int __timeline_active(struct i915_active *active)
 	return 0;
 }
 
+I915_SELFTEST_EXPORT int
+intel_timeline_pin_map(struct intel_timeline *timeline)
+{
+	struct drm_i915_gem_object *obj = timeline->hwsp_ggtt->obj;
+	u32 ofs = offset_in_page(timeline->hwsp_offset);
+	void *vaddr;
+
+	vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
+	if (IS_ERR(vaddr))
+		return PTR_ERR(vaddr);
+
+	timeline->hwsp_map = vaddr;
+	timeline->hwsp_seqno = memset(vaddr + ofs, 0, CACHELINE_BYTES);
+	clflush(vaddr + ofs);
+
+	return 0;
+}
+
 static int intel_timeline_init(struct intel_timeline *timeline,
 			       struct intel_gt *gt,
 			       struct i915_vma *hwsp,
 			       unsigned int offset)
 {
-	void *vaddr;
-	u32 *seqno;
-
 	kref_init(&timeline->kref);
 	atomic_set(&timeline->pin_count, 0);
 
@@ -76,14 +91,8 @@  static int intel_timeline_init(struct intel_timeline *timeline,
 		timeline->hwsp_ggtt = hwsp;
 	}
 
-	vaddr = i915_gem_object_pin_map(hwsp->obj, I915_MAP_WB);
-	if (IS_ERR(vaddr))
-		return PTR_ERR(vaddr);
-
-	timeline->hwsp_map = vaddr;
-	seqno = vaddr + timeline->hwsp_offset;
-	WRITE_ONCE(*seqno, 0);
-	timeline->hwsp_seqno = seqno;
+	timeline->hwsp_map = NULL;
+	timeline->hwsp_seqno = (void *)(long)timeline->hwsp_offset;
 
 	GEM_BUG_ON(timeline->hwsp_offset >= hwsp->size);
 
@@ -113,7 +122,8 @@  static void intel_timeline_fini(struct rcu_head *rcu)
 	struct intel_timeline *timeline =
 		container_of(rcu, struct intel_timeline, rcu);
 
-	i915_gem_object_unpin_map(timeline->hwsp_ggtt->obj);
+	if (timeline->hwsp_map)
+		i915_gem_object_unpin_map(timeline->hwsp_ggtt->obj);
 
 	i915_vma_put(timeline->hwsp_ggtt);
 	i915_active_fini(&timeline->active);
@@ -173,6 +183,12 @@  int intel_timeline_pin(struct intel_timeline *tl, struct i915_gem_ww_ctx *ww)
 	if (atomic_add_unless(&tl->pin_count, 1, 0))
 		return 0;
 
+	if (!tl->hwsp_map) {
+		err = intel_timeline_pin_map(tl);
+		if (err)
+			return err;
+	}
+
 	err = i915_ggtt_pin(tl->hwsp_ggtt, ww, 0, PIN_HIGH);
 	if (err)
 		return err;
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.h b/drivers/gpu/drm/i915/gt/intel_timeline.h
index b1f81d947f8d..57308c4d664a 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.h
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.h
@@ -98,4 +98,6 @@  intel_timeline_is_last(const struct intel_timeline *tl,
 	return list_is_last_rcu(&rq->link, &tl->requests);
 }
 
+I915_SELFTEST_DECLARE(int intel_timeline_pin_map(struct intel_timeline *tl));
+
 #endif
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index 5662f7c2f719..42fd86658ee7 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -13,9 +13,20 @@ 
 #include "mock_engine.h"
 #include "selftests/mock_request.h"
 
-static void mock_timeline_pin(struct intel_timeline *tl)
+static int mock_timeline_pin(struct intel_timeline *tl)
 {
+	int err;
+
+	if (WARN_ON(!i915_gem_object_trylock(tl->hwsp_ggtt->obj)))
+		return -EBUSY;
+
+	err = intel_timeline_pin_map(tl);
+	i915_gem_object_unlock(tl->hwsp_ggtt->obj);
+	if (err)
+		return err;
+
 	atomic_inc(&tl->pin_count);
+	return 0;
 }
 
 static void mock_timeline_unpin(struct intel_timeline *tl)
@@ -133,6 +144,8 @@  static void mock_context_destroy(struct kref *ref)
 
 static int mock_context_alloc(struct intel_context *ce)
 {
+	int err;
+
 	ce->ring = mock_ring(ce->engine);
 	if (!ce->ring)
 		return -ENOMEM;
@@ -143,7 +156,12 @@  static int mock_context_alloc(struct intel_context *ce)
 		return PTR_ERR(ce->timeline);
 	}
 
-	mock_timeline_pin(ce->timeline);
+	err = mock_timeline_pin(ce->timeline);
+	if (err) {
+		intel_timeline_put(ce->timeline);
+		ce->timeline = NULL;
+		return err;
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index a4c78062e92b..31b492eb2982 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -34,7 +34,7 @@  static unsigned long hwsp_cacheline(struct intel_timeline *tl)
 {
 	unsigned long address = (unsigned long)page_address(hwsp_page(tl));
 
-	return (address + tl->hwsp_offset) / CACHELINE_BYTES;
+	return (address + offset_in_page(tl->hwsp_offset)) / CACHELINE_BYTES;
 }
 
 #define CACHELINES_PER_PAGE (PAGE_SIZE / CACHELINE_BYTES)
@@ -58,6 +58,7 @@  static void __mock_hwsp_record(struct mock_hwsp_freelist *state,
 	tl = xchg(&state->history[idx], tl);
 	if (tl) {
 		radix_tree_delete(&state->cachelines, hwsp_cacheline(tl));
+		intel_timeline_unpin(tl);
 		intel_timeline_put(tl);
 	}
 }
@@ -77,6 +78,12 @@  static int __mock_hwsp_timeline(struct mock_hwsp_freelist *state,
 		if (IS_ERR(tl))
 			return PTR_ERR(tl);
 
+		err = intel_timeline_pin(tl, NULL);
+		if (err) {
+			intel_timeline_put(tl);
+			return err;
+		}
+
 		cacheline = hwsp_cacheline(tl);
 		err = radix_tree_insert(&state->cachelines, cacheline, tl);
 		if (err) {
@@ -84,6 +91,7 @@  static int __mock_hwsp_timeline(struct mock_hwsp_freelist *state,
 				pr_err("HWSP cacheline %lu already used; duplicate allocation!\n",
 				       cacheline);
 			}
+			intel_timeline_unpin(tl);
 			intel_timeline_put(tl);
 			return err;
 		}
@@ -451,7 +459,7 @@  static int emit_ggtt_store_dw(struct i915_request *rq, u32 addr, u32 value)
 }
 
 static struct i915_request *
-tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
+checked_tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
 {
 	struct i915_request *rq;
 	int err;
@@ -462,6 +470,13 @@  tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
 		goto out;
 	}
 
+	if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
+		pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
+		       *tl->hwsp_seqno, tl->seqno);
+		intel_timeline_unpin(tl);
+		return ERR_PTR(-EINVAL);
+	}
+
 	rq = intel_engine_create_kernel_request(engine);
 	if (IS_ERR(rq))
 		goto out_unpin;
@@ -483,25 +498,6 @@  tl_write(struct intel_timeline *tl, struct intel_engine_cs *engine, u32 value)
 	return rq;
 }
 
-static struct intel_timeline *
-checked_intel_timeline_create(struct intel_gt *gt)
-{
-	struct intel_timeline *tl;
-
-	tl = intel_timeline_create(gt);
-	if (IS_ERR(tl))
-		return tl;
-
-	if (READ_ONCE(*tl->hwsp_seqno) != tl->seqno) {
-		pr_err("Timeline created with incorrect breadcrumb, found %x, expected %x\n",
-		       *tl->hwsp_seqno, tl->seqno);
-		intel_timeline_put(tl);
-		return ERR_PTR(-EINVAL);
-	}
-
-	return tl;
-}
-
 static int live_hwsp_engine(void *arg)
 {
 #define NUM_TIMELINES 4096
@@ -534,13 +530,13 @@  static int live_hwsp_engine(void *arg)
 			struct intel_timeline *tl;
 			struct i915_request *rq;
 
-			tl = checked_intel_timeline_create(gt);
+			tl = intel_timeline_create(gt);
 			if (IS_ERR(tl)) {
 				err = PTR_ERR(tl);
 				break;
 			}
 
-			rq = tl_write(tl, engine, count);
+			rq = checked_tl_write(tl, engine, count);
 			if (IS_ERR(rq)) {
 				intel_timeline_put(tl);
 				err = PTR_ERR(rq);
@@ -607,14 +603,14 @@  static int live_hwsp_alternate(void *arg)
 			if (!intel_engine_can_store_dword(engine))
 				continue;
 
-			tl = checked_intel_timeline_create(gt);
+			tl = intel_timeline_create(gt);
 			if (IS_ERR(tl)) {
 				err = PTR_ERR(tl);
 				goto out;
 			}
 
 			intel_engine_pm_get(engine);
-			rq = tl_write(tl, engine, count);
+			rq = checked_tl_write(tl, engine, count);
 			intel_engine_pm_put(engine);
 			if (IS_ERR(rq)) {
 				intel_timeline_put(tl);
@@ -1257,6 +1253,10 @@  static int live_hwsp_rollover_user(void *arg)
 		if (!tl->has_initial_breadcrumb)
 			goto out;
 
+		err = intel_context_pin(ce);
+		if (err)
+			goto out;
+
 		tl->seqno = -4u;
 		WRITE_ONCE(*(u32 *)tl->hwsp_seqno, tl->seqno);
 
@@ -1266,7 +1266,7 @@  static int live_hwsp_rollover_user(void *arg)
 			this = intel_context_create_request(ce);
 			if (IS_ERR(this)) {
 				err = PTR_ERR(this);
-				goto out;
+				goto out_unpin;
 			}
 
 			pr_debug("%s: create fence.seqnp:%d\n",
@@ -1285,17 +1285,18 @@  static int live_hwsp_rollover_user(void *arg)
 		if (i915_request_wait(rq[2], 0, HZ / 5) < 0) {
 			pr_err("Wait for timeline wrap timed out!\n");
 			err = -EIO;
-			goto out;
+			goto out_unpin;
 		}
 
 		for (i = 0; i < ARRAY_SIZE(rq); i++) {
 			if (!i915_request_completed(rq[i])) {
 				pr_err("Pre-wrap request not completed!\n");
 				err = -EINVAL;
-				goto out;
+				goto out_unpin;
 			}
 		}
-
+out_unpin:
+		intel_context_unpin(ce);
 out:
 		for (i = 0; i < ARRAY_SIZE(rq); i++)
 			i915_request_put(rq[i]);
@@ -1337,13 +1338,13 @@  static int live_hwsp_recycle(void *arg)
 			struct intel_timeline *tl;
 			struct i915_request *rq;
 
-			tl = checked_intel_timeline_create(gt);
+			tl = intel_timeline_create(gt);
 			if (IS_ERR(tl)) {
 				err = PTR_ERR(tl);
 				break;
 			}
 
-			rq = tl_write(tl, engine, count);
+			rq = checked_tl_write(tl, engine, count);
 			if (IS_ERR(rq)) {
 				intel_timeline_put(tl);
 				err = PTR_ERR(rq);
diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
index d53d207ab6eb..f54de0499be7 100644
--- a/drivers/gpu/drm/i915/i915_selftest.h
+++ b/drivers/gpu/drm/i915/i915_selftest.h
@@ -107,6 +107,7 @@  int __i915_subtests(const char *caller,
 
 #define I915_SELFTEST_DECLARE(x) x
 #define I915_SELFTEST_ONLY(x) unlikely(x)
+#define I915_SELFTEST_EXPORT
 
 #else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */
 
@@ -116,6 +117,7 @@  static inline int i915_perf_selftests(struct pci_dev *pdev) { return 0; }
 
 #define I915_SELFTEST_DECLARE(x)
 #define I915_SELFTEST_ONLY(x) 0
+#define I915_SELFTEST_EXPORT static
 
 #endif