All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: David Herrmann <dh.herrmann@gmail.com>
Cc: linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Tom Gundersen <teg@jklm.no>,
	Alexandre Courbot <acourbot@nvidia.com>,
	dri-devel@lists.freedesktop.org,
	Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>
Subject: Re: [PATCH 1/2] simplefb: fix unmapping fb during destruction
Date: Wed, 02 Oct 2013 16:16:34 +0000	[thread overview]
Message-ID: <524C46E2.5040804@wwwdotorg.org> (raw)
In-Reply-To: <1380725919-1961-1-git-send-email-dh.herrmann@gmail.com>

On 10/02/2013 08:58 AM, David Herrmann wrote:
> Unfortunately, fbdev does not create its own "struct device" for
> framebuffers. Instead, it attaches to the device of the parent layer. This
> has the side-effect that devm_* managed resources are not cleaned up on
> framebuffer-destruction but rather during destruction of the
> parent-device. In case of fbdev this might be too late, though.
> remove_conflicting_framebuffer() may remove fbdev devices but keep the
> parent device as it is.
> 
> Therefore, we now use plain ioremap() and unmap the framebuffer in the
> fb_destroy() callback. Note that we must not free the device here as this
> might race with the parent-device removal. Instead, we rely on
> unregister_framebuffer() as barrier and we're safe.

So, once the .fb_destroy callback has been executed, there's no other
callback to resurrect it? The framebuffer itself is still registered
until the device's remove, yet after .fb_destroy, the memory is
unmapped, which would be dangerous if the FB can be re-started.

If that's not an issue, this patch seems fine, so
Acked-by: Stephen Warren <swarren@nvidia.com>

> I know that simplefb was supposed to stay "as simple as possible" but I really
> think this series is the last set of fixes I have. Unfortunately framebuffer DRM
> handover is mandatory so we cannot ignore it in simplefb.

I don't think this patch adds any significant complexity, so I'm not
worried at least.

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren@wwwdotorg.org>
To: David Herrmann <dh.herrmann@gmail.com>
Cc: linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Tom Gundersen <teg@jklm.no>,
	Alexandre Courbot <acourbot@nvidia.com>,
	dri-devel@lists.freedesktop.org,
	Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>
Subject: Re: [PATCH 1/2] simplefb: fix unmapping fb during destruction
Date: Wed, 02 Oct 2013 10:16:34 -0600	[thread overview]
Message-ID: <524C46E2.5040804@wwwdotorg.org> (raw)
In-Reply-To: <1380725919-1961-1-git-send-email-dh.herrmann@gmail.com>

On 10/02/2013 08:58 AM, David Herrmann wrote:
> Unfortunately, fbdev does not create its own "struct device" for
> framebuffers. Instead, it attaches to the device of the parent layer. This
> has the side-effect that devm_* managed resources are not cleaned up on
> framebuffer-destruction but rather during destruction of the
> parent-device. In case of fbdev this might be too late, though.
> remove_conflicting_framebuffer() may remove fbdev devices but keep the
> parent device as it is.
> 
> Therefore, we now use plain ioremap() and unmap the framebuffer in the
> fb_destroy() callback. Note that we must not free the device here as this
> might race with the parent-device removal. Instead, we rely on
> unregister_framebuffer() as barrier and we're safe.

So, once the .fb_destroy callback has been executed, there's no other
callback to resurrect it? The framebuffer itself is still registered
until the device's remove, yet after .fb_destroy, the memory is
unmapped, which would be dangerous if the FB can be re-started.

If that's not an issue, this patch seems fine, so
Acked-by: Stephen Warren <swarren@nvidia.com>

> I know that simplefb was supposed to stay "as simple as possible" but I really
> think this series is the last set of fixes I have. Unfortunately framebuffer DRM
> handover is mandatory so we cannot ignore it in simplefb.

I don't think this patch adds any significant complexity, so I'm not
worried at least.

  parent reply	other threads:[~2013-10-02 16:16 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-02 14:58 [PATCH 1/2] simplefb: fix unmapping fb during destruction David Herrmann
2013-10-02 14:58 ` David Herrmann
2013-10-02 14:58 ` David Herrmann
2013-10-02 14:58 ` [PATCH 2/2] simplefb: use write-combined remapping David Herrmann
2013-10-02 14:58   ` David Herrmann
2013-10-30  7:49   ` David Herrmann
2013-10-30  7:49     ` David Herrmann
2013-10-30  7:49     ` David Herrmann
2013-10-30 10:54     ` Tomi Valkeinen
2013-10-30 10:54       ` Tomi Valkeinen
2013-10-02 16:16 ` Stephen Warren [this message]
2013-10-02 16:16   ` [PATCH 1/2] simplefb: fix unmapping fb during destruction Stephen Warren
2013-10-02 16:23   ` David Herrmann
2013-10-02 16:23     ` David Herrmann
2013-10-30  7:48     ` David Herrmann
2013-10-30  7:48       ` David Herrmann
2013-10-30  7:48       ` David Herrmann

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=524C46E2.5040804@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=acourbot@nvidia.com \
    --cc=dh.herrmann@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=plagnioj@jcrosoft.com \
    --cc=teg@jklm.no \
    --cc=tomi.valkeinen@ti.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.