diff mbox

drm/i915: Insert i915_preliminary_hw_support variable.

Message ID 1350332183-1418-1-git-send-email-rodrigo.vivi@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi Oct. 15, 2012, 8:16 p.m. UTC
On the worst scenario, users with new hardwares and old kernel from enabling times can get black screens.
So, now on, this i915_perliminary_hw_support variable shall be used by all upcoming platforms that are still under enabling.

Although it is uncomfortable for developers use this extra variable it brings more stability for end users.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 13 +++++++++++++
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 2 files changed, 14 insertions(+)

Comments

Dave Airlie Oct. 15, 2012, 11:39 p.m. UTC | #1
On Tue, Oct 16, 2012 at 6:16 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> On the worst scenario, users with new hardwares and old kernel from enabling times can get black screens.
> So, now on, this i915_perliminary_hw_support variable shall be used by all upcoming platforms that are still under enabling.
>
> Although it is uncomfortable for developers use this extra variable it brings more stability for end users.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

I like this idea, since I've got to ship known broken kernels on HSW by default.

The biggest issue is when we consider it working, I suppose if it can
successfully light up most of its output, since I know we'll always be
plagued with eDP randomly not working.

Reviewed-by: Dave Airlie <airlied@redhat.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 13 +++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h |  1 +
>  2 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a7837e5..41bee22 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -119,6 +119,13 @@ module_param_named(i915_enable_ppgtt, i915_enable_ppgtt, int, 0600);
>  MODULE_PARM_DESC(i915_enable_ppgtt,
>                 "Enable PPGTT (default: true)");
>
> +unsigned int i915_preliminary_hw_support __read_mostly = 0;
> +module_param_named(i915_preliminary_hw_support, i915_preliminary_hw_support, int, 0600);
> +MODULE_PARM_DESC(i915_preliminary_hw_support,
> +               "Enable preliminary hardware support. "
> +               "Enable Haswell and ValleyView Support. "
> +               "(default: false)");
> +
>  static struct drm_driver driver;
>  extern int intel_agp_enabled;
>
> @@ -827,6 +834,12 @@ i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>         struct intel_device_info *intel_info =
>                 (struct intel_device_info *) ent->driver_data;
>
> +       if (intel_info->is_haswell || intel_info->is_valleyview)
> +               if(!i915_preliminary_hw_support) {
> +                       DRM_ERROR("Preliminary hardware support disabled\n");
> +                       return -ENODEV;
> +               }
> +
>         /* Only bind to function 0 of the device. Early generations
>          * used function 1 as a placeholder for multi-head. This causes
>          * us confusion instead, especially on the systems where both
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9e446b6..5617881 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1225,6 +1225,7 @@ extern int i915_enable_rc6 __read_mostly;
>  extern int i915_enable_fbc __read_mostly;
>  extern bool i915_enable_hangcheck __read_mostly;
>  extern int i915_enable_ppgtt __read_mostly;
> +extern unsigned int i915_preliminary_hw_support __read_mostly;
>
>  extern int i915_suspend(struct drm_device *dev, pm_message_t state);
>  extern int i915_resume(struct drm_device *dev);
> --
> 1.7.11.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter Oct. 17, 2012, 7:22 p.m. UTC | #2
On Mon, Oct 15, 2012 at 05:16:23PM -0300, Rodrigo Vivi wrote:
> On the worst scenario, users with new hardwares and old kernel from enabling times can get black screens.
> So, now on, this i915_perliminary_hw_support variable shall be used by all upcoming platforms that are still under enabling.
> 
> Although it is uncomfortable for developers use this extra variable it brings more stability for end users.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

Merged, with the linewrap in the commit message fixed and the module param
shrunk by the redudant i915_ prefix.

Thanks, Daniel
Rodrigo Vivi Oct. 18, 2012, 12:55 a.m. UTC | #3
On Wed, Oct 17, 2012 at 4:22 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Oct 15, 2012 at 05:16:23PM -0300, Rodrigo Vivi wrote:
> > On the worst scenario, users with new hardwares and old kernel from
> enabling times can get black screens.
> > So, now on, this i915_perliminary_hw_support variable shall be used by
> all upcoming platforms that are still under enabling.
> >
> > Although it is uncomfortable for developers use this extra variable it
> brings more stability for end users.
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>
> Merged, with the linewrap in the commit message fixed and the module param
> shrunk by the redudant i915_ prefix.
>

Thanks!
To be honest I don't like any of those i915_... all redundants i195.i915_



>
> Thanks, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
Daniel Vetter Oct. 18, 2012, 7:05 a.m. UTC | #4
On Thu, Oct 18, 2012 at 2:55 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> To be honest I don't like any of those i915_... all redundants i195.i915_

Yeah, unfortunately we can't kill them because tons of people set
random options they've found while googling (i915.rc6=7 anyone?) and
if we rename them, the module won't load any more :(
-Daniel
Chris Wilson Oct. 18, 2012, 7:35 a.m. UTC | #5
On Wed, 17 Oct 2012 21:55:10 -0300, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> On Wed, Oct 17, 2012 at 4:22 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Mon, Oct 15, 2012 at 05:16:23PM -0300, Rodrigo Vivi wrote:
> > > On the worst scenario, users with new hardwares and old kernel from
> > enabling times can get black screens.
> > > So, now on, this i915_perliminary_hw_support variable shall be used by
> > all upcoming platforms that are still under enabling.
> > >
> > > Although it is uncomfortable for developers use this extra variable it
> > brings more stability for end users.
> > >
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> >
> > Merged, with the linewrap in the commit message fixed and the module param
> > shrunk by the redudant i915_ prefix.
> >
> 
> Thanks!
> To be honest I don't like any of those i915_... all redundants i195.i915_

There weren't meant to be there, they were just cut'n'pasting the
variable name into the wrong field and then enshrined as a public
interface.
> 
> 
> 
> >
> > Thanks, Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> >
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
Non-text part: text/html
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7837e5..41bee22 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -119,6 +119,13 @@  module_param_named(i915_enable_ppgtt, i915_enable_ppgtt, int, 0600);
 MODULE_PARM_DESC(i915_enable_ppgtt,
 		"Enable PPGTT (default: true)");
 
+unsigned int i915_preliminary_hw_support __read_mostly = 0;
+module_param_named(i915_preliminary_hw_support, i915_preliminary_hw_support, int, 0600);
+MODULE_PARM_DESC(i915_preliminary_hw_support,
+		"Enable preliminary hardware support. "
+		"Enable Haswell and ValleyView Support. "
+		"(default: false)");
+
 static struct drm_driver driver;
 extern int intel_agp_enabled;
 
@@ -827,6 +834,12 @@  i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct intel_device_info *intel_info =
 		(struct intel_device_info *) ent->driver_data;
 
+	if (intel_info->is_haswell || intel_info->is_valleyview)
+		if(!i915_preliminary_hw_support) {
+			DRM_ERROR("Preliminary hardware support disabled\n");
+			return -ENODEV;
+		}
+
 	/* Only bind to function 0 of the device. Early generations
 	 * used function 1 as a placeholder for multi-head. This causes
 	 * us confusion instead, especially on the systems where both
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9e446b6..5617881 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1225,6 +1225,7 @@  extern int i915_enable_rc6 __read_mostly;
 extern int i915_enable_fbc __read_mostly;
 extern bool i915_enable_hangcheck __read_mostly;
 extern int i915_enable_ppgtt __read_mostly;
+extern unsigned int i915_preliminary_hw_support __read_mostly;
 
 extern int i915_suspend(struct drm_device *dev, pm_message_t state);
 extern int i915_resume(struct drm_device *dev);