On Sat, 29 Feb 2020, Mario Kleiner <[email protected]> wrote:
> This fixes a problem found on the MacBookPro 2017 Retina panel.
>
> The panel reports 10 bpc color depth in its EDID, and the
> firmware chooses link settings at boot which support enough
> bandwidth for 10 bpc (324000 kbit/sec = multiplier 0xc),
> but the DP_MAX_LINK_RATE dpcd register only reports
> 2.7 Gbps (multiplier value 0xa) as possible, in direct
> contradiction of what the firmware successfully set up.
>
> This restricts the panel to 8 bpc, not providing the full
> color depth of the panel.
>
> This patch adds a quirk specific to the MBP 2017 15" Retina
> panel to add the additiional 324000 kbps link rate during
> edp setup.
>
> Link to previous discussion of a different attempted fix
> with Ville and Jani:
>
> https://patchwork.kernel.org/patch/11325935/
>
> Signed-off-by: Mario Kleiner <[email protected]>
> Cc: Ville Syrjälä <[email protected]>
> Cc: Jani Nikula <[email protected]>
> ---
> drivers/gpu/drm/drm_dp_helper.c | 2 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 7 +++++++
> include/drm/drm_dp_helper.h | 7 +++++++
> 3 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 5a103e9b3c86..36a371c016cb 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1179,6 +1179,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
> { OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'),
> false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
> /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */
> { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true,
> BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) },
> + /* Apple MacBookPro 2017 15 inch eDP Retina panel reports too low
> DP_MAX_LINK_RATE */
> + { OUI(0x00, 0x10, 0xfa), DEVICE_ID(101, 68, 21, 101, 98, 97), false,
> BIT(DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS) },
> };
>
> #undef OUI
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4074d83b1a5f..1f6bd659ad41 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -178,6 +178,13 @@ static void intel_dp_set_sink_rates(struct intel_dp
> *intel_dp)
> }
>
> intel_dp->num_sink_rates = i;
> +
> + if (drm_dp_has_quirk(&intel_dp->desc,
> + DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS)) {
> + /* Needed for Apple MBP 2017, 15 inch eDP Retina panel */
> + intel_dp->sink_rates[i] = 324000;
> + intel_dp->num_sink_rates++;
> + }
If we can isolate the quirk to this one function, I'll be happy. \o/
However, even if this might work on said machine, I'd prefer it if we
didn't give the idea that you could just append a value in sink_rates
(it must be sorted). How about putting something like this in the
beginning of the function, to be a bit more explicit:
if (quirk) {
static const int quirk_rates[] = { 162000, 270000, 324000 };
memcpy(intel_dp->sink_rates, quirk_rates, sizeof(quirk_rates));
intel_dp->num_sink_rates = ARRAY_SIZE(quirk_rates);
return;
}
BR,
Jani.
> }
>
> /* Get length of rates array potentially limited by max_rate. */
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 262faf9e5e94..4b86a1f2a559 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1532,6 +1532,13 @@ enum drm_dp_quirk {
> * The DSC caps can be read from the physical aux instead.
> */
> DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD,
> + /**
> + * @DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS:
> + *
> + * The device supports a link rate of 3.24 Gbps (multiplier 0xc) despite
> + * the DP_MAX_LINK_RATE register reporting a lower max multiplier.
> + */
> + DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS,
> };
>
> /**
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
