All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration
@ 2018-04-17 22:34 José Roberto de Souza
  2018-04-17 23:06 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: José Roberto de Souza @ 2018-04-17 22:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

If the initial fbdev configuration(intel_fbdev_initial_config()) runs and
there still no sink connected it will cause
drm_fb_helper_initial_config() to return 0 as no error happened(but
internally the return is -EAGAIN).
Because no framebuffer was allocated, when a sink is connected
intel_fbdev_output_poll_changed() will not execute
drm_fb_helper_hotplug_event() that would trigger another try to do the
initial fbdev configuration.

So here creating a dummy framebuffer of 800x600, so
drm_fb_helper_hotplug_event() will be executed and fbdev can be properly
setup when a sink is connected, if needed the dummy framebuffer will be
freed and a new with the proper size will be allocated.

This issue also happens when a MST DP sink is connected since boot, as
the MST topology is discovered in parallel if intel_fbdev_initial_config()
is executed before the first sink MST is discovered it will cause this
same issue.

This is a follow up patch of
https://patchwork.freedesktop.org/patch/196089/

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104158
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104425
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_fbdev.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 7d41d139341b..773577d39782 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -696,14 +696,36 @@ int intel_fbdev_init(struct drm_device *dev)
 	return 0;
 }
 
+static void intel_fbdev_dummy_fb_create(struct intel_fbdev *ifbdev)
+{
+	struct drm_fb_helper_surface_size sizes;
+
+	sizes.fb_width = 800;
+	sizes.fb_height = 600;
+	sizes.surface_width = sizes.fb_width;
+	sizes.surface_height = sizes.fb_height;
+	sizes.surface_bpp = 32;
+	sizes.surface_depth = 24;
+
+	if (intelfb_create(&ifbdev->helper, &sizes))
+		DRM_ERROR("Unable to create dummy framebufer");
+}
+
 static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
 {
 	struct intel_fbdev *ifbdev = data;
 
 	/* Due to peculiar init order wrt to hpd handling this is separate. */
 	if (drm_fb_helper_initial_config(&ifbdev->helper,
-					 ifbdev->preferred_bpp))
+					 ifbdev->preferred_bpp)) {
 		intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
+		return;
+	}
+
+	mutex_lock(&ifbdev->helper.lock);
+	if (!ifbdev->vma)
+		intel_fbdev_dummy_fb_create(ifbdev);
+	mutex_unlock(&ifbdev->helper.lock);
 }
 
 void intel_fbdev_initial_config_async(struct drm_device *dev)
