On Wed, Oct 22, 2014 at 11:45:23AM +0530, sonika.jindal@intel.com wrote:
From: Sonika Jindal sonika.jindal@intel.com
v2: Reading DP_EDP_REV, only when DISPLAY_CONTROL_CAPABLE field is set (Satheesh)
v3: Moving the utility function to drm_dp_helper (Daniel)
Signed-off-by: Sonika Jindal sonika.jindal@intel.com
drivers/gpu/drm/drm_dp_helper.c | 15 +++++++++++++++ include/drm/drm_dp_helper.h | 2 ++ 2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 08e33b8..a54a760 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -768,3 +768,18 @@ void drm_dp_aux_unregister(struct drm_dp_aux *aux) i2c_del_adapter(&aux->ddc); } EXPORT_SYMBOL(drm_dp_aux_unregister);
+bool drm_dp_is_edp_v1_4(struct drm_dp_aux *aux, const u8 dpcd[DP_RECEIVER_CAP_SIZE])
I'd prefer if this didn't take a dpcd argument but rather directly accessed the DP_EDP_CONFIGURATION_CAP register so that it can be used directly rather than rely on the driver to have read a dpcd block in the appropriate format.
+{
- uint8_t reg;
- if (dpcd[DP_EDP_CONFIGURATION_CAP] &
DP_DPCD_DISPLAY_CONTROL_CAPABLE) {if (drm_dp_dpcd_read(aux, DP_EDP_REV, ®, 1))if (reg == 0x03)return true;- }
- return false;
+} +EXPORT_SYMBOL(drm_dp_is_edp_v1_4);
Does it make sense to have a function that checks for a specific version? Why not add one that returns the revision so that it can be compared, something like:
u8 value;
drm_dp_dpcd_read(aux, DP_EDP_REV, &value, 1);
return value;
Then we can do something like:
#define DP_EDP_REV_1_1 0x00 #define DP_EDP_REV_1_2 0x01 #define DP_EDP_REV_1_3 0x02 #define DP_EDP_REV_1_4 0x03
And code can simply compare against that:
drm_dp_get_edp_revision(aux, &rev);
if (rev >= DP_EDP_REV_1_4) { ... }
The check in your variant will only match v1.4 exactly, but presumably v1.5 will be backwards compatible. Having a direct check on the revision code will allow code to continue to work with future, backwards- compatible revisions.
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 8edeed0..b017e1e 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -102,6 +102,8 @@
#define DP_EDP_CONFIGURATION_CAP 0x00d /* XXX 1.2? */ #define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */ +#define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3)
This seems to be a field in the DP_EDP_CONFIGURATION_CAP register, so it should be sorted below that register, not DP_TRAINING_AUX_RD_INTERVAL.
+#define DP_EDP_REV 0x700
And this belongs further down, so it properly sorts into the list of registers.
Thierry