| Message ID | 1442525356-32218-1-git-send-email-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 18 Sep 2015, Rob Clark <robdclark@gmail.com> wrote: > Apparently some people see this message and try to turn on > preliminary_hw_support. And then for some reason are surprised > when it doesn't work. So let's set expectations a bit lower. I think the message goes wrong already at the beginning, "hardware requires preliminary hardware support". The hardware really requires full support, but preliminary is all we've got so far, and that's been disabled by default... How about something along the lines of, """ This kernel version does not properly support the hardware. Preliminary support disabled by default. See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT or i915.preliminary_hw_support. """ BR, Jani. > > Signed-off-by: Rob Clark <robdclark@gmail.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 8edcec8..5e87fe1 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -926,7 +926,8 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > if (IS_PRELIMINARY_HW(intel_info) && !i915.preliminary_hw_support) { > DRM_INFO("This hardware requires preliminary hardware support.\n" > - "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support\n"); > + "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support.\n" > + "Note that preliminary hw support is unsupported, use at your own risk!\n"); > return -ENODEV; > } > > -- > 2.4.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Sep 18, 2015 at 7:59 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Fri, 18 Sep 2015, Rob Clark <robdclark@gmail.com> wrote: >> Apparently some people see this message and try to turn on >> preliminary_hw_support. And then for some reason are surprised >> when it doesn't work. So let's set expectations a bit lower. > > I think the message goes wrong already at the beginning, "hardware > requires preliminary hardware support". The hardware really requires > full support, but preliminary is all we've got so far, and that's been > disabled by default... > > How about something along the lines of, > > """ > This kernel version does not properly support the hardware. > Preliminary support disabled by default. > See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT or i915.preliminary_hw_support. > """ I'm not too picky on the wording, but I think it should include something along the lines of "unsupported" or "use at your own risk" just to make it clear that you may be left with a dark screen.. I kinda think we should also drop CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT too and leave only the module param (since the config option encourages one to build a kernel that won't work unless you know to override i915.reliminary_hw_support in grub. After that, perhaps switch to module_param_named_unsafe() for good measure.. BR, -R > BR, > Jani. > > > >> >> Signed-off-by: Rob Clark <robdclark@gmail.com> >> --- >> drivers/gpu/drm/i915/i915_drv.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >> index 8edcec8..5e87fe1 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -926,7 +926,8 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >> >> if (IS_PRELIMINARY_HW(intel_info) && !i915.preliminary_hw_support) { >> DRM_INFO("This hardware requires preliminary hardware support.\n" >> - "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support\n"); >> + "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support.\n" >> + "Note that preliminary hw support is unsupported, use at your own risk!\n"); >> return -ENODEV; >> } >> >> -- >> 2.4.3 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Jani Nikula, Intel Open Source Technology Center
On Fri, 18 Sep 2015, Rob Clark <robdclark@gmail.com> wrote: > On Fri, Sep 18, 2015 at 7:59 AM, Jani Nikula > <jani.nikula@linux.intel.com> wrote: >> On Fri, 18 Sep 2015, Rob Clark <robdclark@gmail.com> wrote: >>> Apparently some people see this message and try to turn on >>> preliminary_hw_support. And then for some reason are surprised >>> when it doesn't work. So let's set expectations a bit lower. >> >> I think the message goes wrong already at the beginning, "hardware >> requires preliminary hardware support". The hardware really requires >> full support, but preliminary is all we've got so far, and that's been >> disabled by default... >> >> How about something along the lines of, >> >> """ >> This kernel version does not properly support the hardware. >> Preliminary support disabled by default. >> See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT or i915.preliminary_hw_support. >> """ > > I'm not too picky on the wording, but I think it should include > something along the lines of "unsupported" or "use at your own risk" > just to make it clear that you may be left with a dark screen.. I was just tring to avoid writing "unsupported support" there. ;) > I kinda think we should also drop > CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT too and leave only the module I think that was for distros so they wouldn't have to enable the module parameter if they take their chances. > param (since the config option encourages one to build a kernel that > won't work unless you know to override i915.reliminary_hw_support in > grub. > > After that, perhaps switch to module_param_named_unsafe() for good measure.. Actually I'm slightly surprised it isn't already. I take it that alone isn't enough of a "sharp edges inside" warning for you? BR, Jani. > > BR, > -R > >> BR, >> Jani. >> >> >> >>> >>> Signed-off-by: Rob Clark <robdclark@gmail.com> >>> --- >>> drivers/gpu/drm/i915/i915_drv.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >>> index 8edcec8..5e87fe1 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.c >>> +++ b/drivers/gpu/drm/i915/i915_drv.c >>> @@ -926,7 +926,8 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >>> >>> if (IS_PRELIMINARY_HW(intel_info) && !i915.preliminary_hw_support) { >>> DRM_INFO("This hardware requires preliminary hardware support.\n" >>> - "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support\n"); >>> + "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support.\n" >>> + "Note that preliminary hw support is unsupported, use at your own risk!\n"); >>> return -ENODEV; >>> } >>> >>> -- >>> 2.4.3 >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> -- >> Jani Nikula, Intel Open Source Technology Center
On Fri, Sep 18, 2015 at 10:01 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Fri, 18 Sep 2015, Rob Clark <robdclark@gmail.com> wrote: >> On Fri, Sep 18, 2015 at 7:59 AM, Jani Nikula >> <jani.nikula@linux.intel.com> wrote: >>> On Fri, 18 Sep 2015, Rob Clark <robdclark@gmail.com> wrote: >>>> Apparently some people see this message and try to turn on >>>> preliminary_hw_support. And then for some reason are surprised >>>> when it doesn't work. So let's set expectations a bit lower. >>> >>> I think the message goes wrong already at the beginning, "hardware >>> requires preliminary hardware support". The hardware really requires >>> full support, but preliminary is all we've got so far, and that's been >>> disabled by default... >>> >>> How about something along the lines of, >>> >>> """ >>> This kernel version does not properly support the hardware. >>> Preliminary support disabled by default. >>> See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT or i915.preliminary_hw_support. >>> """ >> >> I'm not too picky on the wording, but I think it should include >> something along the lines of "unsupported" or "use at your own risk" >> just to make it clear that you may be left with a dark screen.. > > I was just tring to avoid writing "unsupported support" there. ;) > >> I kinda think we should also drop >> CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT too and leave only the module > > I think that was for distros so they wouldn't have to enable the module > parameter if they take their chances. I'm not sure that is something to be encouraged.. at least it has been my experience that people enable this option and then file bug reports when they end up with a non-booting system ;-) >> param (since the config option encourages one to build a kernel that >> won't work unless you know to override i915.reliminary_hw_support in >> grub. >> >> After that, perhaps switch to module_param_named_unsafe() for good measure.. > > Actually I'm slightly surprised it isn't already. I take it that alone > isn't enough of a "sharp edges inside" warning for you? well, _unsafe() makes more sense if we drop the config option, ie. we shouldn't taint the kernel if you are overriding to *disable* prelim.. it would almost be enough 'sharp edges', except that if you wind up with a non-booting kernel and no display and never see the tainted kernel warning ;-) BR, -R > BR, > Jani. > > >> >> BR, >> -R >> >>> BR, >>> Jani. >>> >>> >>> >>>> >>>> Signed-off-by: Rob Clark <robdclark@gmail.com> >>>> --- >>>> drivers/gpu/drm/i915/i915_drv.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >>>> index 8edcec8..5e87fe1 100644 >>>> --- a/drivers/gpu/drm/i915/i915_drv.c >>>> +++ b/drivers/gpu/drm/i915/i915_drv.c >>>> @@ -926,7 +926,8 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >>>> >>>> if (IS_PRELIMINARY_HW(intel_info) && !i915.preliminary_hw_support) { >>>> DRM_INFO("This hardware requires preliminary hardware support.\n" >>>> - "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support\n"); >>>> + "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support.\n" >>>> + "Note that preliminary hw support is unsupported, use at your own risk!\n"); >>>> return -ENODEV; >>>> } >>>> >>>> -- >>>> 2.4.3 >>>> >>>> _______________________________________________ >>>> dri-devel mailing list >>>> dri-devel@lists.freedesktop.org >>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >>> >>> -- >>> Jani Nikula, Intel Open Source Technology Center > > -- > Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 8edcec8..5e87fe1 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -926,7 +926,8 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (IS_PRELIMINARY_HW(intel_info) && !i915.preliminary_hw_support) { DRM_INFO("This hardware requires preliminary hardware support.\n" - "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support\n"); + "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support.\n" + "Note that preliminary hw support is unsupported, use at your own risk!\n"); return -ENODEV; }
Apparently some people see this message and try to turn on preliminary_hw_support. And then for some reason are surprised when it doesn't work. So let's set expectations a bit lower. Signed-off-by: Rob Clark <robdclark@gmail.com> --- drivers/gpu/drm/i915/i915_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)