-- 
2.17.0

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/fbdev: Enable late fbdev initial configuration
  2018-04-17 22:34 [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration José Roberto de Souza
@ 2018-04-17 23:06 ` Patchwork
  2018-04-17 23:44 ` [PATCH] " Chris Wilson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-04-17 23:06 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/fbdev: Enable late fbdev initial configuration
URL   : https://patchwork.freedesktop.org/series/41851/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4060 -> Patchwork_8714 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8714 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8714, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41851/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8714:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        SKIP -> PASS

    igt@kms_force_connector_basic@force-load-detect:
      fi-ivb-3520m:       PASS -> SKIP +3

    
== Known issues ==

  Here are the changes found in Patchwork_8714 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-ivb-3520m:       PASS -> DMESG-WARN (fdo#106084)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-glk-j4005:       DMESG-WARN (fdo#106097) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
  fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097


== Participating hosts (34 -> 32) ==

  Additional (1): fi-glk-1 
  Missing    (3): fi-ilk-m540 fi-cnl-y3 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4060 -> Patchwork_8714

  CI_DRM_4060: 17148956c3830de3194c17693be76f85f05f692f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4441: 83ba5b7d3bde48b383df41792fc9c955a5a23bdb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8714: 07a7c687e23721dccc869d9c0a4e2b0b1e817396 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4441: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

07a7c687e237 drm/i915/fbdev: Enable late fbdev initial configuration

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8714/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration
  2018-04-17 22:34 [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration José Roberto de Souza
  2018-04-17 23:06 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-04-17 23:44 ` Chris Wilson
  2018-04-18  0:07   ` Souza, Jose
  2018-04-17 23:51 ` ✗ Fi.CI.BAT: failure for drm/i915/fbdev: Enable late fbdev initial configuration (rev2) Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2018-04-17 23:44 UTC (permalink / raw)
  To: José Roberto de Souza, intel-gfx; +Cc: Rodrigo

Quoting José Roberto de Souza (2018-04-17 23:34:18)
> If the initial fbdev configuration(intel_fbdev_initial_config()) runs and
> there still no sink connected it will cause
> drm_fb_helper_initial_config() to return 0 as no error happened(but
> internally the return is -EAGAIN).
> Because no framebuffer was allocated, when a sink is connected
> intel_fbdev_output_poll_changed() will not execute
> drm_fb_helper_hotplug_event() that would trigger another try to do the
> initial fbdev configuration.
> 
> So here creating a dummy framebuffer of 800x600, so
> drm_fb_helper_hotplug_event() will be executed and fbdev can be properly
> setup when a sink is connected, if needed the dummy framebuffer will be
> freed and a new with the proper size will be allocated.
> 
> This issue also happens when a MST DP sink is connected since boot, as
> the MST topology is discovered in parallel if intel_fbdev_initial_config()
> is executed before the first sink MST is discovered it will cause this
> same issue.
> 
> This is a follow up patch of
> https://patchwork.freedesktop.org/patch/196089/
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104158
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104425
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 7d41d139341b..773577d39782 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -696,14 +696,36 @@ int intel_fbdev_init(struct drm_device *dev)
>         return 0;
>  }
>  
> +static void intel_fbdev_dummy_fb_create(struct intel_fbdev *ifbdev)
> +{
> +       struct drm_fb_helper_surface_size sizes;
> +
> +       sizes.fb_width = 800;
> +       sizes.fb_height = 600;
> +       sizes.surface_width = sizes.fb_width;
> +       sizes.surface_height = sizes.fb_height;
> +       sizes.surface_bpp = 32;
> +       sizes.surface_depth = 24;
> +
> +       if (intelfb_create(&ifbdev->helper, &sizes))
> +               DRM_ERROR("Unable to create dummy framebufer");
> +}
> +
>  static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
>  {
>         struct intel_fbdev *ifbdev = data;
>  
>         /* Due to peculiar init order wrt to hpd handling this is separate. */
>         if (drm_fb_helper_initial_config(&ifbdev->helper,
> -                                        ifbdev->preferred_bpp))
> +                                        ifbdev->preferred_bpp)) {
>                 intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
> +               return;
> +       }
> +
> +       mutex_lock(&ifbdev->helper.lock);
> +       if (!ifbdev->vma)
> +               intel_fbdev_dummy_fb_create(ifbdev);
> +       mutex_unlock(&ifbdev->helper.lock);
>  }

Did you try

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 65a3313723c9..4120c635742d 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -493,6 +493,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 bail:
                DRM_DEBUG_KMS("Not using firmware configuration\n");
                memcpy(enabled, save_enabled, count);
+               fb_helper->deferred_setup = true;
                ret = false;
        }

possible as of 

commit ca91a2758fcef6635626993557dd51cfbb6dd134
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Jul 6 15:00:21 2017 +0200

    drm/fb-helper: Support deferred setup
    
    FB helper code falls back to a 1024x768 mode if no outputs are connected
    or don't report back any modes upon initialization. This can be annoying
    because outputs that are added to FB helper later on can't be used with
    FB helper if they don't support a matching mode.
    ...

-Chris
 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/fbdev: Enable late fbdev initial configuration (rev2)
  2018-04-17 22:34 [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration José Roberto de Souza
  2018-04-17 23:06 ` ✓ Fi.CI.BAT: success for " Patchwork
  2018-04-17 23:44 ` [PATCH] " Chris Wilson
@ 2018-04-17 23:51 ` Patchwork
  2018-04-18  0:28 ` ✓ Fi.CI.IGT: success for drm/i915/fbdev: Enable late fbdev initial configuration Patchwork
  2018-04-18 14:34 ` ✗ Fi.CI.BAT: failure for drm/i915/fbdev: Enable late fbdev initial configuration (rev3) Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-04-17 23:51 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/fbdev: Enable late fbdev initial configuration (rev2)
URL   : https://patchwork.freedesktop.org/series/41851/
State : failure

== Summary ==

Applying: drm/i915/fbdev: Enable late fbdev initial configuration
error: patch failed: drivers/gpu/drm/i915/intel_fbdev.c:493
error: drivers/gpu/drm/i915/intel_fbdev.c: patch does not apply
error: Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/intel_fbdev.c
Patch failed at 0001 drm/i915/fbdev: Enable late fbdev initial configuration
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration
  2018-04-17 23:44 ` [PATCH] " Chris Wilson
@ 2018-04-18  0:07   ` Souza, Jose
  2018-04-18  8:26     ` Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Souza, Jose @ 2018-04-18  0:07 UTC (permalink / raw)
  To: intel-gfx, chris; +Cc: Vivi, Rodrigo

On Wed, 2018-04-18 at 00:44 +0100, Chris Wilson wrote:
> Quoting José Roberto de Souza (2018-04-17 23:34:18)
> > If the initial fbdev configuration(intel_fbdev_initial_config())
> > runs and
> > there still no sink connected it will cause
> > drm_fb_helper_initial_config() to return 0 as no error happened(but
> > internally the return is -EAGAIN).
> > Because no framebuffer was allocated, when a sink is connected
> > intel_fbdev_output_poll_changed() will not execute
> > drm_fb_helper_hotplug_event() that would trigger another try to do
> > the
> > initial fbdev configuration.
> > 
> > So here creating a dummy framebuffer of 800x600, so
> > drm_fb_helper_hotplug_event() will be executed and fbdev can be
> > properly
> > setup when a sink is connected, if needed the dummy framebuffer
> > will be
> > freed and a new with the proper size will be allocated.
> > 
> > This issue also happens when a MST DP sink is connected since boot,
> > as
> > the MST topology is discovered in parallel if
> > intel_fbdev_initial_config()
> > is executed before the first sink MST is discovered it will cause
> > this
> > same issue.
> > 
> > This is a follow up patch of
> > https://patchwork.freedesktop.org/patch/196089/
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104158
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104425
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_fbdev.c | 24 +++++++++++++++++++++++-
> >  1 file changed, 23 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > b/drivers/gpu/drm/i915/intel_fbdev.c
> > index 7d41d139341b..773577d39782 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -696,14 +696,36 @@ int intel_fbdev_init(struct drm_device *dev)
> >         return 0;
> >  }
> >  
> > +static void intel_fbdev_dummy_fb_create(struct intel_fbdev
> > *ifbdev)
> > +{
> > +       struct drm_fb_helper_surface_size sizes;
> > +
> > +       sizes.fb_width = 800;
> > +       sizes.fb_height = 600;
> > +       sizes.surface_width = sizes.fb_width;
> > +       sizes.surface_height = sizes.fb_height;
> > +       sizes.surface_bpp = 32;
> > +       sizes.surface_depth = 24;
> > +
> > +       if (intelfb_create(&ifbdev->helper, &sizes))
> > +               DRM_ERROR("Unable to create dummy framebufer");
> > +}
> > +
> >  static void intel_fbdev_initial_config(void *data, async_cookie_t
> > cookie)
> >  {
> >         struct intel_fbdev *ifbdev = data;
> >  
> >         /* Due to peculiar init order wrt to hpd handling this is
> > separate. */
> >         if (drm_fb_helper_initial_config(&ifbdev->helper,
> > -                                        ifbdev->preferred_bpp))
> > +                                        ifbdev->preferred_bpp)) {
> >                 intel_fbdev_unregister(to_i915(ifbdev-
> > >helper.dev));
> > +               return;
> > +       }
> > +
> > +       mutex_lock(&ifbdev->helper.lock);
> > +       if (!ifbdev->vma)
> > +               intel_fbdev_dummy_fb_create(ifbdev);
> > +       mutex_unlock(&ifbdev->helper.lock);
> >  }
> 
> Did you try
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index 65a3313723c9..4120c635742d 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -493,6 +493,7 @@ static bool intel_fb_initial_config(struct
> drm_fb_helper *fb_helper,
>  bail:
>                 DRM_DEBUG_KMS("Not using firmware configuration\n");
>                 memcpy(enabled, save_enabled, count);
> +               fb_helper->deferred_setup = true;

This is too earlier to set deferred_setup when
intel_fb_initial_config() fails,
__drm_fb_helper_initial_config_and_unlock() will then try to
probe/create a fb but as there is no sink connected it will fail too.

Also __drm_fb_helper_initial_config_and_unlock() is already setting
deferred_setup if both attempts fails.

>                 ret = false;
>         }
> 
> possible as of 
> 
> commit ca91a2758fcef6635626993557dd51cfbb6dd134
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Thu Jul 6 15:00:21 2017 +0200
> 
>     drm/fb-helper: Support deferred setup
>     
>     FB helper code falls back to a 1024x768 mode if no outputs are
> connected
>     or don't report back any modes upon initialization. This can be
> annoying
>     because outputs that are added to FB helper later on can't be
> used with
>     FB helper if they don't support a matching mode.
>     ...
> 
> -Chris
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915/fbdev: Enable late fbdev initial configuration
  2018-04-17 22:34 [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration José Roberto de Souza
                   ` (2 preceding siblings ...)
  2018-04-17 23:51 ` ✗ Fi.CI.BAT: failure for drm/i915/fbdev: Enable late fbdev initial configuration (rev2) Patchwork
@ 2018-04-18  0:28 ` Patchwork
  2018-04-18 14:34 ` ✗ Fi.CI.BAT: failure for drm/i915/fbdev: Enable late fbdev initial configuration (rev3) Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-04-18  0:28 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/fbdev: Enable late fbdev initial configuration
URL   : https://patchwork.freedesktop.org/series/41851/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4060_full -> Patchwork_8714_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8714_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8714_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41851/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8714_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          SKIP -> PASS +1

    igt@gem_mocs_settings@mocs-rc6-dirty-render:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_8714_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_atomic_transition@1x-modeset-transitions-fencing:
      shard-hsw:          PASS -> DMESG-WARN (fdo#102614)

    igt@kms_flip@modeset-vs-vblank-race:
      shard-apl:          PASS -> FAIL (fdo#103060)

    igt@kms_sysfs_edid_timing:
      shard-apl:          PASS -> WARN (fdo#100047)

    
    ==== Possible fixes ====

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106023) -> PASS

    igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
      shard-hsw:          FAIL (fdo#104873) -> PASS

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023


== Participating hosts (6 -> 4) ==

  Missing    (2): shard-glk shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4060 -> Patchwork_8714

  CI_DRM_4060: 17148956c3830de3194c17693be76f85f05f692f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4441: 83ba5b7d3bde48b383df41792fc9c955a5a23bdb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8714: 07a7c687e23721dccc869d9c0a4e2b0b1e817396 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4441: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8714/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration
  2018-04-18  0:07   ` Souza, Jose
@ 2018-04-18  8:26     ` Chris Wilson
  2018-04-18 16:42       ` Souza, Jose
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2018-04-18  8:26 UTC (permalink / raw)
  To: Souza, Jose, intel-gfx; +Cc: Vivi, Rodrigo

Quoting Souza, Jose (2018-04-18 01:07:16)
> On Wed, 2018-04-18 at 00:44 +0100, Chris Wilson wrote:
> > Quoting José Roberto de Souza (2018-04-17 23:34:18)
> > > If the initial fbdev configuration(intel_fbdev_initial_config())
> > > runs and
> > > there still no sink connected it will cause
> > > drm_fb_helper_initial_config() to return 0 as no error happened(but
> > > internally the return is -EAGAIN).
> > > Because no framebuffer was allocated, when a sink is connected
> > > intel_fbdev_output_poll_changed() will not execute
> > > drm_fb_helper_hotplug_event() that would trigger another try to do
> > > the
> > > initial fbdev configuration.
> > > 
> > > So here creating a dummy framebuffer of 800x600, so
> > > drm_fb_helper_hotplug_event() will be executed and fbdev can be
> > > properly
> > > setup when a sink is connected, if needed the dummy framebuffer
> > > will be
> > > freed and a new with the proper size will be allocated.
> > > 
> > > This issue also happens when a MST DP sink is connected since boot,
> > > as
> > > the MST topology is discovered in parallel if
> > > intel_fbdev_initial_config()
> > > is executed before the first sink MST is discovered it will cause
> > > this
> > > same issue.
> > > 
> > > This is a follow up patch of
> > > https://patchwork.freedesktop.org/patch/196089/
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104158
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104425
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_fbdev.c | 24 +++++++++++++++++++++++-
> > >  1 file changed, 23 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > > b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index 7d41d139341b..773577d39782 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -696,14 +696,36 @@ int intel_fbdev_init(struct drm_device *dev)
> > >         return 0;
> > >  }
> > >  
> > > +static void intel_fbdev_dummy_fb_create(struct intel_fbdev
> > > *ifbdev)
> > > +{
> > > +       struct drm_fb_helper_surface_size sizes;
> > > +
> > > +       sizes.fb_width = 800;
> > > +       sizes.fb_height = 600;
> > > +       sizes.surface_width = sizes.fb_width;
> > > +       sizes.surface_height = sizes.fb_height;
> > > +       sizes.surface_bpp = 32;
> > > +       sizes.surface_depth = 24;
> > > +
> > > +       if (intelfb_create(&ifbdev->helper, &sizes))
> > > +               DRM_ERROR("Unable to create dummy framebufer");
> > > +}
> > > +
> > >  static void intel_fbdev_initial_config(void *data, async_cookie_t
> > > cookie)
> > >  {
> > >         struct intel_fbdev *ifbdev = data;
> > >  
> > >         /* Due to peculiar init order wrt to hpd handling this is
> > > separate. */
> > >         if (drm_fb_helper_initial_config(&ifbdev->helper,
> > > -                                        ifbdev->preferred_bpp))
> > > +                                        ifbdev->preferred_bpp)) {
> > >                 intel_fbdev_unregister(to_i915(ifbdev-
> > > >helper.dev));
> > > +               return;
> > > +       }
> > > +
> > > +       mutex_lock(&ifbdev->helper.lock);
> > > +       if (!ifbdev->vma)
> > > +               intel_fbdev_dummy_fb_create(ifbdev);
> > > +       mutex_unlock(&ifbdev->helper.lock);
> > >  }
> > 
> > Did you try
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > b/drivers/gpu/drm/i915/intel_fbdev.c
> > index 65a3313723c9..4120c635742d 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -493,6 +493,7 @@ static bool intel_fb_initial_config(struct
> > drm_fb_helper *fb_helper,
> >  bail:
> >                 DRM_DEBUG_KMS("Not using firmware configuration\n");
> >                 memcpy(enabled, save_enabled, count);
> > +               fb_helper->deferred_setup = true;
> 
> This is too earlier to set deferred_setup when
> intel_fb_initial_config() fails,
> __drm_fb_helper_initial_config_and_unlock() will then try to
> probe/create a fb but as there is no sink connected it will fail too.
> 
> Also __drm_fb_helper_initial_config_and_unlock() is already setting
> deferred_setup if both attempts fails.

So...

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 7d41d139341b..5f138a03dd2a 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -807,7 +807,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
                return;
 
        intel_fbdev_sync(ifbdev);
-       if (ifbdev->vma)
+       if (ifbdev->vma || ifbdev->helper->deferred_setup)
                drm_fb_helper_hotplug_event(&ifbdev->helper);
 }

