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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox