All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "Manna, Animesh" <animesh.manna@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 1/9] drm/i915/dsb: Replace HAS_DSB check with dsb->cmd_buf check
Date: Fri, 31 Jan 2020 13:42:52 +0200	[thread overview]
Message-ID: <20200131114252.GL13686@intel.com> (raw)
In-Reply-To: <5a7b6710-fb66-ca0e-666b-4b98c0e8052e@intel.com>

On Fri, Jan 31, 2020 at 03:04:17PM +0530, Manna, Animesh wrote:
> 
> On 30-01-2020 23:43, Souza, Jose wrote:
> > On Wed, 2020-01-29 at 20:20 +0200, Ville Syrjala wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> We may want to not use the DSB even if the platform has one.
> >> So replace the HAS_DSB check in the _put() with a cmd_buf check
> >> that will work in either case.
> > Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> >
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_dsb.c | 3 +--
> >>   1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> >> b/drivers/gpu/drm/i915/display/intel_dsb.c
> >> index 9dd18144a664..12776f09f227 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> >> @@ -160,9 +160,8 @@ intel_dsb_get(struct intel_crtc *crtc)
> >>   void intel_dsb_put(struct intel_dsb *dsb)
> >>   {
> >>   	struct intel_crtc *crtc = container_of(dsb, typeof(*crtc),
> >> dsb);
> >> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> >>   
> >> -	if (!HAS_DSB(i915))
> >> +	if (!dsb->cmd_buf)
> 
> Ville and Jose,
> 
> Have a concern here. In intel_dsb_get() if get failure during i915_gem_object_create_internal, i915_gem_object_ggtt_pin, i915_gem_object_pin_map then we may not have dsb->cmd_buf.
> Then ref-count mechanism will break.

Hmm. Yeah. The refcount WARN could easily be fixed by either
decrementung refcount on get() fail or doing the "let's never use
DSB" patch after the refcount inc.

> I feel HAS_DSB(i915) check is better than dsb->cmd_buf otherwise need to do some cleanup is intel_dsb_get() as well.
> 
> Regards,
> Animesh
> 
> >>   		return;
> >>   
> >>   	if (WARN_ON(dsb->refcount == 0))
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2020-01-31 11:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-29 18:20 [Intel-gfx] [PATCH 1/9] drm/i915/dsb: Replace HAS_DSB check with dsb->cmd_buf check Ville Syrjala
2020-01-29 18:20 ` [Intel-gfx] [PATCH 2/9] drm/i915/dsb: Disable DSB until fixed Ville Syrjala
2020-01-30 18:13   ` Souza, Jose
2020-01-29 18:20 ` [Intel-gfx] [PATCH 3/9] drm/i915/dsb: Turn the "DSB is busy" into an error Ville Syrjala
2020-02-03 12:36   ` Sharma, Swati2
2020-01-29 18:20 ` [Intel-gfx] [PATCH 4/9] drm/i915/dsb: Stop with the RMW Ville Syrjala
2020-01-29 18:20 ` [Intel-gfx] [PATCH 5/9] drm/i915/dsb: Unwind on map error Ville Syrjala
2020-01-29 18:20 ` [Intel-gfx] [PATCH 6/9] drm/i915/dsb: Inline DSB_CTRL writes into intel_dsb_commit() Ville Syrjala
2020-01-29 18:32   ` Chris Wilson
2020-01-29 18:44     ` Ville Syrjälä
2020-01-29 18:20 ` [Intel-gfx] [PATCH 7/9] drm/i915/dsb: Wait for DSB to idle after disabling it Ville Syrjala
2020-01-29 18:20 ` [Intel-gfx] [PATCH 8/9] drm/i915/dsb: Introduce intel_dsb_align_tail() Ville Syrjala
2020-01-29 18:20 ` [Intel-gfx] [PATCH 9/9] drm/i915/dsb: Nuke the 'dev' variables Ville Syrjala
2020-02-03 12:32   ` Sharma, Swati2
2020-01-30  0:47 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/9] drm/i915/dsb: Replace HAS_DSB check with dsb->cmd_buf check Patchwork
2020-01-30  1:09 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-01-30 15:11 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/9] drm/i915/dsb: Replace HAS_DSB check with dsb->cmd_buf check (rev2) Patchwork
2020-01-30 15:41 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-01-30 18:13 ` [Intel-gfx] [PATCH 1/9] drm/i915/dsb: Replace HAS_DSB check with dsb->cmd_buf check Souza, Jose
2020-01-31  9:34   ` Manna, Animesh
2020-01-31 11:42     ` Ville Syrjälä [this message]
2020-01-31 12:06       ` Manna, Animesh
2020-01-31 12:16         ` Ville Syrjälä
2020-03-03 10:43 ` Sharma, Swati2

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=20200131114252.GL13686@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=animesh.manna@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /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.