?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/fbdev: Enable late fbdev initial configuration (rev3)
  2018-04-17 22:34 [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration José Roberto de Souza
                   ` (3 preceding siblings ...)
  2018-04-18  0:28 ` ✓ Fi.CI.IGT: success for drm/i915/fbdev: Enable late fbdev initial configuration Patchwork
@ 2018-04-18 14:34 ` Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2018-04-18 14:34 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/fbdev: Enable late fbdev initial configuration (rev3)
URL   : https://patchwork.freedesktop.org/series/41851/
State : failure

== Summary ==

Applying: drm/i915/fbdev: Enable late fbdev initial configuration
error: patch failed: drivers/gpu/drm/i915/intel_fbdev.c:807
error: drivers/gpu/drm/i915/intel_fbdev.c: patch does not apply
error: Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Using index info to reconstruct a base tree...
Patch failed at 0001 drm/i915/fbdev: Enable late fbdev initial configuration
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration
  2018-04-18  8:26     ` Chris Wilson
@ 2018-04-18 16:42       ` Souza, Jose
  2018-04-18 16:55         ` Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Souza, Jose @ 2018-04-18 16:42 UTC (permalink / raw)
  To: intel-gfx, chris; +Cc: Vivi, Rodrigo

On Wed, 2018-04-18 at 09:26 +0100, Chris Wilson wrote:
> Quoting Souza, Jose (2018-04-18 01:07:16)
> > On Wed, 2018-04-18 at 00:44 +0100, Chris Wilson wrote:
> > > Quoting José Roberto de Souza (2018-04-17 23:34:18)
> > > > If the initial fbdev
> > > > configuration(intel_fbdev_initial_config())
> > > > runs and
> > > > there still no sink connected it will cause
> > > > drm_fb_helper_initial_config() to return 0 as no error
> > > > happened(but
> > > > internally the return is -EAGAIN).
> > > > Because no framebuffer was allocated, when a sink is connected
> > > > intel_fbdev_output_poll_changed() will not execute
> > > > drm_fb_helper_hotplug_event() that would trigger another try to
> > > > do
> > > > the
> > > > initial fbdev configuration.
> > > > 
> > > > So here creating a dummy framebuffer of 800x600, so
> > > > drm_fb_helper_hotplug_event() will be executed and fbdev can be
> > > > properly
> > > > setup when a sink is connected, if needed the dummy framebuffer
> > > > will be
> > > > freed and a new with the proper size will be allocated.
> > > > 
> > > > This issue also happens when a MST DP sink is connected since
> > > > boot,
> > > > as
> > > > the MST topology is discovered in parallel if
> > > > intel_fbdev_initial_config()
> > > > is executed before the first sink MST is discovered it will
> > > > cause
> > > > this
> > > > same issue.
> > > > 
> > > > This is a follow up patch of
> > > > https://patchwork.freedesktop.org/patch/196089/
> > > > 
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104158
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104425
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_fbdev.c | 24
> > > > +++++++++++++++++++++++-
> > > >  1 file changed, 23 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > > > b/drivers/gpu/drm/i915/intel_fbdev.c
> > > > index 7d41d139341b..773577d39782 100644
> > > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > > @@ -696,14 +696,36 @@ int intel_fbdev_init(struct drm_device
> > > > *dev)
> > > >         return 0;
> > > >  }
> > > >  
> > > > +static void intel_fbdev_dummy_fb_create(struct intel_fbdev
> > > > *ifbdev)
> > > > +{
> > > > +       struct drm_fb_helper_surface_size sizes;
> > > > +
> > > > +       sizes.fb_width = 800;
> > > > +       sizes.fb_height = 600;
> > > > +       sizes.surface_width = sizes.fb_width;
> > > > +       sizes.surface_height = sizes.fb_height;
> > > > +       sizes.surface_bpp = 32;
> > > > +       sizes.surface_depth = 24;
> > > > +
> > > > +       if (intelfb_create(&ifbdev->helper, &sizes))
> > > > +               DRM_ERROR("Unable to create dummy framebufer");
> > > > +}
> > > > +
> > > >  static void intel_fbdev_initial_config(void *data,
> > > > async_cookie_t
> > > > cookie)
> > > >  {
> > > >         struct intel_fbdev *ifbdev = data;
> > > >  
> > > >         /* Due to peculiar init order wrt to hpd handling this
> > > > is
> > > > separate. */
> > > >         if (drm_fb_helper_initial_config(&ifbdev->helper,
> > > > -                                        ifbdev-
> > > > >preferred_bpp))
> > > > +                                        ifbdev-
> > > > >preferred_bpp)) {
> > > >                 intel_fbdev_unregister(to_i915(ifbdev-
> > > > > helper.dev));
> > > > 
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       mutex_lock(&ifbdev->helper.lock);
> > > > +       if (!ifbdev->vma)
> > > > +               intel_fbdev_dummy_fb_create(ifbdev);
> > > > +       mutex_unlock(&ifbdev->helper.lock);
> > > >  }
> > > 
> > > Did you try
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > > b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index 65a3313723c9..4120c635742d 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -493,6 +493,7 @@ static bool intel_fb_initial_config(struct
> > > drm_fb_helper *fb_helper,
> > >  bail:
> > >                 DRM_DEBUG_KMS("Not using firmware
> > > configuration\n");
> > >                 memcpy(enabled, save_enabled, count);
> > > +               fb_helper->deferred_setup = true;
> > 
> > This is too earlier to set deferred_setup when
> > intel_fb_initial_config() fails,
> > __drm_fb_helper_initial_config_and_unlock() will then try to
> > probe/create a fb but as there is no sink connected it will fail
> > too.
> > 
> > Also __drm_fb_helper_initial_config_and_unlock() is already setting
> > deferred_setup if both attempts fails.
> 
> So...
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index 7d41d139341b..5f138a03dd2a 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -807,7 +807,7 @@ void intel_fbdev_output_poll_changed(struct
> drm_device *dev)
>                 return;
>  
>         intel_fbdev_sync(ifbdev);
> -       if (ifbdev->vma)
> +       if (ifbdev->vma || ifbdev->helper->deferred_setup)
>                 drm_fb_helper_hotplug_event(&ifbdev->helper);
>  }

That would work but according to your comment here: https://patchwork.f
reedesktop.org/patch/196089/ that would lead to a crash in some case.

> 
> ?
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration
  2018-04-18 16:42       ` Souza, Jose
@ 2018-04-18 16:55         ` Chris Wilson
  2018-04-18 18:00           ` Souza, Jose
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2018-04-18 16:55 UTC (permalink / raw)
  To: Souza, Jose, intel-gfx; +Cc: Vivi, Rodrigo

Quoting Souza, Jose (2018-04-18 17:42:47)
> On Wed, 2018-04-18 at 09:26 +0100, Chris Wilson wrote:
> > Quoting Souza, Jose (2018-04-18 01:07:16)
> > > On Wed, 2018-04-18 at 00:44 +0100, Chris Wilson wrote:
> > > > Quoting José Roberto de Souza (2018-04-17 23:34:18)
> > > > > If the initial fbdev
> > > > > configuration(intel_fbdev_initial_config())
> > > > > runs and
> > > > > there still no sink connected it will cause
> > > > > drm_fb_helper_initial_config() to return 0 as no error
> > > > > happened(but
> > > > > internally the return is -EAGAIN).
> > > > > Because no framebuffer was allocated, when a sink is connected
> > > > > intel_fbdev_output_poll_changed() will not execute
> > > > > drm_fb_helper_hotplug_event() that would trigger another try to
> > > > > do
> > > > > the
> > > > > initial fbdev configuration.
> > > > > 
> > > > > So here creating a dummy framebuffer of 800x600, so
> > > > > drm_fb_helper_hotplug_event() will be executed and fbdev can be
> > > > > properly
> > > > > setup when a sink is connected, if needed the dummy framebuffer
> > > > > will be
> > > > > freed and a new with the proper size will be allocated.
> > > > > 
> > > > > This issue also happens when a MST DP sink is connected since
> > > > > boot,
> > > > > as
> > > > > the MST topology is discovered in parallel if
> > > > > intel_fbdev_initial_config()
> > > > > is executed before the first sink MST is discovered it will
> > > > > cause
> > > > > this
> > > > > same issue.
> > > > > 
> > > > > This is a follow up patch of
> > > > > https://patchwork.freedesktop.org/patch/196089/
> > > > > 
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104158
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104425
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_fbdev.c | 24
> > > > > +++++++++++++++++++++++-
> > > > >  1 file changed, 23 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > b/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > index 7d41d139341b..773577d39782 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > @@ -696,14 +696,36 @@ int intel_fbdev_init(struct drm_device
> > > > > *dev)
> > > > >         return 0;
> > > > >  }
> > > > >  
> > > > > +static void intel_fbdev_dummy_fb_create(struct intel_fbdev
> > > > > *ifbdev)
> > > > > +{
> > > > > +       struct drm_fb_helper_surface_size sizes;
> > > > > +
> > > > > +       sizes.fb_width = 800;
> > > > > +       sizes.fb_height = 600;
> > > > > +       sizes.surface_width = sizes.fb_width;
> > > > > +       sizes.surface_height = sizes.fb_height;
> > > > > +       sizes.surface_bpp = 32;
> > > > > +       sizes.surface_depth = 24;
> > > > > +
> > > > > +       if (intelfb_create(&ifbdev->helper, &sizes))
> > > > > +               DRM_ERROR("Unable to create dummy framebufer");
> > > > > +}
> > > > > +
> > > > >  static void intel_fbdev_initial_config(void *data,
> > > > > async_cookie_t
> > > > > cookie)
> > > > >  {
> > > > >         struct intel_fbdev *ifbdev = data;
> > > > >  
> > > > >         /* Due to peculiar init order wrt to hpd handling this
> > > > > is
> > > > > separate. */
> > > > >         if (drm_fb_helper_initial_config(&ifbdev->helper,
> > > > > -                                        ifbdev-
> > > > > >preferred_bpp))
> > > > > +                                        ifbdev-
> > > > > >preferred_bpp)) {
> > > > >                 intel_fbdev_unregister(to_i915(ifbdev-
> > > > > > helper.dev));
> > > > > 
> > > > > +               return;
> > > > > +       }
> > > > > +
> > > > > +       mutex_lock(&ifbdev->helper.lock);
> > > > > +       if (!ifbdev->vma)
> > > > > +               intel_fbdev_dummy_fb_create(ifbdev);
> > > > > +       mutex_unlock(&ifbdev->helper.lock);
> > > > >  }
> > > > 
> > > > Did you try
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > > > b/drivers/gpu/drm/i915/intel_fbdev.c
> > > > index 65a3313723c9..4120c635742d 100644
> > > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > > @@ -493,6 +493,7 @@ static bool intel_fb_initial_config(struct
> > > > drm_fb_helper *fb_helper,
> > > >  bail:
> > > >                 DRM_DEBUG_KMS("Not using firmware
> > > > configuration\n");
> > > >                 memcpy(enabled, save_enabled, count);
> > > > +               fb_helper->deferred_setup = true;
> > > 
> > > This is too earlier to set deferred_setup when
> > > intel_fb_initial_config() fails,
> > > __drm_fb_helper_initial_config_and_unlock() will then try to
> > > probe/create a fb but as there is no sink connected it will fail
> > > too.
> > > 
> > > Also __drm_fb_helper_initial_config_and_unlock() is already setting
> > > deferred_setup if both attempts fails.
> > 
> > So...
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > b/drivers/gpu/drm/i915/intel_fbdev.c
> > index 7d41d139341b..5f138a03dd2a 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -807,7 +807,7 @@ void intel_fbdev_output_poll_changed(struct
> > drm_device *dev)
> >                 return;
> >  
> >         intel_fbdev_sync(ifbdev);
> > -       if (ifbdev->vma)
> > +       if (ifbdev->vma || ifbdev->helper->deferred_setup)
> >                 drm_fb_helper_hotplug_event(&ifbdev->helper);
> >  }
> 
> That would work but according to your comment here: https://patchwork.f
> reedesktop.org/patch/196089/ that would lead to a crash in some case.

In the deferred_setup case, won't it go to intelfb_create() and after
that point if we still have no ifdev->vma we won't hit any other pathway?

Just because it's been historically buggy (at least 2 instances now of
having to apply protection here), it may have been fixed... Unlikely,
but maybe it all finally works.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration
  2018-04-18 16:55         ` Chris Wilson
@ 2018-04-18 18:00           ` Souza, Jose
  0 siblings, 0 replies; 11+ messages in thread
From: Souza, Jose @ 2018-04-18 18:00 UTC (permalink / raw)
  To: intel-gfx, chris; +Cc: Vivi, Rodrigo

On Wed, 2018-04-18 at 17:55 +0100, Chris Wilson wrote:
> Quoting Souza, Jose (2018-04-18 17:42:47)
> > On Wed, 2018-04-18 at 09:26 +0100, Chris Wilson wrote:
> > > Quoting Souza, Jose (2018-04-18 01:07:16)
> > > > On Wed, 2018-04-18 at 00:44 +0100, Chris Wilson wrote:
> > > > > Quoting José Roberto de Souza (2018-04-17 23:34:18)
> > > > > > If the initial fbdev
> > > > > > configuration(intel_fbdev_initial_config())
> > > > > > runs and
> > > > > > there still no sink connected it will cause
> > > > > > drm_fb_helper_initial_config() to return 0 as no error
> > > > > > happened(but
> > > > > > internally the return is -EAGAIN).
> > > > > > Because no framebuffer was allocated, when a sink is
> > > > > > connected
> > > > > > intel_fbdev_output_poll_changed() will not execute
> > > > > > drm_fb_helper_hotplug_event() that would trigger another
> > > > > > try to
> > > > > > do
> > > > > > the
> > > > > > initial fbdev configuration.
> > > > > > 
> > > > > > So here creating a dummy framebuffer of 800x600, so
> > > > > > drm_fb_helper_hotplug_event() will be executed and fbdev
> > > > > > can be
> > > > > > properly
> > > > > > setup when a sink is connected, if needed the dummy
> > > > > > framebuffer
> > > > > > will be
> > > > > > freed and a new with the proper size will be allocated.
> > > > > > 
> > > > > > This issue also happens when a MST DP sink is connected
> > > > > > since
> > > > > > boot,
> > > > > > as
> > > > > > the MST topology is discovered in parallel if
> > > > > > intel_fbdev_initial_config()
> > > > > > is executed before the first sink MST is discovered it will
> > > > > > cause
> > > > > > this
> > > > > > same issue.
> > > > > > 
> > > > > > This is a follow up patch of
> > > > > > https://patchwork.freedesktop.org/patch/196089/
> > > > > > 
> > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=1041
> > > > > > 58
> > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=1044
> > > > > > 25
> > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_fbdev.c | 24
> > > > > > +++++++++++++++++++++++-
> > > > > >  1 file changed, 23 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > > b/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > > index 7d41d139341b..773577d39782 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > > @@ -696,14 +696,36 @@ int intel_fbdev_init(struct
> > > > > > drm_device
> > > > > > *dev)
> > > > > >         return 0;
> > > > > >  }
> > > > > >  
> > > > > > +static void intel_fbdev_dummy_fb_create(struct intel_fbdev
> > > > > > *ifbdev)
> > > > > > +{
> > > > > > +       struct drm_fb_helper_surface_size sizes;
> > > > > > +
> > > > > > +       sizes.fb_width = 800;
> > > > > > +       sizes.fb_height = 600;
> > > > > > +       sizes.surface_width = sizes.fb_width;
> > > > > > +       sizes.surface_height = sizes.fb_height;
> > > > > > +       sizes.surface_bpp = 32;
> > > > > > +       sizes.surface_depth = 24;
> > > > > > +
> > > > > > +       if (intelfb_create(&ifbdev->helper, &sizes))
> > > > > > +               DRM_ERROR("Unable to create dummy
> > > > > > framebufer");
> > > > > > +}
> > > > > > +
> > > > > >  static void intel_fbdev_initial_config(void *data,
> > > > > > async_cookie_t
> > > > > > cookie)
> > > > > >  {
> > > > > >         struct intel_fbdev *ifbdev = data;
> > > > > >  
> > > > > >         /* Due to peculiar init order wrt to hpd handling
> > > > > > this
> > > > > > is
> > > > > > separate. */
> > > > > >         if (drm_fb_helper_initial_config(&ifbdev->helper,
> > > > > > -                                        ifbdev-
> > > > > > > preferred_bpp))
> > > > > > 
> > > > > > +                                        ifbdev-
> > > > > > > preferred_bpp)) {
> > > > > > 
> > > > > >                 intel_fbdev_unregister(to_i915(ifbdev-
> > > > > > > helper.dev));
> > > > > > 
> > > > > > +               return;
> > > > > > +       }
> > > > > > +
> > > > > > +       mutex_lock(&ifbdev->helper.lock);
> > > > > > +       if (!ifbdev->vma)
> > > > > > +               intel_fbdev_dummy_fb_create(ifbdev);
> > > > > > +       mutex_unlock(&ifbdev->helper.lock);
> > > > > >  }
> > > > > 
> > > > > Did you try
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > b/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > index 65a3313723c9..4120c635742d 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > > > @@ -493,6 +493,7 @@ static bool
> > > > > intel_fb_initial_config(struct
> > > > > drm_fb_helper *fb_helper,
> > > > >  bail:
> > > > >                 DRM_DEBUG_KMS("Not using firmware
> > > > > configuration\n");
> > > > >                 memcpy(enabled, save_enabled, count);
> > > > > +               fb_helper->deferred_setup = true;
> > > > 
> > > > This is too earlier to set deferred_setup when
> > > > intel_fb_initial_config() fails,
> > > > __drm_fb_helper_initial_config_and_unlock() will then try to
> > > > probe/create a fb but as there is no sink connected it will
> > > > fail
> > > > too.
> > > > 
> > > > Also __drm_fb_helper_initial_config_and_unlock() is already
> > > > setting
> > > > deferred_setup if both attempts fails.
> > > 
> > > So...
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > > b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index 7d41d139341b..5f138a03dd2a 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -807,7 +807,7 @@ void intel_fbdev_output_poll_changed(struct
> > > drm_device *dev)
> > >                 return;
> > >  
> > >         intel_fbdev_sync(ifbdev);
> > > -       if (ifbdev->vma)
> > > +       if (ifbdev->vma || ifbdev->helper->deferred_setup)
> > >                 drm_fb_helper_hotplug_event(&ifbdev->helper);
> > >  }
> > 
> > That would work but according to your comment here: https://patchwo
> > rk.f
> > reedesktop.org/patch/196089/ that would lead to a crash in some
> > case.
> 
> In the deferred_setup case, won't it go to intelfb_create() and after
> that point if we still have no ifdev->vma we won't hit any other
> pathway?

It will, okay I will just run some tests and send it adding you as
'Signed-off-by:' that is okay?

Thanks

> 
> Just because it's been historically buggy (at least 2 instances now
> of
> having to apply protection here), it may have been fixed... Unlikely,
> but maybe it all finally works.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-04-18 18:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-17 22:34 [PATCH] drm/i915/fbdev: Enable late fbdev initial configuration José Roberto de Souza
2018-04-17 23:06 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-04-17 23:44 ` [PATCH] " Chris Wilson
2018-04-18  0:07   ` Souza, Jose
2018-04-18  8:26     ` Chris Wilson
2018-04-18 16:42       ` Souza, Jose
2018-04-18 16:55         ` Chris Wilson
2018-04-18 18:00           ` Souza, Jose
2018-04-17 23:51 ` ✗ Fi.CI.BAT: failure for drm/i915/fbdev: Enable late fbdev initial configuration (rev2) Patchwork
2018-04-18  0:28 ` ✓ Fi.CI.IGT: success for drm/i915/fbdev: Enable late fbdev initial configuration Patchwork
2018-04-18 14:34 ` ✗ Fi.CI.BAT: failure for drm/i915/fbdev: Enable late fbdev initial configuration (rev3) Patchwork

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.