Re: [PATCH] drm/edid: set ELD for firmware and debugfs override EDIDs

[Jani Nikula <jani.nikula@intel.com>]

On Fri, 27 Mar 2015, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Fri, Mar 27, 2015 at 8:08 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>> On Thu, 26 Mar 2015, Daniel Vetter <daniel@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@gmail.com>
>>>> Reported-by: Rob Engle <grenoble@gmail.com>
>>>> Tested-by: Jolan Luff <jolan@gormsby.com>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Jani Nikula <jani.nikula@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@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.

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx