From: Aun-Ali Zaidi <[email protected]>
The eDP link rate reported by the DP_MAX_LINK_RATE dpcd register (0xa) is
contradictory to the highest rate supported reported by
EDID (0xc = LINK_RATE_RBR2). The effects of this compounded with commit
'4a8ca46bae8a ("drm/amd/display: Default max bpc to 16 for eDP")' results
in no display modes being found and a dark panel.
For now, simply force the maximum supported link rate for the eDP attached
2018 15" Apple Retina panels.
Additionally, we must also check the firmware revision since the device ID
reported by the DPCD is identical to that of the more capable 16,1,
incorrectly quirking it. We also use said firmware check to quirk the
refreshed 15,1 models with Vega graphics as they use a slightly newer
firmware version.
Tested-by: Aun-Ali Zaidi <[email protected]>
Signed-off-by: Aun-Ali Zaidi <[email protected]>
Signed-off-by: Aditya Garg <[email protected]>
---
.../gpu/drm/amd/display/dc/core/dc_link_dp.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 05e216524..17939ad17 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -5597,6 +5597,25 @@ static bool retrieve_link_cap(struct dc_link *link)
dp_hw_fw_revision.ieee_fw_rev,
sizeof(dp_hw_fw_revision.ieee_fw_rev));
+ /* Quirk for Apple MBP 2018 15" Retina panels: wrong DP_MAX_LINK_RATE */
+ {
+ uint8_t str_mbp_2018[] = { 101, 68, 21, 103, 98, 97 };
+ uint8_t fwrev_mbp_2018[] = { 7, 4 };
+ uint8_t fwrev_mbp_2018_vega[] = { 8, 4 };
+
+ // We also check for the firmware revision as 16,1 models have an
+ // identical device id and are incorrectly quirked otherwise.
+ if ((link->dpcd_caps.sink_dev_id == 0x0010fa) &&
+ !memcmp(link->dpcd_caps.sink_dev_id_str, str_mbp_2018,
+ sizeof(str_mbp_2018)) &&
+ (!memcmp(link->dpcd_caps.sink_fw_revision, fwrev_mbp_2018,
+ sizeof(fwrev_mbp_2018)) ||
+ !memcmp(link->dpcd_caps.sink_fw_revision, fwrev_mbp_2018_vega,
+ sizeof(fwrev_mbp_2018_vega)))) {
+ link->reported_link_cap.link_rate = LINK_RATE_RBR2;
+ }
+ }
+
memset(&link->dpcd_caps.dsc_caps, '\0',
sizeof(link->dpcd_caps.dsc_caps));
memset(&link->dpcd_caps.fec_cap, '\0', sizeof(link->dpcd_caps.fec_cap));
--
2.25.1
On Wed, Jan 26, 2022 at 8:56 AM Aditya Garg <[email protected]> wrote:
>
> From: Aun-Ali Zaidi <[email protected]>
>
> The eDP link rate reported by the DP_MAX_LINK_RATE dpcd register (0xa) is
> contradictory to the highest rate supported reported by
> EDID (0xc = LINK_RATE_RBR2). The effects of this compounded with commit
> '4a8ca46bae8a ("drm/amd/display: Default max bpc to 16 for eDP")' results
> in no display modes being found and a dark panel.
>
> For now, simply force the maximum supported link rate for the eDP attached
> 2018 15" Apple Retina panels.
>
> Additionally, we must also check the firmware revision since the device ID
> reported by the DPCD is identical to that of the more capable 16,1,
> incorrectly quirking it. We also use said firmware check to quirk the
> refreshed 15,1 models with Vega graphics as they use a slightly newer
> firmware version.
>
> Tested-by: Aun-Ali Zaidi <[email protected]>
> Signed-off-by: Aun-Ali Zaidi <[email protected]>
> Signed-off-by: Aditya Garg <[email protected]>
> ---
> .../gpu/drm/amd/display/dc/core/dc_link_dp.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> index 05e216524..17939ad17 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> @@ -5597,6 +5597,25 @@ static bool retrieve_link_cap(struct dc_link *link)
> dp_hw_fw_revision.ieee_fw_rev,
> sizeof(dp_hw_fw_revision.ieee_fw_rev));
>
> + /* Quirk for Apple MBP 2018 15" Retina panels: wrong DP_MAX_LINK_RATE */
> + {
> + uint8_t str_mbp_2018[] = { 101, 68, 21, 103, 98, 97 };
> + uint8_t fwrev_mbp_2018[] = { 7, 4 };
> + uint8_t fwrev_mbp_2018_vega[] = { 8, 4 };
> +
> + // We also check for the firmware revision as 16,1 models have an
> + // identical device id and are incorrectly quirked otherwise.
C style comments please. I'll let one of the display guys comment on
the rest of the patch. Seems reasonable, we have a similar quirk for
the Apple MBP 2017 15" Retina panel later in this function. Could you
move this next to the other quirk?
Alex
> + if ((link->dpcd_caps.sink_dev_id == 0x0010fa) &&
> + !memcmp(link->dpcd_caps.sink_dev_id_str, str_mbp_2018,
> + sizeof(str_mbp_2018)) &&
> + (!memcmp(link->dpcd_caps.sink_fw_revision, fwrev_mbp_2018,
> + sizeof(fwrev_mbp_2018)) ||
> + !memcmp(link->dpcd_caps.sink_fw_revision, fwrev_mbp_2018_vega,
> + sizeof(fwrev_mbp_2018_vega)))) {
> + link->reported_link_cap.link_rate = LINK_RATE_RBR2;
> + }
> + }
> +
> memset(&link->dpcd_caps.dsc_caps, '\0',
> sizeof(link->dpcd_caps.dsc_caps));
> memset(&link->dpcd_caps.fec_cap, '\0', sizeof(link->dpcd_caps.fec_cap));
> --
> 2.25.1
>
>
Hi Alex
> On 27-Jan-2022, at 11:06 PM, Alex Deucher <[email protected]> wrote:
>
> C style comments please.
Shall be fixed in v2
> I'll let one of the display guys comment on
> the rest of the patch. Seems reasonable, we have a similar quirk for
> the Apple MBP 2017 15" Retina panel later in this function. Could you
> move this next to the other quirk?
I guess moving it next to the other quirk may break the functionality of this quirk, cause the MBP 2018 one involves stuff regarding firmware revision as well. The original patch applies the quirk after the following lines of the code :-
core_link_read_dpcd(
link,
DP_SINK_HW_REVISION_START,
(uint8_t *)&dp_hw_fw_revision,
sizeof(dp_hw_fw_revision));
link->dpcd_caps.sink_hw_revision =
dp_hw_fw_revision.ieee_hw_rev;
memmove(
link->dpcd_caps.sink_fw_revision,
dp_hw_fw_revision.ieee_fw_rev,
sizeof(dp_hw_fw_revision.ieee_fw_rev));
Which seem to related to the firmware stuff. Moving it along with the 2017 quirk doesn't sound right to me, as this shall move the quirk BEFORE these lines of code instead. Maybe the author also knowingly added the quirk after these lines of code?
As a workaround, could we move the 2017 quirk later, instead of moving the 2018 quirk before? This sounds more logical to me.
Regards
Aditya
On 1/28/22 08:06, Aditya Garg wrote:
>
> Hi Alex
>
>> On 27-Jan-2022, at 11:06 PM, Alex Deucher <[email protected]> wrote:
>>
>> C style comments please.
> Shall be fixed in v2
>> I'll let one of the display guys comment on
>> the rest of the patch. Seems reasonable, we have a similar quirk for
>> the Apple MBP 2017 15" Retina panel later in this function. Could you
>> move this next to the other quirk?
> I guess moving it next to the other quirk may break the functionality of this quirk, cause the MBP 2018 one involves stuff regarding firmware revision as well. The original patch applies the quirk after the following lines of the code :-
>
>
> core_link_read_dpcd(
> link,
> DP_SINK_HW_REVISION_START,
> (uint8_t *)&dp_hw_fw_revision,
> sizeof(dp_hw_fw_revision));
>
> link->dpcd_caps.sink_hw_revision =
> dp_hw_fw_revision.ieee_hw_rev;
>
> memmove(
> link->dpcd_caps.sink_fw_revision,
> dp_hw_fw_revision.ieee_fw_rev,
> sizeof(dp_hw_fw_revision.ieee_fw_rev));
>
> Which seem to related to the firmware stuff. Moving it along with the 2017 quirk doesn't sound right to me, as this shall move the quirk BEFORE these lines of code instead. Maybe the author also knowingly added the quirk after these lines of code?
>
> As a workaround, could we move the 2017 quirk later, instead of moving the 2018 quirk before? This sounds more logical to me.
>
I think either leaving the 2017 quirk in its original place or moving it
down works. I don't have a strong preference.
With the comment style addressed this patch is
Reviewed-by: Harry Wentland <[email protected]>
Harry
> Regards
> Aditya
> On 28-Jan-2022, at 8:33 PM, Harry Wentland <[email protected]> wrote:
>
> I think either leaving the 2017 quirk in its original place or moving it down works. I don't have a strong preference.
I’d better leave it in the original place then
>
> With the comment style addressed this patch is
Sending a v2 now with this issue addressed.
> Reviewed-by: Harry Wentland <[email protected]>
>
> Harry
From: Aun-Ali Zaidi <[email protected]>
The eDP link rate reported by the DP_MAX_LINK_RATE dpcd register (0xa) is
contradictory to the highest rate supported reported by
EDID (0xc = LINK_RATE_RBR2). The effects of this compounded with commit
'4a8ca46bae8a ("drm/amd/display: Default max bpc to 16 for eDP")' results
in no display modes being found and a dark panel.
For now, simply force the maximum supported link rate for the eDP attached
2018 15" Apple Retina panels.
Additionally, we must also check the firmware revision since the device ID
reported by the DPCD is identical to that of the more capable 16,1,
incorrectly quirking it. We also use said firmware check to quirk the
refreshed 15,1 models with Vega graphics as they use a slightly newer
firmware version.
Tested-by: Aun-Ali Zaidi <[email protected]>
Reviewed-by: Harry Wentland <[email protected]>
Signed-off-by: Aun-Ali Zaidi <[email protected]>
Signed-off-by: Aditya Garg <[email protected]>
---
v2 :- Use C styled comments
.../gpu/drm/amd/display/dc/core/dc_link_dp.c | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 05e216524..086f7ee2c 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -5597,6 +5597,26 @@ static bool retrieve_link_cap(struct dc_link *link)
dp_hw_fw_revision.ieee_fw_rev,
sizeof(dp_hw_fw_revision.ieee_fw_rev));
+ /* Quirk for Apple MBP 2018 15" Retina panels: wrong DP_MAX_LINK_RATE */
+ {
+ uint8_t str_mbp_2018[] = { 101, 68, 21, 103, 98, 97 };
+ uint8_t fwrev_mbp_2018[] = { 7, 4 };
+ uint8_t fwrev_mbp_2018_vega[] = { 8, 4 };
+
+ /* We also check for the firmware revision as 16,1 models have an
+ * identical device id and are incorrectly quirked otherwise.
+ */
+ if ((link->dpcd_caps.sink_dev_id == 0x0010fa) &&
+ !memcmp(link->dpcd_caps.sink_dev_id_str, str_mbp_2018,
+ sizeof(str_mbp_2018)) &&
+ (!memcmp(link->dpcd_caps.sink_fw_revision, fwrev_mbp_2018,
+ sizeof(fwrev_mbp_2018)) ||
+ !memcmp(link->dpcd_caps.sink_fw_revision, fwrev_mbp_2018_vega,
+ sizeof(fwrev_mbp_2018_vega)))) {
+ link->reported_link_cap.link_rate = LINK_RATE_RBR2;
+ }
+ }
+
memset(&link->dpcd_caps.dsc_caps, '\0',
sizeof(link->dpcd_caps.dsc_caps));
memset(&link->dpcd_caps.fec_cap, '\0', sizeof(link->dpcd_caps.fec_cap));
--
2.25.1
Applied. Thanks!
Alex
On Sat, Jan 29, 2022 at 12:50 AM Aditya Garg <[email protected]> wrote:
>
> From: Aun-Ali Zaidi <[email protected]>
>
> The eDP link rate reported by the DP_MAX_LINK_RATE dpcd register (0xa) is
> contradictory to the highest rate supported reported by
> EDID (0xc = LINK_RATE_RBR2). The effects of this compounded with commit
> '4a8ca46bae8a ("drm/amd/display: Default max bpc to 16 for eDP")' results
> in no display modes being found and a dark panel.
>
> For now, simply force the maximum supported link rate for the eDP attached
> 2018 15" Apple Retina panels.
>
> Additionally, we must also check the firmware revision since the device ID
> reported by the DPCD is identical to that of the more capable 16,1,
> incorrectly quirking it. We also use said firmware check to quirk the
> refreshed 15,1 models with Vega graphics as they use a slightly newer
> firmware version.
>
> Tested-by: Aun-Ali Zaidi <[email protected]>
> Reviewed-by: Harry Wentland <[email protected]>
> Signed-off-by: Aun-Ali Zaidi <[email protected]>
> Signed-off-by: Aditya Garg <[email protected]>
> ---
> v2 :- Use C styled comments
> .../gpu/drm/amd/display/dc/core/dc_link_dp.c | 20 +++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> index 05e216524..086f7ee2c 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> @@ -5597,6 +5597,26 @@ static bool retrieve_link_cap(struct dc_link *link)
> dp_hw_fw_revision.ieee_fw_rev,
> sizeof(dp_hw_fw_revision.ieee_fw_rev));
>
> + /* Quirk for Apple MBP 2018 15" Retina panels: wrong DP_MAX_LINK_RATE */
> + {
> + uint8_t str_mbp_2018[] = { 101, 68, 21, 103, 98, 97 };
> + uint8_t fwrev_mbp_2018[] = { 7, 4 };
> + uint8_t fwrev_mbp_2018_vega[] = { 8, 4 };
> +
> + /* We also check for the firmware revision as 16,1 models have an
> + * identical device id and are incorrectly quirked otherwise.
> + */
> + if ((link->dpcd_caps.sink_dev_id == 0x0010fa) &&
> + !memcmp(link->dpcd_caps.sink_dev_id_str, str_mbp_2018,
> + sizeof(str_mbp_2018)) &&
> + (!memcmp(link->dpcd_caps.sink_fw_revision, fwrev_mbp_2018,
> + sizeof(fwrev_mbp_2018)) ||
> + !memcmp(link->dpcd_caps.sink_fw_revision, fwrev_mbp_2018_vega,
> + sizeof(fwrev_mbp_2018_vega)))) {
> + link->reported_link_cap.link_rate = LINK_RATE_RBR2;
> + }
> + }
> +
> memset(&link->dpcd_caps.dsc_caps, '\0',
> sizeof(link->dpcd_caps.dsc_caps));
> memset(&link->dpcd_caps.fec_cap, '\0', sizeof(link->dpcd_caps.fec_cap));
> --
> 2.25.1
>
>