[Intel-gfx] [PATCH] drm/i915: make prelim hw msg in log a bit more harsh

[Rob Clark]

On Fri, Sep 18, 2015 at 10:01 AM, Jani Nikula
<jani.nikula at linux.intel.com> wrote:
> On Fri, 18 Sep 2015, Rob Clark <robdclark at gmail.com> wrote:
>> On Fri, Sep 18, 2015 at 7:59 AM, Jani Nikula
>> <jani.nikula at linux.intel.com> wrote:
>>> On Fri, 18 Sep 2015, Rob Clark <robdclark at 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 at 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 at 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