Message ID | 20200305081219.2900-1-mario.kleiner.de@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/i915/dp: Add dpcd link_rate quirk for Apple 15" MBP 2017 (v2) | expand |
On Thu, 05 Mar 2020, Mario Kleiner <mario.kleiner.de@gmail.com> 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/ > > v2: Follow Jani's proposal of defining quirk_rates[] instead > of just appending 324000. This for better clarity. > > Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com> > Tested-by: Mario Kleiner <mario.kleiner.de@gmail.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> As much as I dislike quirks, I think we did exhaust other options when we debugged this, and this is now very nicely isolated. Thanks for the patch. Reviewed-by: Jani Nikula <jani.nikula@intel.com> I'll merge once the CI results are in, mostly to ensure this doesn't impact other platforms, as we don't have the machine in the CI farm. > --- > drivers/gpu/drm/drm_dp_helper.c | 2 ++ > drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++++ > include/drm/drm_dp_helper.h | 7 +++++++ > 3 files changed, 20 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..c0d2c70b04fb 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -169,6 +169,17 @@ static void intel_dp_set_sink_rates(struct intel_dp *intel_dp) > }; > int i, max_rate; > > + if (drm_dp_has_quirk(&intel_dp->desc, > + DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS)) { > + /* Needed, e.g., for Apple MBP 2017, 15 inch eDP Retina panel */ > + 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; > + } > + > max_rate = drm_dp_bw_code_to_link_rate(intel_dp->dpcd[DP_MAX_LINK_RATE]); > > for (i = 0; i < ARRAY_SIZE(dp_rates); i++) { > 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, > }; > > /**
On Thu, 05 Mar 2020, Jani Nikula <jani.nikula@intel.com> wrote: > On Thu, 05 Mar 2020, Mario Kleiner <mario.kleiner.de@gmail.com> 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/ >> >> v2: Follow Jani's proposal of defining quirk_rates[] instead >> of just appending 324000. This for better clarity. >> >> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com> >> Tested-by: Mario Kleiner <mario.kleiner.de@gmail.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Jani Nikula <jani.nikula@intel.com> > > As much as I dislike quirks, I think we did exhaust other options when > we debugged this, and this is now very nicely isolated. Thanks for the > patch. > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > I'll merge once the CI results are in, mostly to ensure this doesn't > impact other platforms, as we don't have the machine in the CI farm. Note to self, add this when merging per Mario's suggestion: Cc: stable@vger.kernel.org > >> --- >> drivers/gpu/drm/drm_dp_helper.c | 2 ++ >> drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++++ >> include/drm/drm_dp_helper.h | 7 +++++++ >> 3 files changed, 20 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..c0d2c70b04fb 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dp.c >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c >> @@ -169,6 +169,17 @@ static void intel_dp_set_sink_rates(struct intel_dp *intel_dp) >> }; >> int i, max_rate; >> >> + if (drm_dp_has_quirk(&intel_dp->desc, >> + DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS)) { >> + /* Needed, e.g., for Apple MBP 2017, 15 inch eDP Retina panel */ >> + 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; >> + } >> + >> max_rate = drm_dp_bw_code_to_link_rate(intel_dp->dpcd[DP_MAX_LINK_RATE]); >> >> for (i = 0; i < ARRAY_SIZE(dp_rates); i++) { >> 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, >> }; >> >> /**
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..c0d2c70b04fb 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -169,6 +169,17 @@ static void intel_dp_set_sink_rates(struct intel_dp *intel_dp) }; int i, max_rate; + if (drm_dp_has_quirk(&intel_dp->desc, + DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS)) { + /* Needed, e.g., for Apple MBP 2017, 15 inch eDP Retina panel */ + 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; + } + max_rate = drm_dp_bw_code_to_link_rate(intel_dp->dpcd[DP_MAX_LINK_RATE]); for (i = 0; i < ARRAY_SIZE(dp_rates); i++) { 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, }; /**