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

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

[Jani Nikula]

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>
---
 drivers/gpu/drm/drm_edid_load.c    | 1 +
 drivers/gpu/drm/drm_probe_helper.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 732cb6f8e653..4c0aa97aaf03 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -287,6 +287,7 @@ int drm_load_edid_firmware(struct drm_connector *connector)
 
 	drm_mode_connector_update_edid_property(connector, edid);
 	ret = drm_add_edid_modes(connector, edid);
+	drm_edid_to_eld(connector, edid);
 	kfree(edid);
 
 	return ret;
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 6591d48c1b9d..3fee587bc284 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -174,6 +174,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
 			struct edid *edid = (struct edid *) connector->edid_blob_ptr->data;
 
 			count = drm_add_edid_modes(connector, edid);
+			drm_edid_to_eld(connector, edid);
 		} else
 			count = (*connector_funcs->get_modes)(connector);
 	}
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

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>

Cheers, Daniel
> ---
>  drivers/gpu/drm/drm_edid_load.c    | 1 +
>  drivers/gpu/drm/drm_probe_helper.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 732cb6f8e653..4c0aa97aaf03 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -287,6 +287,7 @@ int drm_load_edid_firmware(struct drm_connector *connector)
>  
>  	drm_mode_connector_update_edid_property(connector, edid);
>  	ret = drm_add_edid_modes(connector, edid);
> +	drm_edid_to_eld(connector, edid);
>  	kfree(edid);
>  
>  	return ret;
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 6591d48c1b9d..3fee587bc284 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -174,6 +174,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect
>  			struct edid *edid = (struct edid *) connector->edid_blob_ptr->data;
>  
>  			count = drm_add_edid_modes(connector, edid);
> +			drm_edid_to_eld(connector, edid);
>  		} else
>  			count = (*connector_funcs->get_modes)(connector);
>  	}
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Jani Nikula]

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.

Thanks,
Jani.


[1] http://cgit.freedesktop.org/drm-intel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Alex Deucher]

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.

Alex

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

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

[Jani Nikula]

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

Re: [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@intel.com> wrote:
> 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.
>

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

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

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6058
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  303/303              303/303
SNB                 -2              304/304              302/304
IVB                                  339/339              339/339
BYT                                  287/287              287/287
HSW                                  362/362              362/362
BDW                                  310/310              310/310
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*SNB  igt@pm_rpm@drm-resources-equal      PASS(2)      DMESG_FAIL(1)PASS(1)
(dmesg patch applied)drm:intel_dp_start_link_train[i915]]*ERROR*failed_to_enable_link_training@failed to enable link training
drm:intel_dp_complete_link_train[i915]]*ERROR*failed_to_start_channel_equalization@failed to start channel equalization
*SNB  igt@pm_rpm@modeset-non-lpsp-stress-no-wait      PASS(2)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:intel_dp_start_link_train[i915]]*ERROR*too_many_voltage_retries,give_up@too many voltage .* give up
drm:intel_dp_complete_link_train[i915]]*ERROR*failed_to_train_DP,aborting@failed to train .* aborting
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx