linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jonas Mark (BT-FIR/ENG1-Grb)" <Mark.Jonas@de.bosch.com>
To: Thomas Zimmermann <tzimmermann@suse.de>, Daniel Vetter <daniel@ffwll.ch>
Cc: "RUAN Tingquan (BT-FIR/ENG1-Zhu)" <Tingquan.Ruan@cn.bosch.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	David Airlie <airlied@linux.ie>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	NXP Linux Team <linux-imx@nxp.com>,
	"Shawn Guo" <shawnguo@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	"Jonas Mark (BT-FIR/ENG1-Grb)" <Mark.Jonas@de.bosch.com>
Subject: AW: [PATCH] drm: imx: Move fbdev setup to before output polling
Date: Wed, 18 Nov 2020 09:47:32 +0000	[thread overview]
Message-ID: <38c2d92ac5f04a228e55af43a12a4bd7@de.bosch.com> (raw)
In-Reply-To: <68af913c-9f4e-73b5-a2cb-8692902a2847@suse.de>

Hi Thomas and Daniel,

Thank you very much for your feedback. We appreciate it.

> >>> diff --git a/drivers/gpu/drm/imx/imx-drm-core.c
> >>> b/drivers/gpu/drm/imx/imx-drm-core.c
> >>> index 9bf5ad6d18a2..2665040e11c7 100644
> >>> --- a/drivers/gpu/drm/imx/imx-drm-core.c
> >>> +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> >>> @@ -240,14 +240,18 @@ static int imx_drm_bind(struct device *dev)
> >>>                legacyfb_depth = 16;
> >>>        }
> >>>
> >>> +     /*
> >>> +      * The generic fbdev has to be setup before enabling output polling.
> >>> +      * Otherwise the fbdev client is not ready to handle delayed events.
> >>> +      */
> >>> +     drm_fbdev_generic_setup(drm, legacyfb_depth);
> >>> +
> >>>        drm_kms_helper_poll_init(drm);
> >>>
> >>>        ret = drm_dev_register(drm, 0);
> >>>        if (ret)
> >>>                goto err_poll_fini;
> >>>
> >>> -     drm_fbdev_generic_setup(drm, legacyfb_depth);
> >>> -
> >>
> >> This does not work well. fbdev is supposed to be another regular DRM
> >> client. It has to be enabled after registering the DRM device.
> >>
> >> I'd rather improve fbdev or the driver to handle this gracefully.
> >
> > Yeah I'm not understanding the point here. Once fbcon is running, you
> > have a screen. Any fbdev userspace client  also should do a modeset
> > first. And if they dont and it's expected uapi for fbdev chardev that
> > the display boots up enabled, then we need to fix that in the fbdev
> > helpers, not through clever reordering in drivers so that a
> > side-effect causes a modeset.
> >
> > Note that this is a bit tricky since fbdev shouldn't take over the
> > screen by default, so we'd need to delay this until first open of
> > /dev/fb0. And we should probably also delay the hotplug handling until
> > the first open. fbcon also fake-opens the fbdev file, so it's the same
> > code path.
> 
> As far as I understand the commit message, the problem is that the display
> blanks out after registering the driver. And fbdev somewhat mitigates this by
> doing an early modeset. Users with fbdev disabled (most of them in
> embedded, I guess) would still run into the issue until userspace makes a
> modeset.
> 
> Mark, if that's the case, an option might be to pick up the device settings
> instead of calling drm_mode_config_reset(). The driver would then continue
> to display whatever is on the screen.

We are started using fbdev in our embedded application with Linux 3.10, later
updated to 4.14 and are now in the process of updating to 5.4. So far the uapi
appeared to us as if we could rely on an already enabled fbdev. That is, none
of our applications does a modeset.

When switching to 5.4 we noticed that the fbdev uapi changed. That is, the
LCD is switched off until it is explicitly enabled. It could be enabled by
writing to /sys/class/graphics/fb0/blank.

You are right, we are not using fbcon. fbcon will still enable the LCD but in our
embedded domain we have it disabled because we must not show a console.

Do we understand your proposal correctly to replace the call to
drm_mode_config_reset() in imx_drm_bind() [imx-drm-core.c] with picking up
the device settings instead?

https://elixir.bootlin.com/linux/v5.10-rc4/source/drivers/gpu/drm/imx/imx-drm-core.c#L231

We are a little clueless right now: How do we pick up the device settings?

Greetings,
Mark

  reply	other threads:[~2020-11-18  9:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-17 15:52 [PATCH] drm: imx: Move fbdev setup to before output polling Mark Jonas
2020-11-18  8:10 ` Thomas Zimmermann
2020-11-18  8:59   ` Daniel Vetter
2020-11-18  9:12     ` Thomas Zimmermann
2020-11-18  9:47       ` Jonas Mark (BT-FIR/ENG1-Grb) [this message]
2020-11-18 10:23         ` Daniel Vetter
2020-11-26  9:44           ` AW: " Jonas Mark (BT-FIR/ENG1-Grb)
2020-11-27 14:50             ` Daniel Vetter

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=38c2d92ac5f04a228e55af43a12a4bd7@de.bosch.com \
    --to=mark.jonas@de.bosch.com \
    --cc=Tingquan.Ruan@cn.bosch.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=tzimmermann@suse.de \
    /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;
as well as URLs for NNTP newsgroup(s).