From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Shankar, Uma" <uma.shankar@intel.com>,
"Hogander, Jouni" <jouni.hogander@intel.com>,
"juhapekka.heikkila@gmail.com" <juhapekka.heikkila@gmail.com>,
"intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>
Subject: Re: [Intel-xe] [v9, 1/6] Revert "FIXME: drm/i915: xe dpt integration"
Date: Wed, 15 Nov 2023 14:59:07 +0200 [thread overview]
Message-ID: <87fs17qijo.fsf@intel.com> (raw)
In-Reply-To: <IA1PR11MB6347762FB53840B06F2188C0F4B2A@IA1PR11MB6347.namprd11.prod.outlook.com>
On Tue, 14 Nov 2023, "Shankar, Uma" <uma.shankar@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Hogander,
>> Jouni
>> Sent: Tuesday, November 14, 2023 2:11 PM
>> To: juhapekka.heikkila@gmail.com; intel-xe@lists.freedesktop.org
>> Subject: Re: [Intel-xe] [v9, 1/6] Revert "FIXME: drm/i915: xe dpt integration"
>>
>> Hello Juha-Pekka,
>>
>> Some of these patches already sent to intel-gfx as discussed offline:
>>
>> https://patchwork.freedesktop.org/series/126352/
>>
>> To my opinion we could already merge this and then do backport when
>> i915 patches are merged. For the whole set:
>>
>> Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
>
> I agree, lets merge the i915 parts to drm-intel-next and then send out a fresh series,
> cherry picking the same along with the fixup to drop the FIXME patch.
I see that this was already merged... but I think at this point in time
it would be better to get the stuff merged to i915 first, and then
backported with cherry-pick -x annotations. Just skip the intermediate
step of merging something that must be fixed later.
BR,
Jani.
>
> Regards,
> Uma Shankar
>
>> BR,
>>
>> Jouni Högander
>>
>> On Mon, 2023-11-13 at 21:56 +0200, Juha-Pekka Heikkila wrote:
>> > This reverts commit e417510409165b44ad59bfdbc59cd90316e91dc1.
>> > ---
>> > .../gpu/drm/i915/display/intel_atomic_plane.c | 4 ---
>> > .../drm/i915/display/intel_display_types.h | 6 ----
>> > drivers/gpu/drm/i915/display/intel_dpt.c | 14 +++-----
>> > drivers/gpu/drm/i915/display/intel_fb.c | 36 +++--------------
>> > --
>> > .../drm/i915/display/skl_universal_plane.c | 6 +---
>> > 5 files changed, 11 insertions(+), 55 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> > b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> > index 77e281bf4cb5..a8f36ca11e2e 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
>> > @@ -111,9 +111,7 @@ intel_plane_duplicate_state(struct drm_plane
>> > *plane)
>> > __drm_atomic_helper_plane_duplicate_state(plane,
>> > &intel_state->uapi);
>> >
>> > intel_state->ggtt_vma = NULL;
>> > -#ifdef I915
>> > intel_state->dpt_vma = NULL;
>> > -#endif
>> > intel_state->flags = 0;
>> >
>> > /* add reference to fb */
>> > @@ -138,9 +136,7 @@ intel_plane_destroy_state(struct drm_plane *plane,
>> > struct intel_plane_state *plane_state =
>> > to_intel_plane_state(state);
>> >
>> > drm_WARN_ON(plane->dev, plane_state->ggtt_vma); -#ifdef I915
>> > drm_WARN_ON(plane->dev, plane_state->dpt_vma); -#endif
>> >
>> > __drm_atomic_helper_plane_destroy_state(&plane_state->uapi);
>> > if (plane_state->hw.fb)
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>> > b/drivers/gpu/drm/i915/display/intel_display_types.h
>> > index b5e4b94c129b..db20dd7e5c0d 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> > @@ -143,9 +143,7 @@ struct intel_framebuffer {
>> > struct intel_fb_view remapped_view;
>> > };
>> >
>> > -#ifdef I915
>> > struct i915_address_space *dpt_vm; -#endif
>> > };
>> >
>> > enum intel_hotplug_state {
>> > @@ -696,11 +694,7 @@ struct intel_plane_state {
>> > } hw;
>> >
>> > struct i915_vma *ggtt_vma;
>> > -#ifdef I915
>> > struct i915_vma *dpt_vma;
>> > -#else
>> > - struct i915_vma embed_vma;
>> > -#endif
>> > unsigned long flags;
>> > #define PLANE_HAS_FENCE BIT(0)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c
>> > b/drivers/gpu/drm/i915/display/intel_dpt.c
>> > index 6e73c7a15942..2b067cb952f0 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dpt.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dpt.c
>> > @@ -3,6 +3,11 @@
>> > * Copyright © 2021 Intel Corporation
>> > */
>> >
>> > +#include "gem/i915_gem_domain.h"
>> > +#include "gem/i915_gem_internal.h"
>> > +#include "gem/i915_gem_lmem.h"
>> > +#include "gt/gen8_ppgtt.h"
>> > +
>> > #include "i915_drv.h"
>> > #include "i915_reg.h"
>> > #include "intel_de.h"
>> > @@ -10,13 +15,6 @@
>> > #include "intel_dpt.h"
>> > #include "intel_fb.h"
>> >
>> > -#ifdef I915
>> > -
>> > -#include "gem/i915_gem_domain.h"
>> > -#include "gem/i915_gem_internal.h"
>> > -#include "gem/i915_gem_lmem.h"
>> > -#include "gt/gen8_ppgtt.h"
>> > -
>> > struct i915_dpt {
>> > struct i915_address_space vm;
>> >
>> > @@ -320,8 +318,6 @@ void intel_dpt_destroy(struct i915_address_space
>> > *vm)
>> > i915_vm_put(&dpt->vm);
>> > }
>> >
>> > -#endif
>> > -
>> > void intel_dpt_configure(struct intel_crtc *crtc)
>> > {
>> > struct drm_i915_private *i915 = to_i915(crtc->base.dev); diff
>> > --git a/drivers/gpu/drm/i915/display/intel_fb.c
>> > b/drivers/gpu/drm/i915/display/intel_fb.c
>> > index 1ce61245728a..3862e55e8980 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_fb.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_fb.c
>> > @@ -1885,34 +1885,16 @@ int intel_plane_compute_gtt(struct
>> > intel_plane_state *plane_state)
>> > return intel_plane_check_stride(plane_state);
>> > }
>> >
>> > -static void intel_user_framebuffer_destroy_vm(struct drm_framebuffer
>> > *fb)
>> > -{
>> > -#ifdef I915
>> > - struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
>> > - if (intel_fb_uses_dpt(fb))
>> > - intel_dpt_destroy(intel_fb->dpt_vm);
>> > -#else
>> > - if (intel_fb_obj(fb)->flags & XE_BO_CREATE_PINNED_BIT) {
>> > - struct xe_bo *bo = intel_fb_obj(fb);
>> > -
>> > - /* Unpin our kernel fb first */
>> > - xe_bo_lock(bo, false);
>> > - xe_bo_unpin(bo);
>> > - xe_bo_unlock(bo);
>> > - }
>> > - xe_bo_put(intel_fb_obj(fb));
>> > -#endif
>> > -}
>> > -
>> > static void intel_user_framebuffer_destroy(struct drm_framebuffer
>> > *fb)
>> > {
>> > struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
>> >
>> > drm_framebuffer_cleanup(fb);
>> >
>> > - intel_frontbuffer_put(intel_fb->frontbuffer);
>> > + if (intel_fb_uses_dpt(fb))
>> > + intel_dpt_destroy(intel_fb->dpt_vm);
>> >
>> > - intel_user_framebuffer_destroy_vm(fb);
>> > + intel_frontbuffer_put(intel_fb->frontbuffer);
>> >
>> > kfree(intel_fb);
>> > }
>> > @@ -2155,18 +2137,13 @@ int intel_framebuffer_init(struct
>> > intel_framebuffer *intel_fb,
>> > }
>> > }
>> >
>> > -#ifdef I915
>> > fb->obj[i] = &obj->base; -#else
>> > - fb->obj[i] = &obj->ttm.base; -#endif
>> > }
>> >
>> > ret = intel_fill_fb_info(dev_priv, intel_fb);
>> > if (ret)
>> > goto err;
>> >
>> > -#ifdef I915
>> > if (intel_fb_uses_dpt(fb)) {
>> > struct i915_address_space *vm;
>> >
>> > @@ -2179,10 +2156,6 @@ int intel_framebuffer_init(struct
>> > intel_framebuffer *intel_fb,
>> >
>> > intel_fb->dpt_vm = vm;
>> > }
>> > -#else
>> > - /* Hold a reference to object while fb is alive */
>> > - xe_bo_get(obj);
>> > -#endif
>> >
>> > ret = drm_framebuffer_init(&dev_priv->drm, fb,
>> > &intel_fb_funcs);
>> > if (ret) {
>> > @@ -2193,7 +2166,8 @@ int intel_framebuffer_init(struct
>> > intel_framebuffer *intel_fb,
>> > return 0;
>> >
>> > err_free_dpt:
>> > - intel_user_framebuffer_destroy_vm(fb);
>> > + if (intel_fb_uses_dpt(fb))
>> > + intel_dpt_destroy(intel_fb->dpt_vm);
>> > err:
>> > intel_frontbuffer_put(intel_fb->frontbuffer);
>> > return ret;
>> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> > b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> > index 68eabe089aaf..4553dd6bfbbd 100644
>> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> > @@ -1010,13 +1010,9 @@ static u32 skl_surf_address(const struct
>> > intel_plane_state *plane_state,
>> > * The DPT object contains only one vma, so the VMA's
>> > offset
>> > * within the DPT is always 0.
>> > */
>> > - drm_WARN_ON(&i915->drm, offset & 0x1fffff); -#ifdef
>> > I915
>> > drm_WARN_ON(&i915->drm, plane_state->dpt_vma-
>> > >node.start);
>> > + drm_WARN_ON(&i915->drm, offset & 0x1fffff);
>> > return offset >> 9;
>> > -#else
>> > - return 0;
>> > -#endif
>> > } else {
>> > drm_WARN_ON(&i915->drm, offset & 0xfff);
>> > return offset;
>
--
Jani Nikula, Intel
next prev parent reply other threads:[~2023-11-15 12:59 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-13 19:56 [Intel-xe] [PATCH v9 1/6] Revert "FIXME: drm/i915: xe dpt integration" Juha-Pekka Heikkila
2023-11-13 19:56 ` [Intel-xe] [PATCH v9 2/6] drm/i915/display: Separate xe and i915 common dpt code into own file Juha-Pekka Heikkila
2023-11-13 19:56 ` [Intel-xe] [PATCH v9 3/6] drm/i915/display: in skl_surf_address check for dpt-vma Juha-Pekka Heikkila
2023-11-13 19:56 ` [Intel-xe] [PATCH v9 4/6] drm/i915/display: In intel_framebuffer_init switch to use intel_bo_to_drm_bo Juha-Pekka Heikkila
2023-11-13 19:56 ` [Intel-xe] [PATCH v9 5/6] fixup! drm/xe/display: Implement display support Juha-Pekka Heikkila
2023-11-13 19:56 ` [Intel-xe] [PATCH v9 6/6] FIXME: drm/i915: xe intel_fb.c framebuffer init and destroy xe changes Juha-Pekka Heikkila
2023-11-13 21:40 ` [Intel-xe] ✓ CI.Patch_applied: success for series starting with [v9,1/6] Revert "FIXME: drm/i915: xe dpt integration" Patchwork
2023-11-13 21:40 ` [Intel-xe] ✗ CI.checkpatch: warning " Patchwork
2023-11-13 21:42 ` [Intel-xe] ✓ CI.KUnit: success " Patchwork
2023-11-13 21:49 ` [Intel-xe] ✓ CI.Build: " Patchwork
2023-11-13 21:49 ` [Intel-xe] ✓ CI.Hooks: " Patchwork
2023-11-13 21:50 ` [Intel-xe] ✗ CI.checksparse: warning " Patchwork
2023-11-14 8:40 ` [Intel-xe] [v9, 1/6] " Hogander, Jouni
2023-11-14 15:34 ` Shankar, Uma
2023-11-15 12:59 ` Jani Nikula [this message]
2023-11-15 9:32 ` Hogander, Jouni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87fs17qijo.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=jouni.hogander@intel.com \
--cc=juhapekka.heikkila@gmail.com \
--cc=uma.shankar@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.