The DRM_KMS_FB_HELPER config is selected only when DRM_MSM_FBDEV config is
selected. The driver accesses drm_fb_helper_* functions even when legacy fbdev
support is disabled in msm. Wrap around these functions with #ifdef checks to
prevent build break.
Signed-off-by: Archit Taneja <[email protected]>
---
drivers/gpu/drm/msm/msm_drv.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index a426911..f15494c 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -21,9 +21,11 @@
static void msm_fb_output_poll_changed(struct drm_device *dev)
{
+#ifdef CONFIG_DRM_MSM_FBDEV
struct msm_drm_private *priv = dev->dev_private;
if (priv->fbdev)
drm_fb_helper_hotplug_event(priv->fbdev);
+#endif
}
static const struct drm_mode_config_funcs mode_config_funcs = {
@@ -373,9 +375,11 @@ static void msm_preclose(struct drm_device *dev, struct drm_file *file)
static void msm_lastclose(struct drm_device *dev)
{
+#ifdef CONFIG_DRM_MSM_FBDEV
struct msm_drm_private *priv = dev->dev_private;
if (priv->fbdev)
drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
+#endif
}
static irqreturn_t msm_irq(int irq, void *arg)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
On Mon, Feb 23, 2015 at 5:29 AM, Archit Taneja <[email protected]> wrote:
> The DRM_KMS_FB_HELPER config is selected only when DRM_MSM_FBDEV config is
> selected. The driver accesses drm_fb_helper_* functions even when legacy fbdev
> support is disabled in msm. Wrap around these functions with #ifdef checks to
> prevent build break.
hmm, perhaps rather than solving this in each driver, we should do
some stub versions of those fb-helper fxns?
There are at least one or two other drivers that can build without
fbdev, and I guess more going forward..
BR,
-R
> Signed-off-by: Archit Taneja <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_drv.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index a426911..f15494c 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -21,9 +21,11 @@
>
> static void msm_fb_output_poll_changed(struct drm_device *dev)
> {
> +#ifdef CONFIG_DRM_MSM_FBDEV
> struct msm_drm_private *priv = dev->dev_private;
> if (priv->fbdev)
> drm_fb_helper_hotplug_event(priv->fbdev);
> +#endif
> }
>
> static const struct drm_mode_config_funcs mode_config_funcs = {
> @@ -373,9 +375,11 @@ static void msm_preclose(struct drm_device *dev, struct drm_file *file)
>
> static void msm_lastclose(struct drm_device *dev)
> {
> +#ifdef CONFIG_DRM_MSM_FBDEV
> struct msm_drm_private *priv = dev->dev_private;
> if (priv->fbdev)
> drm_fb_helper_restore_fbdev_mode_unlocked(priv->fbdev);
> +#endif
> }
>
> static irqreturn_t msm_irq(int irq, void *arg)
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
On Mon, Feb 23, 2015 at 08:33:36AM -0500, Rob Clark wrote:
> On Mon, Feb 23, 2015 at 5:29 AM, Archit Taneja <[email protected]> wrote:
> > The DRM_KMS_FB_HELPER config is selected only when DRM_MSM_FBDEV config is
> > selected. The driver accesses drm_fb_helper_* functions even when legacy fbdev
> > support is disabled in msm. Wrap around these functions with #ifdef checks to
> > prevent build break.
>
> hmm, perhaps rather than solving this in each driver, we should do
> some stub versions of those fb-helper fxns?
>
> There are at least one or two other drivers that can build without
> fbdev, and I guess more going forward..
It's not quite that easy since you also have to start/stop the vt
subsystem at the right point in time in your own driver. See
intel_fbdev_set_suspend. If you don't do that there's no synchronization
between fbcon shutting down/resuming and your driver, which in the best
case means fbcon does some writes to nowhere and worst case means your
chip dies (mmio to offline chip blocks) or writes go to somewhere random
in system memory (iommu contains some stale ptes since not yet fully
restored, more an issue with hibernate).
And because the console_lock is massively contended we do that in a async
worker in i915.
But anyway I agree it would still simply drivers quite a bit if we'd have
support for dummy fb helpers in the core, maybe with an explicit Kconfig.
Then drivers could switch to using that for the additional #ifdef (like
the vt stuff i915 does) and otherwise rely upon dummy static inline. That
would give us fbdev-less support for most drivers for free, which is kinda
neat.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Mon, Feb 23, 2015 at 9:09 AM, Daniel Vetter <[email protected]> wrote:
> On Mon, Feb 23, 2015 at 08:33:36AM -0500, Rob Clark wrote:
>> On Mon, Feb 23, 2015 at 5:29 AM, Archit Taneja <[email protected]> wrote:
>> > The DRM_KMS_FB_HELPER config is selected only when DRM_MSM_FBDEV config is
>> > selected. The driver accesses drm_fb_helper_* functions even when legacy fbdev
>> > support is disabled in msm. Wrap around these functions with #ifdef checks to
>> > prevent build break.
>>
>> hmm, perhaps rather than solving this in each driver, we should do
>> some stub versions of those fb-helper fxns?
>>
>> There are at least one or two other drivers that can build without
>> fbdev, and I guess more going forward..
>
> It's not quite that easy since you also have to start/stop the vt
> subsystem at the right point in time in your own driver. See
> intel_fbdev_set_suspend. If you don't do that there's no synchronization
> between fbcon shutting down/resuming and your driver, which in the best
> case means fbcon does some writes to nowhere and worst case means your
> chip dies (mmio to offline chip blocks) or writes go to somewhere random
> in system memory (iommu contains some stale ptes since not yet fully
> restored, more an issue with hibernate).
I guess I don't fully follow the vt/fbcon interaction if there is no
fbdev driver... but then again I don't have vesafb/efifb to contend
with, so I'm assuming something to do with that..
> And because the console_lock is massively contended we do that in a async
> worker in i915.
>
> But anyway I agree it would still simply drivers quite a bit if we'd have
> support for dummy fb helpers in the core, maybe with an explicit Kconfig.
> Then drivers could switch to using that for the additional #ifdef (like
> the vt stuff i915 does) and otherwise rely upon dummy static inline. That
> would give us fbdev-less support for most drivers for free, which is kinda
> neat.
I guess at least for all the arm drivers, life without fbdev doesn't
have these extra complications, so at least they could use stubs..
Plus, I kind of expect phone/tablet/chromebook type stuff would lead
the charge into an fbdev-less world..
BR,
-R
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Mon, Feb 23, 2015 at 10:03:21AM -0500, Rob Clark wrote:
> On Mon, Feb 23, 2015 at 9:09 AM, Daniel Vetter <[email protected]> wrote:
> > On Mon, Feb 23, 2015 at 08:33:36AM -0500, Rob Clark wrote:
> >> On Mon, Feb 23, 2015 at 5:29 AM, Archit Taneja <[email protected]> wrote:
> >> > The DRM_KMS_FB_HELPER config is selected only when DRM_MSM_FBDEV config is
> >> > selected. The driver accesses drm_fb_helper_* functions even when legacy fbdev
> >> > support is disabled in msm. Wrap around these functions with #ifdef checks to
> >> > prevent build break.
> >>
> >> hmm, perhaps rather than solving this in each driver, we should do
> >> some stub versions of those fb-helper fxns?
> >>
> >> There are at least one or two other drivers that can build without
> >> fbdev, and I guess more going forward..
> >
> > It's not quite that easy since you also have to start/stop the vt
> > subsystem at the right point in time in your own driver. See
> > intel_fbdev_set_suspend. If you don't do that there's no synchronization
> > between fbcon shutting down/resuming and your driver, which in the best
> > case means fbcon does some writes to nowhere and worst case means your
> > chip dies (mmio to offline chip blocks) or writes go to somewhere random
> > in system memory (iommu contains some stale ptes since not yet fully
> > restored, more an issue with hibernate).
>
> I guess I don't fully follow the vt/fbcon interaction if there is no
> fbdev driver... but then again I don't have vesafb/efifb to contend
> with, so I'm assuming something to do with that..
It's the other way round: There's interaction when we have fbdev enabled
beyond just calling a few fbdev helper functions. And we should compile
that out too since the console_lock is way too evil ;-)
Only with these additional #ifdefs is i915 completely console_lock free if
you disable i915 fbdev support. Just stubbing out the fbdev helper
functions is not enough.
> > And because the console_lock is massively contended we do that in a async
> > worker in i915.
> >
> > But anyway I agree it would still simply drivers quite a bit if we'd have
> > support for dummy fb helpers in the core, maybe with an explicit Kconfig.
> > Then drivers could switch to using that for the additional #ifdef (like
> > the vt stuff i915 does) and otherwise rely upon dummy static inline. That
> > would give us fbdev-less support for most drivers for free, which is kinda
> > neat.
>
> I guess at least for all the arm drivers, life without fbdev doesn't
> have these extra complications, so at least they could use stubs..
Does the problem sound more tricky with the above clarification? If you
don't do the fb_set_suspend call then I expect you'll have some
interesting problems.
> Plus, I kind of expect phone/tablet/chromebook type stuff would lead
> the charge into an fbdev-less world..
Yeah, that's another reason to support fbdev-less in the helpers instead
of each driver.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch