dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
	Intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	Chris Wilson <chris@chris-wilson.co.uk>,
	Matthew Auld <matthew.auld@intel.com>
Subject: Re: [PATCH 1/2] drm/i915/overlay: Fix active retire callback alignment
Date: Fri, 30 Apr 2021 13:52:00 +0200	[thread overview]
Message-ID: <YIvvYO2IIzS0FS6f@phenom.ffwll.local> (raw)
In-Reply-To: <76f1488b-f0c3-f110-44ed-b7633256f132@linux.intel.com>

On Thu, Apr 29, 2021 at 06:34:51PM +0100, Tvrtko Ursulin wrote:
> 
> On 29/04/2021 17:31, Ville Syrjälä wrote:
> > On Thu, Apr 29, 2021 at 09:35:29AM +0100, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > 
> > > __i915_active_call annotation is required on the retire callback to ensure
> > > correct function alignment.
> > > 
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Fixes: a21ce8ad12d2 ("drm/i915/overlay: Switch to using i915_active tracking")
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_overlay.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
> > > index fffbde4256db..428819ba18dd 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_overlay.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_overlay.c
> > > @@ -383,7 +383,7 @@ static void intel_overlay_off_tail(struct intel_overlay *overlay)
> > >   		i830_overlay_clock_gating(dev_priv, true);
> > >   }
> > > -static void
> > > +__i915_active_call static void
> > 
> > Am I blind or are we just packing flag bits into a pointer, passing
> > that to a function, and then immediately unpack the bits again in
> > said function? Why not just pass the flags explicitly?
> > 
> > Looks like you missed auto_retire()?
> 
> Yeah, both points already either fixed or under consideration:
> https://patchwork.freedesktop.org/patch/431473/?series=89623&rev=1
> 
> I left the splitting up vfunc vs flags for later.

Yeah pls remove this pointer packing asap.

This is yet another case of pointless complications and fragility in the
code base for not reason at all, and it needs to go.

I'll file a jira and assign to Matt Auld, since he reviewed this
originally. I'll ping you in case you want to take it over.

Thanks, Daniel

> 
> Regards,
> 
> Tvrtko
> 
> > >   intel_overlay_last_flip_retire(struct i915_active *active)
> > >   {
> > >   	struct intel_overlay *overlay =
> > > -- 
> > > 2.30.2
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

      reply	other threads:[~2021-04-30 11:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-29  8:35 Tvrtko Ursulin
2021-04-29  8:35 ` [PATCH 2/2] drm/i915/selftests: " Tvrtko Ursulin
2021-04-29 16:31 ` [PATCH 1/2] drm/i915/overlay: " Ville Syrjälä
2021-04-29 17:31   ` [Intel-gfx] " Ville Syrjälä
2021-04-30 11:14     ` Tvrtko Ursulin
2021-04-29 17:34   ` Tvrtko Ursulin
2021-04-30 11:52     ` Daniel Vetter [this message]

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=YIvvYO2IIzS0FS6f@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=Intel-gfx@lists.freedesktop.org \
    --cc=chris@chris-wilson.co.uk \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    --cc=tvrtko.ursulin@intel.com \
    --cc=tvrtko.ursulin@linux.intel.com \
    --subject='Re: [PATCH 1/2] drm/i915/overlay: Fix active retire callback alignment' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).