For certain eDP 1.4 panels, we need to use max lane count for the link training to succeed.
This patch adds a EDID quirk for such eDP panels using their vendor ID and product ID to force using max lane count in the driver.
Cc: Clint Taylor Clinton.A.Taylor@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Tested-by: Albert Astals Cid aacid@kde.org Tested-by: Emanuele Panigati ilpanich@gmail.com Tested-by: Ralgor ralgorfdb@compuspex.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959 Signed-off-by: Manasi Navare manasi.d.navare@intel.com --- drivers/gpu/drm/drm_edid.c | 10 ++++++++++ include/drm/drm_connector.h | 5 +++++ 2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2c22ea446075..fbc661806484 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -82,6 +82,8 @@ #define EDID_QUIRK_FORCE_10BPC (1 << 11) /* Non desktop display (i.e. HMD) */ #define EDID_QUIRK_NON_DESKTOP (1 << 12) +/* Force max lane count */ +#define EDID_QUIRK_FORCE_MAX_LANE_COUNT (1 << 13)
struct detailed_mode_closure { struct drm_connector *connector; @@ -189,6 +191,10 @@ static const struct edid_quirk {
/* OSVR HDK and HDK2 VR Headsets */ { "SVR", 0x1019, EDID_QUIRK_NON_DESKTOP }, + + /* SHP eDP 1.4 panel only works with max lane count */ + { "SHP", 0x149a, EDID_QUIRK_FORCE_MAX_LANE_COUNT }, + { "SHP", 0x148e, EDID_QUIRK_FORCE_MAX_LANE_COUNT }, };
/* @@ -4463,6 +4469,7 @@ drm_reset_display_info(struct drm_connector *connector) memset(&info->hdmi, 0, sizeof(info->hdmi));
info->non_desktop = 0; + info->force_max_lane_count = 0; }
u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid) @@ -4744,6 +4751,9 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) if (quirks & EDID_QUIRK_FORCE_12BPC) connector->display_info.bpc = 12;
+ if (quirks & EDID_QUIRK_FORCE_MAX_LANE_COUNT) + connector->display_info.force_max_lane_count = true; + return num_modes; } EXPORT_SYMBOL(drm_add_edid_modes); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 02a131202add..45436d40ffe3 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -457,6 +457,11 @@ struct drm_display_info { * @non_desktop: Non desktop display (HMD). */ bool non_desktop; + + /** + * @force_max_lane_count: Link training requires max lane count to pass + */ + bool force_max_lane_count; };
int drm_display_info_set_bus_formats(struct drm_display_info *info,
On 4/2/19 2:52 PM, Manasi Navare wrote:
For certain eDP 1.4 panels, we need to use max lane count for the link training to succeed.
This patch adds a EDID quirk for such eDP panels using their vendor ID and product ID to force using max lane count in the driver.
Cc: Clint Taylor Clinton.A.Taylor@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Tested-by: Albert Astals Cid aacid@kde.org Tested-by: Emanuele Panigati ilpanich@gmail.com Tested-by: Ralgor ralgorfdb@compuspex.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959 Signed-off-by: Manasi Navare manasi.d.navare@intel.com
drivers/gpu/drm/drm_edid.c | 10 ++++++++++ include/drm/drm_connector.h | 5 +++++ 2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2c22ea446075..fbc661806484 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -82,6 +82,8 @@ #define EDID_QUIRK_FORCE_10BPC (1 << 11) /* Non desktop display (i.e. HMD) */ #define EDID_QUIRK_NON_DESKTOP (1 << 12) +/* Force max lane count */
Nit. Would like a little more descriptive comment about this quirk. Maybe even /* Force use of max lane count for link training */
+#define EDID_QUIRK_FORCE_MAX_LANE_COUNT (1 << 13)
struct detailed_mode_closure { struct drm_connector *connector; @@ -189,6 +191,10 @@ static const struct edid_quirk {
/* OSVR HDK and HDK2 VR Headsets */ { "SVR", 0x1019, EDID_QUIRK_NON_DESKTOP },
/* SHP eDP 1.4 panel only works with max lane count */
{ "SHP", 0x149a, EDID_QUIRK_FORCE_MAX_LANE_COUNT },
{ "SHP", 0x148e, EDID_QUIRK_FORCE_MAX_LANE_COUNT }, };
/*
@@ -4463,6 +4469,7 @@ drm_reset_display_info(struct drm_connector *connector) memset(&info->hdmi, 0, sizeof(info->hdmi));
info->non_desktop = 0;
info->force_max_lane_count = 0; }
u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid)
@@ -4744,6 +4751,9 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) if (quirks & EDID_QUIRK_FORCE_12BPC) connector->display_info.bpc = 12;
- if (quirks & EDID_QUIRK_FORCE_MAX_LANE_COUNT)
connector->display_info.force_max_lane_count = true;- return num_modes; } EXPORT_SYMBOL(drm_add_edid_modes);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 02a131202add..45436d40ffe3 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -457,6 +457,11 @@ struct drm_display_info { * @non_desktop: Non desktop display (HMD). */ bool non_desktop;
/**
* @force_max_lane_count: Link training requires max lane count to pass*/bool force_max_lane_count; };
int drm_display_info_set_bus_formats(struct drm_display_info *info,
Rest of the patch appears sane for a quirk.
Reviewed-by: Clint Taylor Clinton.A.Taylor@intel.com
-Clint
Some eDP 1.4 panels cannot use the optimized fast and narrow pipe config approach, but they need to use th maximum supported lane count for the link training to succeed. There is a DRM EDID quirk for such panels that gets set after reading their corresponding EDID. So if it is set, this patch forces the max lane count in compute_config() hook to use max lane count for link training.
Cc: Clint Taylor Clinton.A.Taylor@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Tested-by: Albert Astals Cid aacid@kde.org Tested-by: Emanuele Panigati ilpanich@gmail.com Tested-by: Ralgor ralgorfdb@compuspex.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959 Signed-off-by: Manasi Navare manasi.d.navare@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 5 ++++- drivers/gpu/drm/i915/intel_drv.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 72c49070ed14..421db00f5792 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2028,7 +2028,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder, limits.min_bpp = 6 * 3; limits.max_bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
- if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) { + if (intel_dp->edp_force_max_lane_count || (intel_dp_is_edp(intel_dp) && + intel_dp->edp_dpcd[0] < DP_EDP_14)) { /* * Use the maximum clock and number of lanes the eDP panel * advertizes being capable of. The eDP 1.3 and earlier panels @@ -7101,6 +7102,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, edid = drm_get_edid(connector, &intel_dp->aux.ddc); if (edid) { if (drm_add_edid_modes(connector, edid)) { + if (connector->display_info.force_max_lane_count) + intel_dp->edp_force_max_lane_count = true; drm_connector_update_edid_property(connector, edid); } else { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index f8c7b291fdc3..c67c3c518714 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1327,6 +1327,9 @@ struct intel_dp {
/* Display stream compression testing */ bool force_dsc_en; + + /* eDP 1.4 EDID quirk to use max lane count */ + bool edp_force_max_lane_count; };
enum lspcon_vendor {
On 4/2/19 2:52 PM, Manasi Navare wrote:
Some eDP 1.4 panels cannot use the optimized fast and narrow pipe config approach, but they need to use th maximum supported lane count for the link training to succeed. There is a DRM EDID quirk for such panels that gets set after reading their corresponding EDID. So if it is set, this patch forces the max lane count in compute_config() hook to use max lane count for link training.
Cc: Clint Taylor Clinton.A.Taylor@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Tested-by: Albert Astals Cid aacid@kde.org Tested-by: Emanuele Panigati ilpanich@gmail.com Tested-by: Ralgor ralgorfdb@compuspex.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959 Signed-off-by: Manasi Navare manasi.d.navare@intel.com
drivers/gpu/drm/i915/intel_dp.c | 5 ++++- drivers/gpu/drm/i915/intel_drv.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 72c49070ed14..421db00f5792 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2028,7 +2028,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder, limits.min_bpp = 6 * 3; limits.max_bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
- if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
- if (intel_dp->edp_force_max_lane_count || (intel_dp_is_edp(intel_dp) &&
intel_dp->edp_dpcd[0] < DP_EDP_14)) {
- Use the maximum clock and number of lanes the eDP panel
- advertizes being capable of. The eDP 1.3 and earlier panels
@@ -7101,6 +7102,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, edid = drm_get_edid(connector, &intel_dp->aux.ddc); if (edid) { if (drm_add_edid_modes(connector, edid)) {
if (connector->display_info.force_max_lane_count)intel_dp->edp_force_max_lane_count = true; drm_connector_update_edid_property(connector, edid);diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index f8c7b291fdc3..c67c3c518714 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1327,6 +1327,9 @@ struct intel_dp {
/* Display stream compression testing */ bool force_dsc_en;
/* eDP 1.4 EDID quirk to use max lane count */
bool edp_force_max_lane_count; };
enum lspcon_vendor {
Reviewed-by: Clint Taylor Clinton.A.Taylor@intel.com
-Clint
On Tue, Apr 02, 2019 at 02:52:34PM -0700, Manasi Navare wrote:
For certain eDP 1.4 panels, we need to use max lane count for the link training to succeed.
This patch adds a EDID quirk for such eDP panels using their vendor ID and product ID to force using max lane count in the driver.
Rather than opening the quirk can of worms I think we should consider changing the retry loop to do something more sensible than what it's doing now. The current behaviour of "start at optimal settings (which can be either min lanes or min rate), and then reduce lanes/rate until stuff works" overlooks several possible combinations. One possible approach could be to start the retry loop with max lanes + max rate after the optimal settings have failed. It probably won't give you the best power consumption, but at least you get a picture on the screen if even a single lane count + rate combo works.
Cc: Clint Taylor Clinton.A.Taylor@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Tested-by: Albert Astals Cid aacid@kde.org Tested-by: Emanuele Panigati ilpanich@gmail.com Tested-by: Ralgor ralgorfdb@compuspex.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959 Signed-off-by: Manasi Navare manasi.d.navare@intel.com
drivers/gpu/drm/drm_edid.c | 10 ++++++++++ include/drm/drm_connector.h | 5 +++++ 2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2c22ea446075..fbc661806484 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -82,6 +82,8 @@ #define EDID_QUIRK_FORCE_10BPC (1 << 11) /* Non desktop display (i.e. HMD) */ #define EDID_QUIRK_NON_DESKTOP (1 << 12) +/* Force max lane count */ +#define EDID_QUIRK_FORCE_MAX_LANE_COUNT (1 << 13)
struct detailed_mode_closure { struct drm_connector *connector; @@ -189,6 +191,10 @@ static const struct edid_quirk {
/* OSVR HDK and HDK2 VR Headsets */ { "SVR", 0x1019, EDID_QUIRK_NON_DESKTOP },
- /* SHP eDP 1.4 panel only works with max lane count */
- { "SHP", 0x149a, EDID_QUIRK_FORCE_MAX_LANE_COUNT },
- { "SHP", 0x148e, EDID_QUIRK_FORCE_MAX_LANE_COUNT },
};
/* @@ -4463,6 +4469,7 @@ drm_reset_display_info(struct drm_connector *connector) memset(&info->hdmi, 0, sizeof(info->hdmi));
info->non_desktop = 0;
- info->force_max_lane_count = 0;
}
u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid) @@ -4744,6 +4751,9 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) if (quirks & EDID_QUIRK_FORCE_12BPC) connector->display_info.bpc = 12;
- if (quirks & EDID_QUIRK_FORCE_MAX_LANE_COUNT)
connector->display_info.force_max_lane_count = true;- return num_modes;
} EXPORT_SYMBOL(drm_add_edid_modes); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 02a131202add..45436d40ffe3 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -457,6 +457,11 @@ struct drm_display_info { * @non_desktop: Non desktop display (HMD). */ bool non_desktop;
- /**
* @force_max_lane_count: Link training requires max lane count to pass*/- bool force_max_lane_count;
};
int drm_display_info_set_bus_formats(struct drm_display_info *info,
2.19.1
On Wed, Apr 03, 2019 at 03:14:51PM +0300, Ville Syrjälä wrote:
On Tue, Apr 02, 2019 at 02:52:34PM -0700, Manasi Navare wrote:
For certain eDP 1.4 panels, we need to use max lane count for the link training to succeed.
This patch adds a EDID quirk for such eDP panels using their vendor ID and product ID to force using max lane count in the driver.
Rather than opening the quirk can of worms I think we should consider changing the retry loop to do something more sensible than what it's doing now. The current behaviour of "start at optimal settings (which can be either min lanes or min rate), and then reduce lanes/rate until stuff works" overlooks several possible combinations. One possible approach could be to start the retry loop with max lanes + max rate after the optimal settings have failed. It probably won't give you the best power consumption, but at least you get a picture on the screen if even a single lane count + rate combo works.
Hm yeah I guess this is an approach we haven't tried yet ... I think we've tried everything else already. -Daniel
Cc: Clint Taylor Clinton.A.Taylor@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Tested-by: Albert Astals Cid aacid@kde.org Tested-by: Emanuele Panigati ilpanich@gmail.com Tested-by: Ralgor ralgorfdb@compuspex.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959 Signed-off-by: Manasi Navare manasi.d.navare@intel.com
drivers/gpu/drm/drm_edid.c | 10 ++++++++++ include/drm/drm_connector.h | 5 +++++ 2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2c22ea446075..fbc661806484 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -82,6 +82,8 @@ #define EDID_QUIRK_FORCE_10BPC (1 << 11) /* Non desktop display (i.e. HMD) */ #define EDID_QUIRK_NON_DESKTOP (1 << 12) +/* Force max lane count */ +#define EDID_QUIRK_FORCE_MAX_LANE_COUNT (1 << 13)
struct detailed_mode_closure { struct drm_connector *connector; @@ -189,6 +191,10 @@ static const struct edid_quirk {
/* OSVR HDK and HDK2 VR Headsets */ { "SVR", 0x1019, EDID_QUIRK_NON_DESKTOP },
- /* SHP eDP 1.4 panel only works with max lane count */
- { "SHP", 0x149a, EDID_QUIRK_FORCE_MAX_LANE_COUNT },
- { "SHP", 0x148e, EDID_QUIRK_FORCE_MAX_LANE_COUNT },
};
/* @@ -4463,6 +4469,7 @@ drm_reset_display_info(struct drm_connector *connector) memset(&info->hdmi, 0, sizeof(info->hdmi));
info->non_desktop = 0;
- info->force_max_lane_count = 0;
}
u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid) @@ -4744,6 +4751,9 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) if (quirks & EDID_QUIRK_FORCE_12BPC) connector->display_info.bpc = 12;
- if (quirks & EDID_QUIRK_FORCE_MAX_LANE_COUNT)
connector->display_info.force_max_lane_count = true;- return num_modes;
} EXPORT_SYMBOL(drm_add_edid_modes); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 02a131202add..45436d40ffe3 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -457,6 +457,11 @@ struct drm_display_info { * @non_desktop: Non desktop display (HMD). */ bool non_desktop;
- /**
* @force_max_lane_count: Link training requires max lane count to pass*/- bool force_max_lane_count;
};
int drm_display_info_set_bus_formats(struct drm_display_info *info,
2.19.1
-- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Apr 03, 2019 at 03:14:51PM +0300, Ville Syrjälä wrote:
On Tue, Apr 02, 2019 at 02:52:34PM -0700, Manasi Navare wrote:
For certain eDP 1.4 panels, we need to use max lane count for the link training to succeed.
This patch adds a EDID quirk for such eDP panels using their vendor ID and product ID to force using max lane count in the driver.
Rather than opening the quirk can of worms I think we should consider changing the retry loop to do something more sensible than what it's doing now. The current behaviour of "start at optimal settings (which can be either min lanes or min rate), and then reduce lanes/rate until stuff works" overlooks several possible combinations. One possible approach could be to start the retry loop with max lanes + max rate after the optimal settings have failed. It probably won't give you the best power consumption, but at least you get a picture on the screen if even a single lane count + rate combo works.
So you are saying that for eDP only we should modify the retry function to retry with max lanes and max rate so what we used to do earlier with < eDP 1.4?
Hmm I could try doing that, the only concern I have there is that certain eDP panels just need a retry at same parameters to work so for such panels where the lower values of link rate/lane count work with just an extra retry we would still be using max link rate /lane count now with this change.
Or are you suggesting doing the retry with same params for edp < 1.4 and for all edp 1.4 , we retry with , max link rate lane ocunt?
Manasi
Cc: Clint Taylor Clinton.A.Taylor@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Tested-by: Albert Astals Cid aacid@kde.org Tested-by: Emanuele Panigati ilpanich@gmail.com Tested-by: Ralgor ralgorfdb@compuspex.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959 Signed-off-by: Manasi Navare manasi.d.navare@intel.com
drivers/gpu/drm/drm_edid.c | 10 ++++++++++ include/drm/drm_connector.h | 5 +++++ 2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2c22ea446075..fbc661806484 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -82,6 +82,8 @@ #define EDID_QUIRK_FORCE_10BPC (1 << 11) /* Non desktop display (i.e. HMD) */ #define EDID_QUIRK_NON_DESKTOP (1 << 12) +/* Force max lane count */ +#define EDID_QUIRK_FORCE_MAX_LANE_COUNT (1 << 13)
struct detailed_mode_closure { struct drm_connector *connector; @@ -189,6 +191,10 @@ static const struct edid_quirk {
/* OSVR HDK and HDK2 VR Headsets */ { "SVR", 0x1019, EDID_QUIRK_NON_DESKTOP },
- /* SHP eDP 1.4 panel only works with max lane count */
- { "SHP", 0x149a, EDID_QUIRK_FORCE_MAX_LANE_COUNT },
- { "SHP", 0x148e, EDID_QUIRK_FORCE_MAX_LANE_COUNT },
};
/* @@ -4463,6 +4469,7 @@ drm_reset_display_info(struct drm_connector *connector) memset(&info->hdmi, 0, sizeof(info->hdmi));
info->non_desktop = 0;
- info->force_max_lane_count = 0;
}
u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid) @@ -4744,6 +4751,9 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) if (quirks & EDID_QUIRK_FORCE_12BPC) connector->display_info.bpc = 12;
- if (quirks & EDID_QUIRK_FORCE_MAX_LANE_COUNT)
connector->display_info.force_max_lane_count = true;- return num_modes;
} EXPORT_SYMBOL(drm_add_edid_modes); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 02a131202add..45436d40ffe3 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -457,6 +457,11 @@ struct drm_display_info { * @non_desktop: Non desktop display (HMD). */ bool non_desktop;
- /**
* @force_max_lane_count: Link training requires max lane count to pass*/- bool force_max_lane_count;
};
int drm_display_info_set_bus_formats(struct drm_display_info *info,
2.19.1
-- Ville Syrjälä Intel
On Wed, Apr 03, 2019 at 11:37:21AM -0700, Manasi Navare wrote:
On Wed, Apr 03, 2019 at 03:14:51PM +0300, Ville Syrjälä wrote:
On Tue, Apr 02, 2019 at 02:52:34PM -0700, Manasi Navare wrote:
For certain eDP 1.4 panels, we need to use max lane count for the link training to succeed.
This patch adds a EDID quirk for such eDP panels using their vendor ID and product ID to force using max lane count in the driver.
Rather than opening the quirk can of worms I think we should consider changing the retry loop to do something more sensible than what it's doing now. The current behaviour of "start at optimal settings (which can be either min lanes or min rate), and then reduce lanes/rate until stuff works" overlooks several possible combinations. One possible approach could be to start the retry loop with max lanes + max rate after the optimal settings have failed. It probably won't give you the best power consumption, but at least you get a picture on the screen if even a single lane count + rate combo works.
So you are saying that for eDP only we should modify the retry function to retry with max lanes and max rate so what we used to do earlier with < eDP 1.4?
Hmm I could try doing that, the only concern I have there is that certain eDP panels just need a retry at same parameters to work so for such panels where the lower values of link rate/lane count work with just an extra retry we would still be using max link rate /lane count now with this change.
If the panels are borked then I wouln't worry about it as long a picture appaears on the screen. And if the extra training cycle is because of some bug in our code then we should figure it out what that bug is.
Or are you suggesting doing the retry with same params for edp < 1.4 and for all edp 1.4 , we retry with , max link rate lane ocunt?
retrain_fail() { if (!use_max_params) { use_max_params = true; rate = max; lanes = max; } else { reduce rate/lanes as usual } }
compute_config() { if (use_max_params) { limits.min = limits.max; } ... }
or something like that.
On Wed, Apr 03, 2019 at 09:55:56PM +0300, Ville Syrjälä wrote:
On Wed, Apr 03, 2019 at 11:37:21AM -0700, Manasi Navare wrote:
On Wed, Apr 03, 2019 at 03:14:51PM +0300, Ville Syrjälä wrote:
On Tue, Apr 02, 2019 at 02:52:34PM -0700, Manasi Navare wrote:
For certain eDP 1.4 panels, we need to use max lane count for the link training to succeed.
This patch adds a EDID quirk for such eDP panels using their vendor ID and product ID to force using max lane count in the driver.
Rather than opening the quirk can of worms I think we should consider changing the retry loop to do something more sensible than what it's doing now. The current behaviour of "start at optimal settings (which can be either min lanes or min rate), and then reduce lanes/rate until stuff works" overlooks several possible combinations. One possible approach could be to start the retry loop with max lanes + max rate after the optimal settings have failed. It probably won't give you the best power consumption, but at least you get a picture on the screen if even a single lane count + rate combo works.
So you are saying that for eDP only we should modify the retry function to retry with max lanes and max rate so what we used to do earlier with < eDP 1.4?
Hmm I could try doing that, the only concern I have there is that certain eDP panels just need a retry at same parameters to work so for such panels where the lower values of link rate/lane count work with just an extra retry we would still be using max link rate /lane count now with this change.
If the panels are borked then I wouln't worry about it as long a picture appaears on the screen. And if the extra training cycle is because of some bug in our code then we should figure it out what that bug is.
But we should do this retrain() in our existing modeset_retry() and intel_dp_get_fallback_values() and only do it for eDP right? Because DP we have the retrain loops as per the compliance, if we change that that will affect all the compliance tests.
Manasi
Or are you suggesting doing the retry with same params for edp < 1.4 and for all edp 1.4 , we retry with , max link rate lane ocunt?
retrain_fail() { if (!use_max_params) { use_max_params = true; rate = max; lanes = max; } else { reduce rate/lanes as usual } }
compute_config() { if (use_max_params) { limits.min = limits.max; } ... }
or something like that.
-- Ville Syrjälä Intel
On Wed, Apr 03, 2019 at 12:07:35PM -0700, Manasi Navare wrote:
On Wed, Apr 03, 2019 at 09:55:56PM +0300, Ville Syrjälä wrote:
On Wed, Apr 03, 2019 at 11:37:21AM -0700, Manasi Navare wrote:
On Wed, Apr 03, 2019 at 03:14:51PM +0300, Ville Syrjälä wrote:
On Tue, Apr 02, 2019 at 02:52:34PM -0700, Manasi Navare wrote:
For certain eDP 1.4 panels, we need to use max lane count for the link training to succeed.
This patch adds a EDID quirk for such eDP panels using their vendor ID and product ID to force using max lane count in the driver.
Rather than opening the quirk can of worms I think we should consider changing the retry loop to do something more sensible than what it's doing now. The current behaviour of "start at optimal settings (which can be either min lanes or min rate), and then reduce lanes/rate until stuff works" overlooks several possible combinations. One possible approach could be to start the retry loop with max lanes + max rate after the optimal settings have failed. It probably won't give you the best power consumption, but at least you get a picture on the screen if even a single lane count + rate combo works.
So you are saying that for eDP only we should modify the retry function to retry with max lanes and max rate so what we used to do earlier with < eDP 1.4?
Hmm I could try doing that, the only concern I have there is that certain eDP panels just need a retry at same parameters to work so for such panels where the lower values of link rate/lane count work with just an extra retry we would still be using max link rate /lane count now with this change.
If the panels are borked then I wouln't worry about it as long a picture appaears on the screen. And if the extra training cycle is because of some bug in our code then we should figure it out what that bug is.
But we should do this retrain() in our existing modeset_retry() and intel_dp_get_fallback_values() and only do it for eDP right? Because DP we have the retrain loops as per the compliance, if we change that that will affect all the compliance tests.
I have a rather low opinion on the compliance tests. If they are preventing sensible real world things then what good are they? Someone should probably figure out what it would take to make them more useful...
But anyways, I guess we can start with eDP. If it looks like DP could benefit as well we might have to consider it.
On Wed, Apr 03, 2019 at 10:22:16PM +0300, Ville Syrjälä wrote:
On Wed, Apr 03, 2019 at 12:07:35PM -0700, Manasi Navare wrote:
On Wed, Apr 03, 2019 at 09:55:56PM +0300, Ville Syrjälä wrote:
On Wed, Apr 03, 2019 at 11:37:21AM -0700, Manasi Navare wrote:
On Wed, Apr 03, 2019 at 03:14:51PM +0300, Ville Syrjälä wrote:
On Tue, Apr 02, 2019 at 02:52:34PM -0700, Manasi Navare wrote:
For certain eDP 1.4 panels, we need to use max lane count for the link training to succeed.
This patch adds a EDID quirk for such eDP panels using their vendor ID and product ID to force using max lane count in the driver.
Rather than opening the quirk can of worms I think we should consider changing the retry loop to do something more sensible than what it's doing now. The current behaviour of "start at optimal settings (which can be either min lanes or min rate), and then reduce lanes/rate until stuff works" overlooks several possible combinations. One possible approach could be to start the retry loop with max lanes + max rate after the optimal settings have failed. It probably won't give you the best power consumption, but at least you get a picture on the screen if even a single lane count + rate combo works.
So you are saying that for eDP only we should modify the retry function to retry with max lanes and max rate so what we used to do earlier with < eDP 1.4?
Hmm I could try doing that, the only concern I have there is that certain eDP panels just need a retry at same parameters to work so for such panels where the lower values of link rate/lane count work with just an extra retry we would still be using max link rate /lane count now with this change.
If the panels are borked then I wouln't worry about it as long a picture appaears on the screen. And if the extra training cycle is because of some bug in our code then we should figure it out what that bug is.
But we should do this retrain() in our existing modeset_retry() and intel_dp_get_fallback_values() and only do it for eDP right? Because DP we have the retrain loops as per the compliance, if we change that that will affect all the compliance tests.
I have a rather low opinion on the compliance tests. If they are preventing sensible real world things then what good are they? Someone should probably figure out what it would take to make them more useful...
But anyways, I guess we can start with eDP. If it looks like DP could benefit as well we might have to consider it.
I agree, i will add a patch for edp first and have the folks test it with the edp panels that needed the quirk. DP we really havent had any similar failures where we absolutely need max vales.
Manasi
-- Ville Syrjälä Intel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Clinton Taylor -
Daniel Vetter -
Manasi Navare -
Ville Syrjälä