Re: [REBASE 7/7] drm/edid: make drm_edid_are_equal() more convenient for its single user
On Wed, 17 Apr 2024, Thomas Zimmermann wrote: >> Many thanks! Just to double check, do you want me to move patch 5 >> earlier and squash patches 6&7? > > Your choice. Either is fine by me. I jumped at the easy option and merged this as-is. :) Thanks again, Jani. -- Jani Nikula, Intel
Re: [REBASE 7/7] drm/edid: make drm_edid_are_equal() more convenient for its single user
Hi Am 17.04.24 um 10:21 schrieb Jani Nikula: On Tue, 16 Apr 2024, Thomas Zimmermann wrote: Hi Am 16.04.24 um 14:27 schrieb Jani Nikula: On Tue, 16 Apr 2024, Thomas Zimmermann wrote: Hi Am 16.04.24 um 11:20 schrieb Jani Nikula: Repurpose drm_edid_are_equal() to be more helpful for its single user, and rename drm_edid_eq(). Functionally deduce the length from the blob size, not the blob data, making it more robust against any errors. Could be squashed into patch 6. Ack. Thanks for the review. I'll hold of on resending these until there are some R-b's... I've send them a few times already with no comments. :( Feel free to add Reviewed-by: Thomas Zimmermann to the series. Many thanks! Just to double check, do you want me to move patch 5 earlier and squash patches 6&7? Your choice. Either is fine by me. Best regards Thomas BR, Jani. -- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Re: [REBASE 7/7] drm/edid: make drm_edid_are_equal() more convenient for its single user
On Tue, 16 Apr 2024, Thomas Zimmermann wrote: > Hi > > Am 16.04.24 um 14:27 schrieb Jani Nikula: >> On Tue, 16 Apr 2024, Thomas Zimmermann wrote: >>> Hi >>> >>> Am 16.04.24 um 11:20 schrieb Jani Nikula: Repurpose drm_edid_are_equal() to be more helpful for its single user, and rename drm_edid_eq(). Functionally deduce the length from the blob size, not the blob data, making it more robust against any errors. >>> Could be squashed into patch 6. >> Ack. >> >> Thanks for the review. I'll hold of on resending these until there are >> some R-b's... I've send them a few times already with no comments. :( > > Feel free to add > > Reviewed-by: Thomas Zimmermann > > to the series. Many thanks! Just to double check, do you want me to move patch 5 earlier and squash patches 6&7? BR, Jani. -- Jani Nikula, Intel
Re: [REBASE 7/7] drm/edid: make drm_edid_are_equal() more convenient for its single user
Hi
Am 16.04.24 um 14:27 schrieb Jani Nikula:
On Tue, 16 Apr 2024, Thomas Zimmermann wrote:
Hi
Am 16.04.24 um 11:20 schrieb Jani Nikula:
Repurpose drm_edid_are_equal() to be more helpful for its single user,
and rename drm_edid_eq(). Functionally deduce the length from the blob
size, not the blob data, making it more robust against any errors.
Could be squashed into patch 6.
Ack.
Thanks for the review. I'll hold of on resending these until there are
some R-b's... I've send them a few times already with no comments. :(
Feel free to add
Reviewed-by: Thomas Zimmermann
to the series.
Best regards
Thomas
BR,
Jani.
Best regards
Thomas
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 41 ++
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 463fbad85d90..513590931cc5 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1820,30 +1820,20 @@ static bool edid_block_is_zero(const void *edid)
return !memchr_inv(edid, 0, EDID_LENGTH);
}
-/**
- * drm_edid_are_equal - compare two edid blobs.
- * @edid1: pointer to first blob
- * @edid2: pointer to second blob
- * This helper can be used during probing to determine if
- * edid had changed.
- */
-static bool drm_edid_are_equal(const struct edid *edid1, const struct edid
*edid2)
+static bool drm_edid_eq(const struct drm_edid *drm_edid,
+ const void *raw_edid, size_t raw_edid_size)
{
- int edid1_len, edid2_len;
- bool edid1_present = edid1 != NULL;
- bool edid2_present = edid2 != NULL;
+ bool edid1_present = drm_edid && drm_edid->edid && drm_edid->size;
+ bool edid2_present = raw_edid && raw_edid_size;
if (edid1_present != edid2_present)
return false;
- if (edid1) {
- edid1_len = edid_size(edid1);
- edid2_len = edid_size(edid2);
-
- if (edid1_len != edid2_len)
+ if (edid1_present) {
+ if (drm_edid->size != raw_edid_size)
return false;
- if (memcmp(edid1, edid2, edid1_len))
+ if (memcmp(drm_edid->edid, raw_edid, drm_edid->size))
return false;
}
@@ -6936,15 +6926,14 @@ static int _drm_edid_connector_property_update(struct drm_connector *connector,
int ret;
if (connector->edid_blob_ptr) {
- const struct edid *old_edid = connector->edid_blob_ptr->data;
-
- if (old_edid) {
- if (!drm_edid_are_equal(drm_edid ? drm_edid->edid :
NULL, old_edid)) {
- connector->epoch_counter++;
- drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID changed,
epoch counter %llu\n",
- connector->base.id, connector->name,
- connector->epoch_counter);
- }
+ const void *old_edid = connector->edid_blob_ptr->data;
+ size_t old_edid_size = connector->edid_blob_ptr->length;
+
+ if (old_edid && !drm_edid_eq(drm_edid, old_edid,
old_edid_size)) {
+ connector->epoch_counter++;
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID changed, epoch
counter %llu\n",
+ connector->base.id, connector->name,
+ connector->epoch_counter);
}
}
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Re: [REBASE 7/7] drm/edid: make drm_edid_are_equal() more convenient for its single user
On Tue, 16 Apr 2024, Thomas Zimmermann wrote:
> Hi
>
> Am 16.04.24 um 11:20 schrieb Jani Nikula:
>> Repurpose drm_edid_are_equal() to be more helpful for its single user,
>> and rename drm_edid_eq(). Functionally deduce the length from the blob
>> size, not the blob data, making it more robust against any errors.
>
> Could be squashed into patch 6.
Ack.
Thanks for the review. I'll hold of on resending these until there are
some R-b's... I've send them a few times already with no comments. :(
BR,
Jani.
>
> Best regards
> Thomas
>
>>
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/drm_edid.c | 41 ++
>> 1 file changed, 15 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 463fbad85d90..513590931cc5 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1820,30 +1820,20 @@ static bool edid_block_is_zero(const void *edid)
>> return !memchr_inv(edid, 0, EDID_LENGTH);
>> }
>>
>> -/**
>> - * drm_edid_are_equal - compare two edid blobs.
>> - * @edid1: pointer to first blob
>> - * @edid2: pointer to second blob
>> - * This helper can be used during probing to determine if
>> - * edid had changed.
>> - */
>> -static bool drm_edid_are_equal(const struct edid *edid1, const struct edid
>> *edid2)
>> +static bool drm_edid_eq(const struct drm_edid *drm_edid,
>> +const void *raw_edid, size_t raw_edid_size)
>> {
>> -int edid1_len, edid2_len;
>> -bool edid1_present = edid1 != NULL;
>> -bool edid2_present = edid2 != NULL;
>> +bool edid1_present = drm_edid && drm_edid->edid && drm_edid->size;
>> +bool edid2_present = raw_edid && raw_edid_size;
>>
>> if (edid1_present != edid2_present)
>> return false;
>>
>> -if (edid1) {
>> -edid1_len = edid_size(edid1);
>> -edid2_len = edid_size(edid2);
>> -
>> -if (edid1_len != edid2_len)
>> +if (edid1_present) {
>> +if (drm_edid->size != raw_edid_size)
>> return false;
>>
>> -if (memcmp(edid1, edid2, edid1_len))
>> +if (memcmp(drm_edid->edid, raw_edid, drm_edid->size))
>> return false;
>> }
>>
>> @@ -6936,15 +6926,14 @@ static int
>> _drm_edid_connector_property_update(struct drm_connector *connector,
>> int ret;
>>
>> if (connector->edid_blob_ptr) {
>> -const struct edid *old_edid = connector->edid_blob_ptr->data;
>> -
>> -if (old_edid) {
>> -if (!drm_edid_are_equal(drm_edid ? drm_edid->edid :
>> NULL, old_edid)) {
>> -connector->epoch_counter++;
>> -drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID
>> changed, epoch counter %llu\n",
>> -connector->base.id, connector->name,
>> -connector->epoch_counter);
>> -}
>> +const void *old_edid = connector->edid_blob_ptr->data;
>> +size_t old_edid_size = connector->edid_blob_ptr->length;
>> +
>> +if (old_edid && !drm_edid_eq(drm_edid, old_edid,
>> old_edid_size)) {
>> +connector->epoch_counter++;
>> +drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID changed, epoch
>> counter %llu\n",
>> +connector->base.id, connector->name,
>> +connector->epoch_counter);
>> }
>> }
>>
--
Jani Nikula, Intel
Re: [REBASE 7/7] drm/edid: make drm_edid_are_equal() more convenient for its single user
Hi
Am 16.04.24 um 11:20 schrieb Jani Nikula:
Repurpose drm_edid_are_equal() to be more helpful for its single user,
and rename drm_edid_eq(). Functionally deduce the length from the blob
size, not the blob data, making it more robust against any errors.
Could be squashed into patch 6.
Best regards
Thomas
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 41 ++
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 463fbad85d90..513590931cc5 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1820,30 +1820,20 @@ static bool edid_block_is_zero(const void *edid)
return !memchr_inv(edid, 0, EDID_LENGTH);
}
-/**
- * drm_edid_are_equal - compare two edid blobs.
- * @edid1: pointer to first blob
- * @edid2: pointer to second blob
- * This helper can be used during probing to determine if
- * edid had changed.
- */
-static bool drm_edid_are_equal(const struct edid *edid1, const struct edid
*edid2)
+static bool drm_edid_eq(const struct drm_edid *drm_edid,
+ const void *raw_edid, size_t raw_edid_size)
{
- int edid1_len, edid2_len;
- bool edid1_present = edid1 != NULL;
- bool edid2_present = edid2 != NULL;
+ bool edid1_present = drm_edid && drm_edid->edid && drm_edid->size;
+ bool edid2_present = raw_edid && raw_edid_size;
if (edid1_present != edid2_present)
return false;
- if (edid1) {
- edid1_len = edid_size(edid1);
- edid2_len = edid_size(edid2);
-
- if (edid1_len != edid2_len)
+ if (edid1_present) {
+ if (drm_edid->size != raw_edid_size)
return false;
- if (memcmp(edid1, edid2, edid1_len))
+ if (memcmp(drm_edid->edid, raw_edid, drm_edid->size))
return false;
}
@@ -6936,15 +6926,14 @@ static int _drm_edid_connector_property_update(struct drm_connector *connector,
int ret;
if (connector->edid_blob_ptr) {
- const struct edid *old_edid = connector->edid_blob_ptr->data;
-
- if (old_edid) {
- if (!drm_edid_are_equal(drm_edid ? drm_edid->edid :
NULL, old_edid)) {
- connector->epoch_counter++;
- drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID changed,
epoch counter %llu\n",
- connector->base.id, connector->name,
- connector->epoch_counter);
- }
+ const void *old_edid = connector->edid_blob_ptr->data;
+ size_t old_edid_size = connector->edid_blob_ptr->length;
+
+ if (old_edid && !drm_edid_eq(drm_edid, old_edid,
old_edid_size)) {
+ connector->epoch_counter++;
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID changed, epoch
counter %llu\n",
+ connector->base.id, connector->name,
+ connector->epoch_counter);
}
}
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
