[Intel-gfx] [PATCH] drm/edid: set ELD for firmware and debugfs override EDIDs

[Alex Deucher]

On Fri, Mar 27, 2015 at 12:02 PM, Jani Nikula <jani.nikula at intel.com> wrote:
> On Fri, 27 Mar 2015, Alex Deucher <alexdeucher at gmail.com> wrote:
>> On Fri, Mar 27, 2015 at 8:08 AM, Jani Nikula <jani.nikula at intel.com> wrote:
>>> On Thu, 26 Mar 2015, Daniel Vetter <daniel at ffwll.ch> wrote:
>>>> On Thu, Mar 26, 2015 at 10:42:00AM +0200, Jani Nikula wrote:
>>>>> If the user supplies EDID through firmware or debugfs override, the
>>>>> driver callbacks are bypassed and the connector ELD does not get
>>>>> updated, and audio fails. Set ELD for firmware and debugfs EDIDs too.
>>>>>
>>>>> There should be no harm in gratuitously doing this for non HDMI/DP
>>>>> connectors, as it's still up to the driver to use the ELD, if any.
>>>>>
>>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82349
>>>>> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=80691
>>>>> Reported-by: Emil <emilsvennesson at gmail.com>
>>>>> Reported-by: Rob Engle <grenoble at gmail.com>
>>>>> Tested-by: Jolan Luff <jolan at gormsby.com>
>>>>> Cc: stable at vger.kernel.org
>>>>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>>>>
>>>> Since it's harmless I wonder whether we shouldn't just do this in
>>>> drm_add_edid_modes unconditionally. But this looks like the right minimal
>>>> patch for -fixes, so Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>>
>>> While I was hoping to gather review from outside of drm/i915 folks, I
>>> picked this up and pushed to our new topic/drm-fixes branch of [1].
>>>
>>> I intend to keep picking up (occasional, non-controversial) drm core
>>> fixes aimed at the current development (-rc) kernels, to ensure they're
>>> not dropped, and sending pull requests to Dave as needed. He'll have the
>>> final call whether to pull or not, of course. This is similar to what
>>> Daniel does with the topic/drm-misc branch for drm-next.
>>>
>>> Please let me know if you have any feedback on this.
>>
>>
>> The patch seems fine to me.  However, if we are always going to set
>> the ELD for the override cases, why don't we also always set it for
>> the non-override cases rather than making each driver do it.
>
> So I think this is a good minimal patch for fixes/stable as a first
> step.
>
> But I agree, we should think about the follow-up. I already had a glance
> before, and doing so really begs the question why we wouldn't add a
> helper to handle all of drm_add_edid_modes, drm_edid_to_eld, and
> drm_mode_connector_update_edid_property properly? Similar to
> intel_connector_update_modes in i915/intel_modes.c. All drivers do this
> stuff, although with subtle differences especially wrt error handling.
>

Agreed.

Alex

> BR,
> Jani.
>
>
>>
>> Alex
>>
>>>
>>> Thanks,
>>> Jani.
>>>
>>>
>>> [1] http://cgit.freedesktop.org/drm-intel
>>>
>>> --
>>> Jani Nikula, Intel Open Source Technology Center
>
> --
> Jani Nikula, Intel Open Source Technology Center