Thanks for your comments Thierry.
I agree to all your comments.
I will write a general function to return version and repost the patch
Thanks,
Sonika
On Wednesday 29 October 2014 07:12 PM, Thierry Reding wrote:
On Wed, Oct 22, 2014 at 11:45:23AM +0530, sonika.jindal@xxxxxxxxx wrote:
From: Sonika Jindal <sonika.jindal@xxxxxxxxx>
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@xxxxxxxxx>
---
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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
