* [PATCH v1 01/13] drm/edid: add block count and data helper functions for drm_edid
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-05-24 13:29 ` [Intel-gfx] " Andrzej Hajda
2022-06-02 16:54 ` Ville Syrjälä
2022-05-24 10:39 ` [PATCH v1 02/13] drm/edid: keep track of alloc size in drm_do_get_edid() Jani Nikula
` (11 subsequent siblings)
12 siblings, 2 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula
Add drm_edid based block count and data access helper functions that
take the EDID allocated size into account.
At the moment, the allocated size should always match the EDID size
indicated by the extension count, but this will change in the future.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_edid.c | 42 +++++++++++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 929fc0e46751..682d954a9e42 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1613,6 +1613,35 @@ static const void *edid_extension_block_data(const struct edid *edid, int index)
return edid_block_data(edid, index + 1);
}
+static int drm_edid_block_count(const struct drm_edid *drm_edid)
+{
+ int num_blocks;
+
+ /* Starting point */
+ num_blocks = edid_block_count(drm_edid->edid);
+
+ /* Limit by allocated size */
+ num_blocks = min(num_blocks, (int)drm_edid->size / EDID_LENGTH);
+
+ return num_blocks;
+}
+
+static int drm_edid_extension_block_count(const struct drm_edid *drm_edid)
+{
+ return drm_edid_block_count(drm_edid) - 1;
+}
+
+static const void *drm_edid_block_data(const struct drm_edid *drm_edid, int index)
+{
+ return edid_block_data(drm_edid->edid, index);
+}
+
+static const void *drm_edid_extension_block_data(const struct drm_edid *drm_edid,
+ int index)
+{
+ return edid_extension_block_data(drm_edid->edid, index);
+}
+
/*
* Initializer helper for legacy interfaces, where we have no choice but to
* trust edid size. Not for general purpose use.
@@ -1665,8 +1694,8 @@ static const void *__drm_edid_iter_next(struct drm_edid_iter *iter)
if (!iter->drm_edid)
return NULL;
- if (iter->index < edid_block_count(iter->drm_edid->edid))
- block = edid_block_data(iter->drm_edid->edid, iter->index++);
+ if (iter->index < drm_edid_block_count(iter->drm_edid))
+ block = drm_edid_block_data(iter->drm_edid, iter->index++);
return block;
}
@@ -3574,22 +3603,21 @@ static int add_detailed_modes(struct drm_connector *connector,
const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
int ext_id, int *ext_index)
{
- const struct edid *edid = drm_edid ? drm_edid->edid : NULL;
const u8 *edid_ext = NULL;
int i;
/* No EDID or EDID extensions */
- if (!edid || !edid_extension_block_count(edid))
+ if (!drm_edid || !drm_edid_extension_block_count(drm_edid))
return NULL;
/* Find CEA extension */
- for (i = *ext_index; i < edid_extension_block_count(edid); i++) {
- edid_ext = edid_extension_block_data(edid, i);
+ for (i = *ext_index; i < drm_edid_extension_block_count(drm_edid); i++) {
+ edid_ext = drm_edid_extension_block_data(drm_edid, i);
if (edid_block_tag(edid_ext) == ext_id)
break;
}
- if (i >= edid_extension_block_count(edid))
+ if (i >= drm_edid_extension_block_count(drm_edid))
return NULL;
*ext_index = i + 1;
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [Intel-gfx] [PATCH v1 01/13] drm/edid: add block count and data helper functions for drm_edid
2022-05-24 10:39 ` [PATCH v1 01/13] drm/edid: add block count and data helper functions for drm_edid Jani Nikula
@ 2022-05-24 13:29 ` Andrzej Hajda
2022-05-24 13:41 ` Jani Nikula
2022-06-02 16:54 ` Ville Syrjälä
1 sibling, 1 reply; 24+ messages in thread
From: Andrzej Hajda @ 2022-05-24 13:29 UTC (permalink / raw)
To: Jani Nikula, dri-devel, intel-gfx
On 24.05.2022 12:39, Jani Nikula wrote:
> Add drm_edid based block count and data access helper functions that
> take the EDID allocated size into account.
>
> At the moment, the allocated size should always match the EDID size
> indicated by the extension count, but this will change in the future.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 42 +++++++++++++++++++++++++++++++-------
> 1 file changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 929fc0e46751..682d954a9e42 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1613,6 +1613,35 @@ static const void *edid_extension_block_data(const struct edid *edid, int index)
> return edid_block_data(edid, index + 1);
> }
>
> +static int drm_edid_block_count(const struct drm_edid *drm_edid)
> +{
> + int num_blocks;
> +
> + /* Starting point */
> + num_blocks = edid_block_count(drm_edid->edid);
> +
> + /* Limit by allocated size */
> + num_blocks = min(num_blocks, (int)drm_edid->size / EDID_LENGTH);
> +
> + return num_blocks;
> +}
> +
> +static int drm_edid_extension_block_count(const struct drm_edid *drm_edid)
> +{
> + return drm_edid_block_count(drm_edid) - 1;
> +}
> +
> +static const void *drm_edid_block_data(const struct drm_edid *drm_edid, int index)
> +{
> + return edid_block_data(drm_edid->edid, index);
> +}
> +
> +static const void *drm_edid_extension_block_data(const struct drm_edid *drm_edid,
> + int index)
> +{
> + return edid_extension_block_data(drm_edid->edid, index);
> +}
> +
> /*
> * Initializer helper for legacy interfaces, where we have no choice but to
> * trust edid size. Not for general purpose use.
> @@ -1665,8 +1694,8 @@ static const void *__drm_edid_iter_next(struct drm_edid_iter *iter)
> if (!iter->drm_edid)
> return NULL;
>
> - if (iter->index < edid_block_count(iter->drm_edid->edid))
> - block = edid_block_data(iter->drm_edid->edid, iter->index++);
> + if (iter->index < drm_edid_block_count(iter->drm_edid))
> + block = drm_edid_block_data(iter->drm_edid, iter->index++);
>
> return block;
> }
> @@ -3574,22 +3603,21 @@ static int add_detailed_modes(struct drm_connector *connector,
> const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
> int ext_id, int *ext_index)
> {
> - const struct edid *edid = drm_edid ? drm_edid->edid : NULL;
Do we still need this var?
> const u8 *edid_ext = NULL;
> int i;
>
> /* No EDID or EDID extensions */
> - if (!edid || !edid_extension_block_count(edid))
> + if (!drm_edid || !drm_edid_extension_block_count(drm_edid))
> return NULL;
>
> /* Find CEA extension */
> - for (i = *ext_index; i < edid_extension_block_count(edid); i++) {
> - edid_ext = edid_extension_block_data(edid, i);
> + for (i = *ext_index; i < drm_edid_extension_block_count(drm_edid); i++) {
> + edid_ext = drm_edid_extension_block_data(drm_edid, i);
> if (edid_block_tag(edid_ext) == ext_id)
> break;
> }
>
> - if (i >= edid_extension_block_count(edid))
> + if (i >= drm_edid_extension_block_count(drm_edid))
> return NULL;
>
> *ext_index = i + 1;
It looks OK. Some suggestions to consider:
1. While at it, refactor little bit the code to return ext from 'for'
loop and NULL later (to kill after-loop checks, and better code IMO).
2. Implement kind of iterator, for example
drm_edid_extension_block_next(drm_edid, edid_ext), then use loop:
for (edid_ext = NULL; edid_ext = drm_edid_extension_block_next(drm_edid,
edid_ext;)
...
Up to you.
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Regards
Andrzej
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Intel-gfx] [PATCH v1 01/13] drm/edid: add block count and data helper functions for drm_edid
2022-05-24 13:29 ` [Intel-gfx] " Andrzej Hajda
@ 2022-05-24 13:41 ` Jani Nikula
0 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 13:41 UTC (permalink / raw)
To: Andrzej Hajda, dri-devel, intel-gfx
On Tue, 24 May 2022, Andrzej Hajda <andrzej.hajda@intel.com> wrote:
> On 24.05.2022 12:39, Jani Nikula wrote:
>> Add drm_edid based block count and data access helper functions that
>> take the EDID allocated size into account.
>>
>> At the moment, the allocated size should always match the EDID size
>> indicated by the extension count, but this will change in the future.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/drm_edid.c | 42 +++++++++++++++++++++++++++++++-------
>> 1 file changed, 35 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 929fc0e46751..682d954a9e42 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1613,6 +1613,35 @@ static const void *edid_extension_block_data(const struct edid *edid, int index)
>> return edid_block_data(edid, index + 1);
>> }
>>
>> +static int drm_edid_block_count(const struct drm_edid *drm_edid)
>> +{
>> + int num_blocks;
>> +
>> + /* Starting point */
>> + num_blocks = edid_block_count(drm_edid->edid);
>> +
>> + /* Limit by allocated size */
>> + num_blocks = min(num_blocks, (int)drm_edid->size / EDID_LENGTH);
>> +
>> + return num_blocks;
>> +}
>> +
>> +static int drm_edid_extension_block_count(const struct drm_edid *drm_edid)
>> +{
>> + return drm_edid_block_count(drm_edid) - 1;
>> +}
>> +
>> +static const void *drm_edid_block_data(const struct drm_edid *drm_edid, int index)
>> +{
>> + return edid_block_data(drm_edid->edid, index);
>> +}
>> +
>> +static const void *drm_edid_extension_block_data(const struct drm_edid *drm_edid,
>> + int index)
>> +{
>> + return edid_extension_block_data(drm_edid->edid, index);
>> +}
>> +
>> /*
>> * Initializer helper for legacy interfaces, where we have no choice but to
>> * trust edid size. Not for general purpose use.
>> @@ -1665,8 +1694,8 @@ static const void *__drm_edid_iter_next(struct drm_edid_iter *iter)
>> if (!iter->drm_edid)
>> return NULL;
>>
>> - if (iter->index < edid_block_count(iter->drm_edid->edid))
>> - block = edid_block_data(iter->drm_edid->edid, iter->index++);
>> + if (iter->index < drm_edid_block_count(iter->drm_edid))
>> + block = drm_edid_block_data(iter->drm_edid, iter->index++);
>>
>> return block;
>> }
>> @@ -3574,22 +3603,21 @@ static int add_detailed_modes(struct drm_connector *connector,
>> const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
>> int ext_id, int *ext_index)
>> {
>> - const struct edid *edid = drm_edid ? drm_edid->edid : NULL;
>
> Do we still need this var?
I am removing it...?
>
>
>> const u8 *edid_ext = NULL;
>> int i;
>>
>> /* No EDID or EDID extensions */
>> - if (!edid || !edid_extension_block_count(edid))
>> + if (!drm_edid || !drm_edid_extension_block_count(drm_edid))
>> return NULL;
>>
>> /* Find CEA extension */
>> - for (i = *ext_index; i < edid_extension_block_count(edid); i++) {
>> - edid_ext = edid_extension_block_data(edid, i);
>> + for (i = *ext_index; i < drm_edid_extension_block_count(drm_edid); i++) {
>> + edid_ext = drm_edid_extension_block_data(drm_edid, i);
>> if (edid_block_tag(edid_ext) == ext_id)
>> break;
>> }
>>
>> - if (i >= edid_extension_block_count(edid))
>> + if (i >= drm_edid_extension_block_count(drm_edid))
>> return NULL;
>>
>> *ext_index = i + 1;
>
> It looks OK. Some suggestions to consider:
> 1. While at it, refactor little bit the code to return ext from 'for'
> loop and NULL later (to kill after-loop checks, and better code IMO).
> 2. Implement kind of iterator, for example
> drm_edid_extension_block_next(drm_edid, edid_ext), then use loop:
> for (edid_ext = NULL; edid_ext = drm_edid_extension_block_next(drm_edid,
> edid_ext;)
> ...
>
> Up to you.
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
There's already the struct drm_edid_iter stuff that this could be
converted to, but just haven't gotten around to it yet. I'll follow up
with that later. Thanks for the review.
BR,
Jani.
>
> Regards
> Andrzej
>
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 01/13] drm/edid: add block count and data helper functions for drm_edid
2022-05-24 10:39 ` [PATCH v1 01/13] drm/edid: add block count and data helper functions for drm_edid Jani Nikula
2022-05-24 13:29 ` [Intel-gfx] " Andrzej Hajda
@ 2022-06-02 16:54 ` Ville Syrjälä
2022-06-02 17:08 ` Jani Nikula
1 sibling, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2022-06-02 16:54 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, dri-devel
On Tue, May 24, 2022 at 01:39:23PM +0300, Jani Nikula wrote:
> Add drm_edid based block count and data access helper functions that
> take the EDID allocated size into account.
>
> At the moment, the allocated size should always match the EDID size
> indicated by the extension count, but this will change in the future.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 42 +++++++++++++++++++++++++++++++-------
> 1 file changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 929fc0e46751..682d954a9e42 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1613,6 +1613,35 @@ static const void *edid_extension_block_data(const struct edid *edid, int index)
> return edid_block_data(edid, index + 1);
> }
>
> +static int drm_edid_block_count(const struct drm_edid *drm_edid)
> +{
> + int num_blocks;
> +
> + /* Starting point */
> + num_blocks = edid_block_count(drm_edid->edid);
> +
> + /* Limit by allocated size */
> + num_blocks = min(num_blocks, (int)drm_edid->size / EDID_LENGTH);
Hmm. Is there a particular reason we couldn't just always
return drm_edid->size/EDID_LENGTH here? That is, why would we not
set drm_edid->size to always reflect the actual size of the EDID?
> +
> + return num_blocks;
> +}
> +
> +static int drm_edid_extension_block_count(const struct drm_edid *drm_edid)
> +{
> + return drm_edid_block_count(drm_edid) - 1;
> +}
> +
> +static const void *drm_edid_block_data(const struct drm_edid *drm_edid, int index)
> +{
> + return edid_block_data(drm_edid->edid, index);
> +}
> +
> +static const void *drm_edid_extension_block_data(const struct drm_edid *drm_edid,
> + int index)
> +{
> + return edid_extension_block_data(drm_edid->edid, index);
> +}
> +
> /*
> * Initializer helper for legacy interfaces, where we have no choice but to
> * trust edid size. Not for general purpose use.
> @@ -1665,8 +1694,8 @@ static const void *__drm_edid_iter_next(struct drm_edid_iter *iter)
> if (!iter->drm_edid)
> return NULL;
>
> - if (iter->index < edid_block_count(iter->drm_edid->edid))
> - block = edid_block_data(iter->drm_edid->edid, iter->index++);
> + if (iter->index < drm_edid_block_count(iter->drm_edid))
> + block = drm_edid_block_data(iter->drm_edid, iter->index++);
>
> return block;
> }
> @@ -3574,22 +3603,21 @@ static int add_detailed_modes(struct drm_connector *connector,
> const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
> int ext_id, int *ext_index)
> {
> - const struct edid *edid = drm_edid ? drm_edid->edid : NULL;
> const u8 *edid_ext = NULL;
> int i;
>
> /* No EDID or EDID extensions */
> - if (!edid || !edid_extension_block_count(edid))
> + if (!drm_edid || !drm_edid_extension_block_count(drm_edid))
> return NULL;
>
> /* Find CEA extension */
> - for (i = *ext_index; i < edid_extension_block_count(edid); i++) {
> - edid_ext = edid_extension_block_data(edid, i);
> + for (i = *ext_index; i < drm_edid_extension_block_count(drm_edid); i++) {
> + edid_ext = drm_edid_extension_block_data(drm_edid, i);
> if (edid_block_tag(edid_ext) == ext_id)
> break;
> }
>
> - if (i >= edid_extension_block_count(edid))
> + if (i >= drm_edid_extension_block_count(drm_edid))
> return NULL;
>
> *ext_index = i + 1;
> --
> 2.30.2
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v1 01/13] drm/edid: add block count and data helper functions for drm_edid
2022-06-02 16:54 ` Ville Syrjälä
@ 2022-06-02 17:08 ` Jani Nikula
0 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-06-02 17:08 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Thu, 02 Jun 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, May 24, 2022 at 01:39:23PM +0300, Jani Nikula wrote:
>> Add drm_edid based block count and data access helper functions that
>> take the EDID allocated size into account.
>>
>> At the moment, the allocated size should always match the EDID size
>> indicated by the extension count, but this will change in the future.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/drm_edid.c | 42 +++++++++++++++++++++++++++++++-------
>> 1 file changed, 35 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 929fc0e46751..682d954a9e42 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1613,6 +1613,35 @@ static const void *edid_extension_block_data(const struct edid *edid, int index)
>> return edid_block_data(edid, index + 1);
>> }
>>
>> +static int drm_edid_block_count(const struct drm_edid *drm_edid)
>> +{
>> + int num_blocks;
>> +
>> + /* Starting point */
>> + num_blocks = edid_block_count(drm_edid->edid);
>> +
>> + /* Limit by allocated size */
>> + num_blocks = min(num_blocks, (int)drm_edid->size / EDID_LENGTH);
>
> Hmm. Is there a particular reason we couldn't just always
> return drm_edid->size/EDID_LENGTH here? That is, why would we not
> set drm_edid->size to always reflect the actual size of the EDID?
Basically playing it safe for obscure cases and the future. For example
EDIDs originating from firmware, debugfs or i915 opregion could have a
bigger size than indicated by the extension count, if we want to go to a
direction where it's really just a blob with size.
BR,
Jani.
>
>> +
>> + return num_blocks;
>> +}
>> +
>> +static int drm_edid_extension_block_count(const struct drm_edid *drm_edid)
>> +{
>> + return drm_edid_block_count(drm_edid) - 1;
>> +}
>> +
>> +static const void *drm_edid_block_data(const struct drm_edid *drm_edid, int index)
>> +{
>> + return edid_block_data(drm_edid->edid, index);
>> +}
>> +
>> +static const void *drm_edid_extension_block_data(const struct drm_edid *drm_edid,
>> + int index)
>> +{
>> + return edid_extension_block_data(drm_edid->edid, index);
>> +}
>> +
>> /*
>> * Initializer helper for legacy interfaces, where we have no choice but to
>> * trust edid size. Not for general purpose use.
>> @@ -1665,8 +1694,8 @@ static const void *__drm_edid_iter_next(struct drm_edid_iter *iter)
>> if (!iter->drm_edid)
>> return NULL;
>>
>> - if (iter->index < edid_block_count(iter->drm_edid->edid))
>> - block = edid_block_data(iter->drm_edid->edid, iter->index++);
>> + if (iter->index < drm_edid_block_count(iter->drm_edid))
>> + block = drm_edid_block_data(iter->drm_edid, iter->index++);
>>
>> return block;
>> }
>> @@ -3574,22 +3603,21 @@ static int add_detailed_modes(struct drm_connector *connector,
>> const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
>> int ext_id, int *ext_index)
>> {
>> - const struct edid *edid = drm_edid ? drm_edid->edid : NULL;
>> const u8 *edid_ext = NULL;
>> int i;
>>
>> /* No EDID or EDID extensions */
>> - if (!edid || !edid_extension_block_count(edid))
>> + if (!drm_edid || !drm_edid_extension_block_count(drm_edid))
>> return NULL;
>>
>> /* Find CEA extension */
>> - for (i = *ext_index; i < edid_extension_block_count(edid); i++) {
>> - edid_ext = edid_extension_block_data(edid, i);
>> + for (i = *ext_index; i < drm_edid_extension_block_count(drm_edid); i++) {
>> + edid_ext = drm_edid_extension_block_data(drm_edid, i);
>> if (edid_block_tag(edid_ext) == ext_id)
>> break;
>> }
>>
>> - if (i >= edid_extension_block_count(edid))
>> + if (i >= drm_edid_extension_block_count(drm_edid))
>> return NULL;
>>
>> *ext_index = i + 1;
>> --
>> 2.30.2
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v1 02/13] drm/edid: keep track of alloc size in drm_do_get_edid()
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
2022-05-24 10:39 ` [PATCH v1 01/13] drm/edid: add block count and data helper functions for drm_edid Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-05-24 10:39 ` [PATCH v1 03/13] drm/edid: add new interfaces around struct drm_edid Jani Nikula
` (10 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula
We'll want to return the allocated buffer size in the future. Keep track
of it.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_edid.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 682d954a9e42..2132a38ed701 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2021,13 +2021,16 @@ bool drm_edid_is_valid(struct edid *edid)
EXPORT_SYMBOL(drm_edid_is_valid);
static struct edid *edid_filter_invalid_blocks(const struct edid *edid,
- int invalid_blocks)
+ int invalid_blocks,
+ size_t *alloc_size)
{
struct edid *new, *dest_block;
int valid_extensions = edid->extensions - invalid_blocks;
int i;
- new = kmalloc(edid_size_by_blocks(valid_extensions + 1), GFP_KERNEL);
+ *alloc_size = edid_size_by_blocks(valid_extensions + 1);
+
+ new = kmalloc(*alloc_size, GFP_KERNEL);
if (!new)
goto out;
@@ -2140,7 +2143,8 @@ static void connector_bad_edid(struct drm_connector *connector,
}
/* Get override or firmware EDID */
-static struct edid *drm_get_override_edid(struct drm_connector *connector)
+static struct edid *drm_get_override_edid(struct drm_connector *connector,
+ size_t *alloc_size)
{
struct edid *override = NULL;
@@ -2150,6 +2154,10 @@ static struct edid *drm_get_override_edid(struct drm_connector *connector)
if (!override)
override = drm_load_edid_firmware(connector);
+ /* FIXME: Get alloc size from deeper down the stack */
+ if (!IS_ERR_OR_NULL(override) && alloc_size)
+ *alloc_size = edid_size(override);
+
return IS_ERR(override) ? NULL : override;
}
@@ -2169,7 +2177,7 @@ int drm_add_override_edid_modes(struct drm_connector *connector)
struct edid *override;
int num_modes = 0;
- override = drm_get_override_edid(connector);
+ override = drm_get_override_edid(connector, NULL);
if (override) {
drm_connector_update_edid_property(connector, override);
num_modes = drm_add_edid_modes(connector, override);
@@ -2245,12 +2253,13 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
enum edid_block_status status;
int i, invalid_blocks = 0;
struct edid *edid, *new;
+ size_t alloc_size = EDID_LENGTH;
- edid = drm_get_override_edid(connector);
+ edid = drm_get_override_edid(connector, &alloc_size);
if (edid)
goto ok;
- edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
+ edid = kmalloc(alloc_size, GFP_KERNEL);
if (!edid)
return NULL;
@@ -2278,7 +2287,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
if (!edid_extension_block_count(edid))
goto ok;
- new = krealloc(edid, edid_size(edid), GFP_KERNEL);
+ alloc_size = edid_size(edid);
+ new = krealloc(edid, alloc_size, GFP_KERNEL);
if (!new)
goto fail;
edid = new;
@@ -2300,7 +2310,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
if (invalid_blocks) {
connector_bad_edid(connector, edid, edid_block_count(edid));
- edid = edid_filter_invalid_blocks(edid, invalid_blocks);
+ edid = edid_filter_invalid_blocks(edid, invalid_blocks,
+ &alloc_size);
}
ok:
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v1 03/13] drm/edid: add new interfaces around struct drm_edid
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
2022-05-24 10:39 ` [PATCH v1 01/13] drm/edid: add block count and data helper functions for drm_edid Jani Nikula
2022-05-24 10:39 ` [PATCH v1 02/13] drm/edid: keep track of alloc size in drm_do_get_edid() Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-05-24 10:39 ` [PATCH v1 04/13] drm/edid: add drm_edid_connector_update() Jani Nikula
` (9 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula, David Airlie
Add new functions drm_edid_read(), drm_edid_read_ddc(), and
drm_edid_read_custom() to replace drm_get_edid() and drm_do_get_edid()
for reading the EDID. The transition is expected to happen over a fairly
long time.
Note that the new drm_edid_read*() functions do not do any of the
connector updates anymore. The reading and parsing will be completely
separated from each other.
Add new functions drm_edid_alloc(), drm_edid_dup(), and drm_edid_free()
for allocating and freeing drm_edid containers.
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_edid.c | 245 +++++++++++++++++++++++++++++++++----
include/drm/drm_edid.h | 9 ++
2 files changed, 230 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 2132a38ed701..0d640e7dcff7 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2226,29 +2226,9 @@ static enum edid_block_status edid_block_read(void *block, unsigned int block_nu
return status;
}
-/**
- * drm_do_get_edid - get EDID data using a custom EDID block read function
- * @connector: connector we're probing
- * @read_block: EDID block read function
- * @context: private data passed to the block read function
- *
- * When the I2C adapter connected to the DDC bus is hidden behind a device that
- * exposes a different interface to read EDID blocks this function can be used
- * to get EDID data using a custom block read function.
- *
- * As in the general case the DDC bus is accessible by the kernel at the I2C
- * level, drivers must make all reasonable efforts to expose it as an I2C
- * adapter and use drm_get_edid() instead of abusing this function.
- *
- * The EDID may be overridden using debugfs override_edid or firmware EDID
- * (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
- * order. Having either of them bypasses actual EDID reads.
- *
- * Return: Pointer to valid EDID or NULL if we couldn't find any.
- */
-struct edid *drm_do_get_edid(struct drm_connector *connector,
- read_block_fn read_block,
- void *context)
+static struct edid *_drm_do_get_edid(struct drm_connector *connector,
+ read_block_fn read_block, void *context,
+ size_t *size)
{
enum edid_block_status status;
int i, invalid_blocks = 0;
@@ -2315,14 +2295,125 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
}
ok:
+ if (size)
+ *size = alloc_size;
+
return edid;
fail:
kfree(edid);
return NULL;
}
+
+/**
+ * drm_do_get_edid - get EDID data using a custom EDID block read function
+ * @connector: connector we're probing
+ * @read_block: EDID block read function
+ * @context: private data passed to the block read function
+ *
+ * When the I2C adapter connected to the DDC bus is hidden behind a device that
+ * exposes a different interface to read EDID blocks this function can be used
+ * to get EDID data using a custom block read function.
+ *
+ * As in the general case the DDC bus is accessible by the kernel at the I2C
+ * level, drivers must make all reasonable efforts to expose it as an I2C
+ * adapter and use drm_get_edid() instead of abusing this function.
+ *
+ * The EDID may be overridden using debugfs override_edid or firmware EDID
+ * (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
+ * order. Having either of them bypasses actual EDID reads.
+ *
+ * Return: Pointer to valid EDID or NULL if we couldn't find any.
+ */
+struct edid *drm_do_get_edid(struct drm_connector *connector,
+ read_block_fn read_block,
+ void *context)
+{
+ return _drm_do_get_edid(connector, read_block, context, NULL);
+}
EXPORT_SYMBOL_GPL(drm_do_get_edid);
+/* Allocate struct drm_edid container *without* duplicating the edid data */
+static const struct drm_edid *_drm_edid_alloc(const void *edid, size_t size)
+{
+ struct drm_edid *drm_edid;
+
+ if (!edid || !size || size < EDID_LENGTH)
+ return NULL;
+
+ drm_edid = kzalloc(sizeof(*drm_edid), GFP_KERNEL);
+ if (drm_edid) {
+ drm_edid->edid = edid;
+ drm_edid->size = size;
+ }
+
+ return drm_edid;
+}
+
+/**
+ * drm_edid_alloc - Allocate a new drm_edid container
+ * @edid: Pointer to raw EDID data
+ * @size: Size of memory allocated for EDID
+ *
+ * Allocate a new drm_edid container. Do not calculate edid size from edid, pass
+ * the actual size that has been allocated for the data. There is no validation
+ * of the raw EDID data against the size, but at least the EDID base block must
+ * fit in the buffer.
+ *
+ * The returned pointer must be freed using drm_edid_free().
+ *
+ * Return: drm_edid container, or NULL on errors
+ */
+const struct drm_edid *drm_edid_alloc(const void *edid, size_t size)
+{
+ const struct drm_edid *drm_edid;
+
+ if (!edid || !size || size < EDID_LENGTH)
+ return NULL;
+
+ edid = kmemdup(edid, size, GFP_KERNEL);
+ if (!edid)
+ return NULL;
+
+ drm_edid = _drm_edid_alloc(edid, size);
+ if (!drm_edid)
+ kfree(edid);
+
+ return drm_edid;
+}
+EXPORT_SYMBOL(drm_edid_alloc);
+
+/**
+ * drm_edid_dup - Duplicate a drm_edid container
+ * @drm_edid: EDID to duplicate
+ *
+ * The returned pointer must be freed using drm_edid_free().
+ *
+ * Returns: drm_edid container copy, or NULL on errors
+ */
+const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid)
+{
+ if (!drm_edid)
+ return NULL;
+
+ return drm_edid_alloc(drm_edid->edid, drm_edid->size);
+}
+EXPORT_SYMBOL(drm_edid_dup);
+
+/**
+ * drm_edid_free - Free the drm_edid container
+ * @drm_edid: EDID to free
+ */
+void drm_edid_free(const struct drm_edid *drm_edid)
+{
+ if (!drm_edid)
+ return;
+
+ kfree(drm_edid->edid);
+ kfree(drm_edid);
+}
+EXPORT_SYMBOL(drm_edid_free);
+
/**
* drm_probe_ddc() - probe DDC presence
* @adapter: I2C adapter to probe
@@ -2359,12 +2450,118 @@ struct edid *drm_get_edid(struct drm_connector *connector,
if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter))
return NULL;
- edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);
+ edid = _drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter, NULL);
drm_connector_update_edid_property(connector, edid);
return edid;
}
EXPORT_SYMBOL(drm_get_edid);
+/**
+ * drm_edid_read_custom - Read EDID data using given EDID block read function
+ * @connector: Connector to use
+ * @read_block: EDID block read function
+ * @context: Private data passed to the block read function
+ *
+ * When the I2C adapter connected to the DDC bus is hidden behind a device that
+ * exposes a different interface to read EDID blocks this function can be used
+ * to get EDID data using a custom block read function.
+ *
+ * As in the general case the DDC bus is accessible by the kernel at the I2C
+ * level, drivers must make all reasonable efforts to expose it as an I2C
+ * adapter and use drm_edid_read() or drm_edid_read_ddc() instead of abusing
+ * this function.
+ *
+ * The EDID may be overridden using debugfs override_edid or firmware EDID
+ * (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
+ * order. Having either of them bypasses actual EDID reads.
+ *
+ * The returned pointer must be freed using drm_edid_free().
+ *
+ * Return: Pointer to EDID, or NULL if probe/read failed.
+ */
+const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector,
+ read_block_fn read_block,
+ void *context)
+{
+ const struct drm_edid *drm_edid;
+ struct edid *edid;
+ size_t size = 0;
+
+ edid = _drm_do_get_edid(connector, read_block, context, &size);
+ if (!edid)
+ return NULL;
+
+ /* Sanity check for now */
+ drm_WARN_ON(connector->dev, !size);
+
+ drm_edid = _drm_edid_alloc(edid, size);
+ if (!drm_edid)
+ kfree(edid);
+
+ return drm_edid;
+}
+EXPORT_SYMBOL(drm_edid_read_custom);
+
+/**
+ * drm_edid_read_ddc - Read EDID data using given I2C adapter
+ * @connector: Connector to use
+ * @adapter: I2C adapter to use for DDC
+ *
+ * Read EDID using the given I2C adapter.
+ *
+ * The EDID may be overridden using debugfs override_edid or firmware EDID
+ * (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
+ * order. Having either of them bypasses actual EDID reads.
+ *
+ * Prefer initializing connector->ddc with drm_connector_init_with_ddc() and
+ * using drm_edid_read() instead of this function.
+ *
+ * The returned pointer must be freed using drm_edid_free().
+ *
+ * Return: Pointer to EDID, or NULL if probe/read failed.
+ */
+const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,
+ struct i2c_adapter *adapter)
+{
+ const struct drm_edid *drm_edid;
+
+ if (connector->force == DRM_FORCE_OFF)
+ return NULL;
+
+ if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter))
+ return NULL;
+
+ drm_edid = drm_edid_read_custom(connector, drm_do_probe_ddc_edid, adapter);
+
+ /* Note: Do *not* call connector updates here. */
+
+ return drm_edid;
+}
+EXPORT_SYMBOL(drm_edid_read_ddc);
+
+/**
+ * drm_edid_read - Read EDID data using connector's I2C adapter
+ * @connector: Connector to use
+ *
+ * Read EDID using the connector's I2C adapter.
+ *
+ * The EDID may be overridden using debugfs override_edid or firmware EDID
+ * (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
+ * order. Having either of them bypasses actual EDID reads.
+ *
+ * The returned pointer must be freed using drm_edid_free().
+ *
+ * Return: Pointer to EDID, or NULL if probe/read failed.
+ */
+const struct drm_edid *drm_edid_read(struct drm_connector *connector)
+{
+ if (drm_WARN_ON(connector->dev, !connector->ddc))
+ return NULL;
+
+ return drm_edid_read_ddc(connector, connector->ddc);
+}
+EXPORT_SYMBOL(drm_edid_read);
+
static u32 edid_extract_panel_id(const struct edid *edid)
{
/*
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index c61e75ab8f63..b729e18968dd 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -581,6 +581,15 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
u8 video_code);
/* Interface based on struct drm_edid */
+const struct drm_edid *drm_edid_alloc(const void *edid, size_t size);
+const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid);
+void drm_edid_free(const struct drm_edid *drm_edid);
+const struct drm_edid *drm_edid_read(struct drm_connector *connector);
+const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,
+ struct i2c_adapter *adapter);
+const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector,
+ int (*read_block)(void *context, u8 *buf, unsigned int block, size_t len),
+ void *context);
const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
int ext_id, int *ext_index);
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v1 04/13] drm/edid: add drm_edid_connector_update()
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
` (2 preceding siblings ...)
2022-05-24 10:39 ` [PATCH v1 03/13] drm/edid: add new interfaces around struct drm_edid Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-06-02 17:09 ` Ville Syrjälä
2022-05-24 10:39 ` [PATCH v1 05/13] drm/probe-helper: abstract .get_modes() connector helper call Jani Nikula
` (8 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula, David Airlie
Add a new function drm_edid_connector_update() to replace the
combination of calls drm_connector_update_edid_property() and
drm_add_edid_modes(). Usually they are called in the drivers in this
order, however the former needs information from the latter.
This is all in drm_edid.c simply to keep struct drm_edid opaque.
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_connector.c | 2 +
drivers/gpu/drm/drm_edid.c | 71 +++++++++++++++++++++++++++++++--
include/drm/drm_edid.h | 2 +
3 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 1c48d162c77e..ae9c640a641a 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2088,6 +2088,8 @@ EXPORT_SYMBOL(drm_connector_set_tile_property);
* set the connector's tile property here. See drm_connector_set_tile_property()
* for more details.
*
+ * This function is deprecated. Use drm_edid_connector_update() instead.
+ *
* Returns:
* Zero on success, negative errno on failure.
*/
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0d640e7dcff7..09328298aabf 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6127,8 +6127,8 @@ static int add_displayid_detailed_modes(struct drm_connector *connector,
return num_modes;
}
-static int drm_edid_connector_update(struct drm_connector *connector,
- const struct drm_edid *drm_edid)
+static int _drm_edid_connector_update(struct drm_connector *connector,
+ const struct drm_edid *drm_edid)
{
int num_modes = 0;
u32 quirks;
@@ -6191,6 +6191,67 @@ static int drm_edid_connector_update(struct drm_connector *connector,
return num_modes;
}
+static void _drm_update_tile_info(struct drm_connector *connector,
+ const struct drm_edid *drm_edid);
+
+/**
+ * drm_edid_connector_update - Update connector information from EDID
+ * @connector: Connector
+ * @drm_edid: EDID
+ *
+ * Update the connector mode list, display info, ELD, HDR metadata, relevant
+ * properties, etc. from the passed in EDID.
+ *
+ * If EDID is NULL, reset the information.
+ *
+ * Return: The number of modes added or 0 if we couldn't find any.
+ */
+int drm_edid_connector_update(struct drm_connector *connector,
+ const struct drm_edid *drm_edid)
+{
+ struct drm_device *dev = connector->dev;
+ const struct edid *old_edid = connector->edid_blob_ptr ?
+ connector->edid_blob_ptr->data : NULL;
+ const struct edid *edid = drm_edid ? drm_edid->edid : NULL;
+ size_t size = drm_edid ? drm_edid->size : 0;
+ int count, ret;
+
+ count = _drm_edid_connector_update(connector, drm_edid);
+
+ _drm_update_tile_info(connector, drm_edid);
+
+ if (old_edid && !drm_edid_are_equal(edid, 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);
+ }
+
+ ret = drm_property_replace_global_blob(dev, &connector->edid_blob_ptr,
+ size, edid,
+ &connector->base,
+ dev->mode_config.edid_property);
+ if (ret)
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID property update failed (%d)\n",
+ connector->base.id, connector->name, ret);
+
+ ret = drm_object_property_set_value(&connector->base,
+ dev->mode_config.non_desktop_property,
+ connector->display_info.non_desktop);
+ if (ret)
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Non-desktop property update failed (%d)\n",
+ connector->base.id, connector->name, ret);
+
+ ret = drm_connector_set_tile_property(connector);
+ if (ret)
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Tile property update failed (%d)\n",
+ connector->base.id, connector->name, ret);
+
+ return count;
+}
+EXPORT_SYMBOL(drm_edid_connector_update);
+
/**
* drm_add_edid_modes - add modes from EDID data, if available
* @connector: connector we're probing
@@ -6200,6 +6261,8 @@ static int drm_edid_connector_update(struct drm_connector *connector,
* &drm_display_info structure and ELD in @connector with any information which
* can be derived from the edid.
*
+ * This function is deprecated. Use drm_edid_connector_update() instead.
+ *
* Return: The number of modes added or 0 if we couldn't find any.
*/
int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
@@ -6212,8 +6275,8 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
edid = NULL;
}
- return drm_edid_connector_update(connector,
- drm_edid_legacy_init(&drm_edid, edid));
+ return _drm_edid_connector_update(connector,
+ drm_edid_legacy_init(&drm_edid, edid));
}
EXPORT_SYMBOL(drm_add_edid_modes);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index b729e18968dd..289b956d31ea 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -590,6 +590,8 @@ const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,
const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector,
int (*read_block)(void *context, u8 *buf, unsigned int block, size_t len),
void *context);
+int drm_edid_connector_update(struct drm_connector *connector,
+ const struct drm_edid *edid);
const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
int ext_id, int *ext_index);
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v1 04/13] drm/edid: add drm_edid_connector_update()
2022-05-24 10:39 ` [PATCH v1 04/13] drm/edid: add drm_edid_connector_update() Jani Nikula
@ 2022-06-02 17:09 ` Ville Syrjälä
0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2022-06-02 17:09 UTC (permalink / raw)
To: Jani Nikula; +Cc: David Airlie, intel-gfx, dri-devel
On Tue, May 24, 2022 at 01:39:26PM +0300, Jani Nikula wrote:
> Add a new function drm_edid_connector_update() to replace the
> combination of calls drm_connector_update_edid_property() and
> drm_add_edid_modes(). Usually they are called in the drivers in this
> order, however the former needs information from the latter.
>
> This is all in drm_edid.c simply to keep struct drm_edid opaque.
>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_connector.c | 2 +
> drivers/gpu/drm/drm_edid.c | 71 +++++++++++++++++++++++++++++++--
> include/drm/drm_edid.h | 2 +
> 3 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 1c48d162c77e..ae9c640a641a 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2088,6 +2088,8 @@ EXPORT_SYMBOL(drm_connector_set_tile_property);
> * set the connector's tile property here. See drm_connector_set_tile_property()
> * for more details.
> *
> + * This function is deprecated. Use drm_edid_connector_update() instead.
> + *
> * Returns:
> * Zero on success, negative errno on failure.
> */
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 0d640e7dcff7..09328298aabf 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6127,8 +6127,8 @@ static int add_displayid_detailed_modes(struct drm_connector *connector,
> return num_modes;
> }
>
> -static int drm_edid_connector_update(struct drm_connector *connector,
> - const struct drm_edid *drm_edid)
> +static int _drm_edid_connector_update(struct drm_connector *connector,
> + const struct drm_edid *drm_edid)
> {
> int num_modes = 0;
> u32 quirks;
> @@ -6191,6 +6191,67 @@ static int drm_edid_connector_update(struct drm_connector *connector,
> return num_modes;
> }
>
> +static void _drm_update_tile_info(struct drm_connector *connector,
> + const struct drm_edid *drm_edid);
> +
> +/**
> + * drm_edid_connector_update - Update connector information from EDID
> + * @connector: Connector
> + * @drm_edid: EDID
> + *
> + * Update the connector mode list, display info, ELD, HDR metadata, relevant
> + * properties, etc. from the passed in EDID.
> + *
> + * If EDID is NULL, reset the information.
> + *
> + * Return: The number of modes added or 0 if we couldn't find any.
> + */
> +int drm_edid_connector_update(struct drm_connector *connector,
> + const struct drm_edid *drm_edid)
> +{
> + struct drm_device *dev = connector->dev;
> + const struct edid *old_edid = connector->edid_blob_ptr ?
> + connector->edid_blob_ptr->data : NULL;
> + const struct edid *edid = drm_edid ? drm_edid->edid : NULL;
> + size_t size = drm_edid ? drm_edid->size : 0;
> + int count, ret;
> +
> + count = _drm_edid_connector_update(connector, drm_edid);
> +
> + _drm_update_tile_info(connector, drm_edid);
> +
> + if (old_edid && !drm_edid_are_equal(edid, 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);
There was some recent discussion on the blob IDs changing willy nilly.
We could avoid some of that by bailing out here if nothing changed.
But that's not really something that should be done in this patch.
> + }
> +
> + ret = drm_property_replace_global_blob(dev, &connector->edid_blob_ptr,
> + size, edid,
> + &connector->base,
> + dev->mode_config.edid_property);
> + if (ret)
> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID property update failed (%d)\n",
> + connector->base.id, connector->name, ret);
> +
> + ret = drm_object_property_set_value(&connector->base,
> + dev->mode_config.non_desktop_property,
> + connector->display_info.non_desktop);
> + if (ret)
> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Non-desktop property update failed (%d)\n",
> + connector->base.id, connector->name, ret);
> +
> + ret = drm_connector_set_tile_property(connector);
> + if (ret)
> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Tile property update failed (%d)\n",
> + connector->base.id, connector->name, ret);
I was a bit confused why this function is needed, until I recalled that
the EDID read thing you introduce in the earlier patch doesn't do
any of this (unlike the old function). Would be good to point that
out in the commit message.
> +
> + return count;
> +}
> +EXPORT_SYMBOL(drm_edid_connector_update);
> +
> /**
> * drm_add_edid_modes - add modes from EDID data, if available
> * @connector: connector we're probing
> @@ -6200,6 +6261,8 @@ static int drm_edid_connector_update(struct drm_connector *connector,
> * &drm_display_info structure and ELD in @connector with any information which
> * can be derived from the edid.
> *
> + * This function is deprecated. Use drm_edid_connector_update() instead.
> + *
> * Return: The number of modes added or 0 if we couldn't find any.
> */
> int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> @@ -6212,8 +6275,8 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> edid = NULL;
> }
>
> - return drm_edid_connector_update(connector,
> - drm_edid_legacy_init(&drm_edid, edid));
> + return _drm_edid_connector_update(connector,
> + drm_edid_legacy_init(&drm_edid, edid));
> }
> EXPORT_SYMBOL(drm_add_edid_modes);
>
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index b729e18968dd..289b956d31ea 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -590,6 +590,8 @@ const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,
> const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector,
> int (*read_block)(void *context, u8 *buf, unsigned int block, size_t len),
> void *context);
> +int drm_edid_connector_update(struct drm_connector *connector,
> + const struct drm_edid *edid);
> const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
> int ext_id, int *ext_index);
>
> --
> 2.30.2
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v1 05/13] drm/probe-helper: abstract .get_modes() connector helper call
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
` (3 preceding siblings ...)
2022-05-24 10:39 ` [PATCH v1 04/13] drm/edid: add drm_edid_connector_update() Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-05-24 10:39 ` [PATCH v1 06/13] drm/probe-helper: make .get_modes() optional, add default action Jani Nikula
` (7 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula, David Airlie
Abstract the .get_modes() connector helper call, including the
override/firmware EDID fallback, to make it easier to extend it.
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 425f56280d51..836bcd5b4cb6 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -354,6 +354,24 @@ drm_helper_probe_detect(struct drm_connector *connector,
}
EXPORT_SYMBOL(drm_helper_probe_detect);
+static int drm_helper_probe_get_modes(struct drm_connector *connector)
+{
+ const struct drm_connector_helper_funcs *connector_funcs =
+ connector->helper_private;
+ int count;
+
+ count = connector_funcs->get_modes(connector);
+
+ /*
+ * Fallback for when DDC probe failed in drm_get_edid() and thus skipped
+ * override/firmware EDID.
+ */
+ if (count == 0 && connector->status == connector_status_connected)
+ count = drm_add_override_edid_modes(connector);
+
+ return count;
+}
+
static int __drm_helper_update_and_validate(struct drm_connector *connector,
uint32_t maxX, uint32_t maxY,
struct drm_modeset_acquire_ctx *ctx)
@@ -473,8 +491,6 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
{
struct drm_device *dev = connector->dev;
struct drm_display_mode *mode;
- const struct drm_connector_helper_funcs *connector_funcs =
- connector->helper_private;
int count = 0, ret;
enum drm_connector_status old_status;
struct drm_modeset_acquire_ctx ctx;
@@ -559,14 +575,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
goto exit;
}
- count = (*connector_funcs->get_modes)(connector);
-
- /*
- * Fallback for when DDC probe failed in drm_get_edid() and thus skipped
- * override/firmware EDID.
- */
- if (count == 0 && connector->status == connector_status_connected)
- count = drm_add_override_edid_modes(connector);
+ count = drm_helper_probe_get_modes(connector);
if (count == 0 && (connector->status == connector_status_connected ||
connector->status == connector_status_unknown))
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v1 06/13] drm/probe-helper: make .get_modes() optional, add default action
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
` (4 preceding siblings ...)
2022-05-24 10:39 ` [PATCH v1 05/13] drm/probe-helper: abstract .get_modes() connector helper call Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-06-02 17:14 ` Ville Syrjälä
2022-05-24 10:39 ` [PATCH v1 07/13] drm/probe-helper: add .get_edid() callback Jani Nikula
` (6 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula, David Airlie
Add default action when .get_modes() not set. This also defines what a
.get_modes() hook should do.
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_probe_helper.c | 14 +++++++++++++-
include/drm/drm_modeset_helper_vtables.h | 4 ++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 836bcd5b4cb6..9df17f0ae225 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -360,7 +360,19 @@ static int drm_helper_probe_get_modes(struct drm_connector *connector)
connector->helper_private;
int count;
- count = connector_funcs->get_modes(connector);
+ if (connector_funcs->get_modes) {
+ count = connector_funcs->get_modes(connector);
+ } else {
+ const struct drm_edid *drm_edid;
+
+ /* Note: This requires connector->ddc is set */
+ drm_edid = drm_edid_read(connector);
+
+ /* Update modes etc. from the EDID */
+ count = drm_edid_connector_update(connector, drm_edid);
+
+ drm_edid_free(drm_edid);
+ }
/*
* Fallback for when DDC probe failed in drm_get_edid() and thus skipped
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index fafa70ac1337..b4402bc64e57 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -894,6 +894,10 @@ struct drm_connector_helper_funcs {
* libraries always call this with the &drm_mode_config.connection_mutex
* held. Because of this it's safe to inspect &drm_connector->state.
*
+ * This callback is optional. By default, it reads the EDID using
+ * drm_edid_read() and updates the connector display info, modes, and
+ * properties using drm_edid_connector_update().
+ *
* RETURNS:
*
* The number of modes added by calling drm_mode_probed_add().
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v1 06/13] drm/probe-helper: make .get_modes() optional, add default action
2022-05-24 10:39 ` [PATCH v1 06/13] drm/probe-helper: make .get_modes() optional, add default action Jani Nikula
@ 2022-06-02 17:14 ` Ville Syrjälä
2022-06-07 11:14 ` Jani Nikula
0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2022-06-02 17:14 UTC (permalink / raw)
To: Jani Nikula; +Cc: David Airlie, intel-gfx, dri-devel
On Tue, May 24, 2022 at 01:39:28PM +0300, Jani Nikula wrote:
> Add default action when .get_modes() not set. This also defines what a
> .get_modes() hook should do.
>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_probe_helper.c | 14 +++++++++++++-
> include/drm/drm_modeset_helper_vtables.h | 4 ++++
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 836bcd5b4cb6..9df17f0ae225 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -360,7 +360,19 @@ static int drm_helper_probe_get_modes(struct drm_connector *connector)
> connector->helper_private;
> int count;
>
> - count = connector_funcs->get_modes(connector);
> + if (connector_funcs->get_modes) {
> + count = connector_funcs->get_modes(connector);
> + } else {
> + const struct drm_edid *drm_edid;
> +
> + /* Note: This requires connector->ddc is set */
> + drm_edid = drm_edid_read(connector);
> +
> + /* Update modes etc. from the EDID */
> + count = drm_edid_connector_update(connector, drm_edid);
> +
> + drm_edid_free(drm_edid);
> + }
Not really a huge fan of this automagic fallback. I think I'd prefer
to keep it mandatory and just provide this as a helper for drivers to
plug into the right spot. Otherwise I'm sure I'll forget how this works
and then I'll be confused when I see a connector withput any apparent
.get_modes() implementation.
>
> /*
> * Fallback for when DDC probe failed in drm_get_edid() and thus skipped
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index fafa70ac1337..b4402bc64e57 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -894,6 +894,10 @@ struct drm_connector_helper_funcs {
> * libraries always call this with the &drm_mode_config.connection_mutex
> * held. Because of this it's safe to inspect &drm_connector->state.
> *
> + * This callback is optional. By default, it reads the EDID using
> + * drm_edid_read() and updates the connector display info, modes, and
> + * properties using drm_edid_connector_update().
> + *
> * RETURNS:
> *
> * The number of modes added by calling drm_mode_probed_add().
> --
> 2.30.2
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v1 06/13] drm/probe-helper: make .get_modes() optional, add default action
2022-06-02 17:14 ` Ville Syrjälä
@ 2022-06-07 11:14 ` Jani Nikula
2022-06-07 19:43 ` Ville Syrjälä
0 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2022-06-07 11:14 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: David Airlie, intel-gfx, dri-devel
On Thu, 02 Jun 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, May 24, 2022 at 01:39:28PM +0300, Jani Nikula wrote:
>> Add default action when .get_modes() not set. This also defines what a
>> .get_modes() hook should do.
>>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/drm_probe_helper.c | 14 +++++++++++++-
>> include/drm/drm_modeset_helper_vtables.h | 4 ++++
>> 2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
>> index 836bcd5b4cb6..9df17f0ae225 100644
>> --- a/drivers/gpu/drm/drm_probe_helper.c
>> +++ b/drivers/gpu/drm/drm_probe_helper.c
>> @@ -360,7 +360,19 @@ static int drm_helper_probe_get_modes(struct drm_connector *connector)
>> connector->helper_private;
>> int count;
>>
>> - count = connector_funcs->get_modes(connector);
>> + if (connector_funcs->get_modes) {
>> + count = connector_funcs->get_modes(connector);
>> + } else {
>> + const struct drm_edid *drm_edid;
>> +
>> + /* Note: This requires connector->ddc is set */
>> + drm_edid = drm_edid_read(connector);
>> +
>> + /* Update modes etc. from the EDID */
>> + count = drm_edid_connector_update(connector, drm_edid);
>> +
>> + drm_edid_free(drm_edid);
>> + }
>
> Not really a huge fan of this automagic fallback. I think I'd prefer
> to keep it mandatory and just provide this as a helper for drivers to
> plug into the right spot. Otherwise I'm sure I'll forget how this works
> and then I'll be confused when I see a connector withput any apparent
> .get_modes() implementation.
Fair enough.
I'm not sure how useful that is going to be, though, at least not for
i915. If we add a .get_edid hook, where would you bolt that? It doesn't
feel right to set a .get_modes hook to a default function that goes on
to call a .get_edid hook. Or what do you think?
BR,
Jani.
>
>>
>> /*
>> * Fallback for when DDC probe failed in drm_get_edid() and thus skipped
>> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
>> index fafa70ac1337..b4402bc64e57 100644
>> --- a/include/drm/drm_modeset_helper_vtables.h
>> +++ b/include/drm/drm_modeset_helper_vtables.h
>> @@ -894,6 +894,10 @@ struct drm_connector_helper_funcs {
>> * libraries always call this with the &drm_mode_config.connection_mutex
>> * held. Because of this it's safe to inspect &drm_connector->state.
>> *
>> + * This callback is optional. By default, it reads the EDID using
>> + * drm_edid_read() and updates the connector display info, modes, and
>> + * properties using drm_edid_connector_update().
>> + *
>> * RETURNS:
>> *
>> * The number of modes added by calling drm_mode_probed_add().
>> --
>> 2.30.2
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v1 06/13] drm/probe-helper: make .get_modes() optional, add default action
2022-06-07 11:14 ` Jani Nikula
@ 2022-06-07 19:43 ` Ville Syrjälä
0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2022-06-07 19:43 UTC (permalink / raw)
To: Jani Nikula; +Cc: David Airlie, intel-gfx, dri-devel
On Tue, Jun 07, 2022 at 02:14:54PM +0300, Jani Nikula wrote:
> On Thu, 02 Jun 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Tue, May 24, 2022 at 01:39:28PM +0300, Jani Nikula wrote:
> >> Add default action when .get_modes() not set. This also defines what a
> >> .get_modes() hook should do.
> >>
> >> Cc: David Airlie <airlied@linux.ie>
> >> Cc: Daniel Vetter <daniel@ffwll.ch>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >> drivers/gpu/drm/drm_probe_helper.c | 14 +++++++++++++-
> >> include/drm/drm_modeset_helper_vtables.h | 4 ++++
> >> 2 files changed, 17 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> >> index 836bcd5b4cb6..9df17f0ae225 100644
> >> --- a/drivers/gpu/drm/drm_probe_helper.c
> >> +++ b/drivers/gpu/drm/drm_probe_helper.c
> >> @@ -360,7 +360,19 @@ static int drm_helper_probe_get_modes(struct drm_connector *connector)
> >> connector->helper_private;
> >> int count;
> >>
> >> - count = connector_funcs->get_modes(connector);
> >> + if (connector_funcs->get_modes) {
> >> + count = connector_funcs->get_modes(connector);
> >> + } else {
> >> + const struct drm_edid *drm_edid;
> >> +
> >> + /* Note: This requires connector->ddc is set */
> >> + drm_edid = drm_edid_read(connector);
> >> +
> >> + /* Update modes etc. from the EDID */
> >> + count = drm_edid_connector_update(connector, drm_edid);
> >> +
> >> + drm_edid_free(drm_edid);
> >> + }
> >
> > Not really a huge fan of this automagic fallback. I think I'd prefer
> > to keep it mandatory and just provide this as a helper for drivers to
> > plug into the right spot. Otherwise I'm sure I'll forget how this works
> > and then I'll be confused when I see a connector withput any apparent
> > .get_modes() implementation.
>
> Fair enough.
>
> I'm not sure how useful that is going to be, though, at least not for
> i915. If we add a .get_edid hook, where would you bolt that? It doesn't
> feel right to set a .get_modes hook to a default function that goes on
> to call a .get_edid hook. Or what do you think?
If .get_modes() remains mandatory do we need the .get_edid() hook?
Ie. would it be called from anywhere else than from .get_modes()?
So we'd just end with
foo_get_modes()
{
edid = foo_get_edid();
drm_edid_connector_update(edid);
}
in drivers than need a custom edid read function.
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v1 07/13] drm/probe-helper: add .get_edid() callback
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
` (5 preceding siblings ...)
2022-05-24 10:39 ` [PATCH v1 06/13] drm/probe-helper: make .get_modes() optional, add default action Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-05-24 10:39 ` [PATCH v1 08/13] drm/edid: add drm_edid_raw() to access the raw EDID data Jani Nikula
` (5 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula, David Airlie
Add a hook for custom .get_edid() when .get_modes() is not set.
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_probe_helper.c | 11 +++++++++--
include/drm/drm_modeset_helper_vtables.h | 21 ++++++++++++++++++---
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 9df17f0ae225..42481dc9e6db 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -361,12 +361,19 @@ static int drm_helper_probe_get_modes(struct drm_connector *connector)
int count;
if (connector_funcs->get_modes) {
+ /* No point in having both set */
+ drm_WARN_ON_ONCE(connector->dev, connector_funcs->get_edid);
+
count = connector_funcs->get_modes(connector);
} else {
const struct drm_edid *drm_edid;
- /* Note: This requires connector->ddc is set */
- drm_edid = drm_edid_read(connector);
+ if (connector_funcs->get_edid) {
+ drm_edid = connector_funcs->get_edid(connector);
+ } else {
+ /* Note: This requires connector->ddc is set */
+ drm_edid = drm_edid_read(connector);
+ }
/* Update modes etc. from the EDID */
count = drm_edid_connector_update(connector, drm_edid);
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index b4402bc64e57..f4defbdf1768 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -49,6 +49,7 @@
*/
enum mode_set_atomic;
+struct drm_edid;
struct drm_writeback_connector;
struct drm_writeback_job;
@@ -894,9 +895,10 @@ struct drm_connector_helper_funcs {
* libraries always call this with the &drm_mode_config.connection_mutex
* held. Because of this it's safe to inspect &drm_connector->state.
*
- * This callback is optional. By default, it reads the EDID using
- * drm_edid_read() and updates the connector display info, modes, and
- * properties using drm_edid_connector_update().
+ * This callback is optional. By default, it reads the EDID using the
+ * .get_edid() callback if set, drm_edid_read() otherwise, and updates
+ * the connector display info, modes, and properties using
+ * drm_edid_connector_update().
*
* RETURNS:
*
@@ -904,6 +906,19 @@ struct drm_connector_helper_funcs {
*/
int (*get_modes)(struct drm_connector *connector);
+ /**
+ * @get_edid:
+ *
+ * If the get_modes() callback is not set, this function gets called to
+ * retrieve the EDID. This callback is optional. By default,
+ * drm_edid_read() is used.
+ *
+ * This function must return a copy of the EDID; the returned pointer
+ * will be freed using drm_edid_free(). Usually it would be a copy of a
+ * previously cached EDID.
+ */
+ const struct drm_edid *(*get_edid)(struct drm_connector *connector);
+
/**
* @detect_ctx:
*
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v1 08/13] drm/edid: add drm_edid_raw() to access the raw EDID data
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
` (6 preceding siblings ...)
2022-05-24 10:39 ` [PATCH v1 07/13] drm/probe-helper: add .get_edid() callback Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-05-24 10:39 ` [PATCH v1 09/13] drm/i915/edid: convert DP, HDMI and LVDS to drm_edid Jani Nikula
` (4 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula
Unfortunately, there are still plenty of interfaces around that require
a struct edid pointer, and it's impossible to change them all at
once. Add an accessor to the raw EDID data to help the transition.
While there are no such cases now, be defensive against raw EDID
extension count indicating bigger EDID than is actually allocated.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_edid.c | 26 ++++++++++++++++++++++++++
include/drm/drm_edid.h | 1 +
2 files changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 09328298aabf..6617ea70ae5c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2333,6 +2333,32 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
}
EXPORT_SYMBOL_GPL(drm_do_get_edid);
+/**
+ * drm_edid_raw - Get a pointer to the raw EDID data.
+ * @drm_edid: drm_edid container
+ *
+ * Get a pointer to the raw EDID data.
+ *
+ * This is for transition only. Avoid using this like the plague.
+ *
+ * Return: Pointer to raw EDID data.
+ */
+const struct edid *drm_edid_raw(const struct drm_edid *drm_edid)
+{
+ if (!drm_edid || !drm_edid->size)
+ return NULL;
+
+ /*
+ * Do not return pointers where relying on EDID extension count would
+ * lead to buffer overflow.
+ */
+ if (WARN_ON(edid_size(drm_edid->edid) > drm_edid->size))
+ return NULL;
+
+ return drm_edid->edid;
+}
+EXPORT_SYMBOL(drm_edid_raw);
+
/* Allocate struct drm_edid container *without* duplicating the edid data */
static const struct drm_edid *_drm_edid_alloc(const void *edid, size_t size)
{
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 289b956d31ea..f74ec9f7439b 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -584,6 +584,7 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
const struct drm_edid *drm_edid_alloc(const void *edid, size_t size);
const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid);
void drm_edid_free(const struct drm_edid *drm_edid);
+const struct edid *drm_edid_raw(const struct drm_edid *drm_edid);
const struct drm_edid *drm_edid_read(struct drm_connector *connector);
const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,
struct i2c_adapter *adapter);
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v1 09/13] drm/i915/edid: convert DP, HDMI and LVDS to drm_edid
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
` (7 preceding siblings ...)
2022-05-24 10:39 ` [PATCH v1 08/13] drm/edid: add drm_edid_raw() to access the raw EDID data Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-05-24 10:39 ` [PATCH v1 10/13] drm/edid: do invalid block filtering in-place Jani Nikula
` (3 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula
Convert all the connectors that use cached connector edid and
detect_edid to drm_edid.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
.../gpu/drm/i915/display/intel_connector.c | 4 +-
.../drm/i915/display/intel_display_types.h | 4 +-
drivers/gpu/drm/i915/display/intel_dp.c | 72 ++++++++++---------
drivers/gpu/drm/i915/display/intel_hdmi.c | 26 +++----
drivers/gpu/drm/i915/display/intel_lvds.c | 35 ++++-----
5 files changed, 77 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
index 1dcc268927a2..d83b2a64f618 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -95,12 +95,12 @@ void intel_connector_destroy(struct drm_connector *connector)
{
struct intel_connector *intel_connector = to_intel_connector(connector);
- kfree(intel_connector->detect_edid);
+ drm_edid_free(intel_connector->detect_edid);
intel_hdcp_cleanup(intel_connector);
if (!IS_ERR_OR_NULL(intel_connector->edid))
- kfree(intel_connector->edid);
+ drm_edid_free(intel_connector->edid);
intel_panel_fini(intel_connector);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 408152f9f46a..b72724918761 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -519,8 +519,8 @@ struct intel_connector {
struct intel_panel panel;
/* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
- struct edid *edid;
- struct edid *detect_edid;
+ const struct drm_edid *edid;
+ const struct drm_edid *detect_edid;
/* Number of times hotplug detection was tried after an HPD interrupt */
int hotplug_retries;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index e4a79c11fd25..771857ed0052 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3511,12 +3511,11 @@ static u8 intel_dp_autotest_edid(struct intel_dp *intel_dp)
intel_dp->aux.i2c_defer_count);
intel_dp->compliance.test_data.edid = INTEL_DP_RESOLUTION_FAILSAFE;
} else {
- struct edid *block = intel_connector->detect_edid;
+ /* FIXME: Get rid of drm_edid_raw() */
+ const struct edid *block = drm_edid_raw(intel_connector->detect_edid);
- /* We have to write the checksum
- * of the last block read
- */
- block += intel_connector->detect_edid->extensions;
+ /* We have to write the checksum of the last block read */
+ block += block->extensions;
if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_EDID_CHECKSUM,
block->checksum) <= 0)
@@ -4395,7 +4394,7 @@ bool intel_digital_port_connected(struct intel_encoder *encoder)
return is_connected;
}
-static struct edid *
+static const struct drm_edid *
intel_dp_get_edid(struct intel_dp *intel_dp)
{
struct intel_connector *intel_connector = intel_dp->attached_connector;
@@ -4406,18 +4405,22 @@ intel_dp_get_edid(struct intel_dp *intel_dp)
if (IS_ERR(intel_connector->edid))
return NULL;
- return drm_edid_duplicate(intel_connector->edid);
+ return drm_edid_dup(intel_connector->edid);
} else
- return drm_get_edid(&intel_connector->base,
- &intel_dp->aux.ddc);
+ return drm_edid_read_ddc(&intel_connector->base,
+ &intel_dp->aux.ddc);
}
static void
intel_dp_update_dfp(struct intel_dp *intel_dp,
- const struct edid *edid)
+ const struct drm_edid *drm_edid)
{
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
struct intel_connector *connector = intel_dp->attached_connector;
+ const struct edid *edid;
+
+ /* FIXME: Get rid of drm_edid_raw() */
+ edid = drm_edid_raw(drm_edid);
intel_dp->dfp.max_bpc =
drm_dp_downstream_max_bpc(intel_dp->dpcd,
@@ -4517,21 +4520,24 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
{
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
struct intel_connector *connector = intel_dp->attached_connector;
- struct edid *edid;
+ const struct drm_edid *drm_edid;
+ const struct edid *edid;
bool vrr_capable;
intel_dp_unset_edid(intel_dp);
- edid = intel_dp_get_edid(intel_dp);
- connector->detect_edid = edid;
+ drm_edid = intel_dp_get_edid(intel_dp);
+ connector->detect_edid = drm_edid;
vrr_capable = intel_vrr_is_capable(&connector->base);
drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
connector->base.base.id, connector->base.name, str_yes_no(vrr_capable));
drm_connector_set_vrr_capable_property(&connector->base, vrr_capable);
- intel_dp_update_dfp(intel_dp, edid);
+ intel_dp_update_dfp(intel_dp, drm_edid);
intel_dp_update_420(intel_dp);
+ /* FIXME: Get rid of drm_edid_raw() */
+ edid = drm_edid_raw(drm_edid);
if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
intel_dp->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
intel_dp->has_audio = drm_detect_monitor_audio(edid);
@@ -4546,7 +4552,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
struct intel_connector *connector = intel_dp->attached_connector;
drm_dp_cec_unset_edid(&intel_dp->aux);
- kfree(connector->detect_edid);
+ drm_edid_free(connector->detect_edid);
connector->detect_edid = NULL;
intel_dp->has_hdmi_sink = false;
@@ -4710,12 +4716,11 @@ intel_dp_force(struct drm_connector *connector)
static int intel_dp_get_modes(struct drm_connector *connector)
{
struct intel_connector *intel_connector = to_intel_connector(connector);
- struct edid *edid;
+ const struct drm_edid *drm_edid;
int num_modes = 0;
- edid = intel_connector->detect_edid;
- if (edid)
- num_modes = intel_connector_update_modes(connector, edid);
+ drm_edid = intel_connector->detect_edid;
+ num_modes = drm_edid_connector_update(connector, drm_edid);
/* Also add fixed mode, which may or may not be present in EDID */
if (intel_dp_is_edp(intel_attached_dp(intel_connector)))
@@ -4724,7 +4729,7 @@ static int intel_dp_get_modes(struct drm_connector *connector)
if (num_modes)
return num_modes;
- if (!edid) {
+ if (!drm_edid) {
struct intel_dp *intel_dp = intel_attached_dp(intel_connector);
struct drm_display_mode *mode;
@@ -5131,7 +5136,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
struct drm_display_mode *fixed_mode;
bool has_dpcd;
enum pipe pipe = INVALID_PIPE;
- struct edid *edid;
+ const struct drm_edid *drm_edid;
if (!intel_dp_is_edp(intel_dp))
return true;
@@ -5164,26 +5169,29 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
}
mutex_lock(&dev->mode_config.mutex);
- edid = drm_get_edid(connector, &intel_dp->aux.ddc);
- if (!edid) {
+ drm_edid = drm_edid_read_ddc(connector, &intel_dp->aux.ddc);
+ if (!drm_edid) {
+ const struct edid *edid;
+
/* Fallback to EDID from ACPI OpRegion, if any */
+ /* FIXME: Make intel_opregion_get_edid() return drm_edid */
edid = intel_opregion_get_edid(intel_connector);
- if (edid)
+ if (edid) {
+ drm_edid = drm_edid_alloc(edid, (edid->extensions + 1) * EDID_LENGTH);
drm_dbg_kms(&dev_priv->drm,
"[CONNECTOR:%d:%s] Using OpRegion EDID\n",
connector->base.id, connector->name);
+ }
}
- if (edid) {
- if (drm_add_edid_modes(connector, edid)) {
- drm_connector_update_edid_property(connector, edid);
- } else {
- kfree(edid);
- edid = ERR_PTR(-EINVAL);
+ if (drm_edid) {
+ if (!drm_edid_connector_update(connector, drm_edid)) {
+ drm_edid_free(drm_edid);
+ drm_edid = ERR_PTR(-EINVAL);
}
} else {
- edid = ERR_PTR(-ENOENT);
+ drm_edid = ERR_PTR(-ENOENT);
}
- intel_connector->edid = edid;
+ intel_connector->edid = drm_edid;
intel_panel_add_edid_fixed_modes(intel_connector,
dev_priv->vbt.drrs_type != DRRS_TYPE_NONE);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 1ae09431f53a..db33a0ccd7ee 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2340,7 +2340,7 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE;
intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
- kfree(to_intel_connector(connector)->detect_edid);
+ drm_edid_free(to_intel_connector(connector)->detect_edid);
to_intel_connector(connector)->detect_edid = NULL;
}
@@ -2407,7 +2407,8 @@ intel_hdmi_set_edid(struct drm_connector *connector)
struct drm_i915_private *dev_priv = to_i915(connector->dev);
struct intel_hdmi *intel_hdmi = intel_attached_hdmi(to_intel_connector(connector));
intel_wakeref_t wakeref;
- struct edid *edid;
+ const struct drm_edid *drm_edid;
+ const struct edid *edid;
bool connected = false;
struct i2c_adapter *i2c;
@@ -2415,21 +2416,24 @@ intel_hdmi_set_edid(struct drm_connector *connector)
i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
- edid = drm_get_edid(connector, i2c);
+ drm_edid = drm_edid_read_ddc(connector, i2c);
- if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
+ if (!drm_edid && !intel_gmbus_is_forced_bit(i2c)) {
drm_dbg_kms(&dev_priv->drm,
"HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
intel_gmbus_force_bit(i2c, true);
- edid = drm_get_edid(connector, i2c);
+ drm_edid = drm_edid_read_ddc(connector, i2c);
intel_gmbus_force_bit(i2c, false);
}
- intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
+ intel_hdmi_dp_dual_mode_detect(connector, drm_edid != NULL);
intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
- to_intel_connector(connector)->detect_edid = edid;
+ to_intel_connector(connector)->detect_edid = drm_edid;
+
+ /* FIXME: Get rid of drm_edid_raw() */
+ edid = drm_edid_raw(drm_edid);
if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
@@ -2501,13 +2505,11 @@ intel_hdmi_force(struct drm_connector *connector)
static int intel_hdmi_get_modes(struct drm_connector *connector)
{
- struct edid *edid;
+ const struct drm_edid *drm_edid;
- edid = to_intel_connector(connector)->detect_edid;
- if (edid == NULL)
- return 0;
+ drm_edid = to_intel_connector(connector)->detect_edid;
- return intel_connector_update_modes(connector, edid);
+ return drm_edid_connector_update(connector, drm_edid);
}
static struct i2c_adapter *
diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
index e8478161f8b9..58070f8858e6 100644
--- a/drivers/gpu/drm/i915/display/intel_lvds.c
+++ b/drivers/gpu/drm/i915/display/intel_lvds.c
@@ -479,7 +479,7 @@ static int intel_lvds_get_modes(struct drm_connector *connector)
/* use cached edid if we have one */
if (!IS_ERR_OR_NULL(intel_connector->edid))
- return drm_add_edid_modes(connector, intel_connector->edid);
+ return drm_edid_connector_update(connector, intel_connector->edid);
return intel_panel_get_modes(intel_connector);
}
@@ -829,7 +829,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
struct intel_connector *intel_connector;
struct drm_connector *connector;
struct drm_encoder *encoder;
- struct edid *edid;
+ const struct drm_edid *drm_edid;
i915_reg_t lvds_reg;
u32 lvds;
u8 pin;
@@ -948,24 +948,27 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
* preferred mode is the right one.
*/
mutex_lock(&dev->mode_config.mutex);
- if (vga_switcheroo_handler_flags() & VGA_SWITCHEROO_CAN_SWITCH_DDC)
+ if (vga_switcheroo_handler_flags() & VGA_SWITCHEROO_CAN_SWITCH_DDC) {
+ const struct edid *edid;
+
+ /* FIXME: Make drm_get_edid_switcheroo() return drm_edid */
edid = drm_get_edid_switcheroo(connector,
- intel_gmbus_get_adapter(dev_priv, pin));
- else
- edid = drm_get_edid(connector,
- intel_gmbus_get_adapter(dev_priv, pin));
- if (edid) {
- if (drm_add_edid_modes(connector, edid)) {
- drm_connector_update_edid_property(connector,
- edid);
- } else {
- kfree(edid);
- edid = ERR_PTR(-EINVAL);
+ intel_gmbus_get_adapter(dev_priv, pin));
+ if (edid)
+ drm_edid = drm_edid_alloc(edid, (edid->extensions + 1) * EDID_LENGTH);
+ } else {
+ drm_edid = drm_edid_read_ddc(connector,
+ intel_gmbus_get_adapter(dev_priv, pin));
+ }
+ if (drm_edid) {
+ if (!drm_edid_connector_update(connector, drm_edid)) {
+ drm_edid_free(drm_edid);
+ drm_edid = ERR_PTR(-EINVAL);
}
} else {
- edid = ERR_PTR(-ENOENT);
+ drm_edid = ERR_PTR(-ENOENT);
}
- intel_connector->edid = edid;
+ intel_connector->edid = drm_edid;
/* Try EDID first */
intel_panel_add_edid_fixed_modes(intel_connector,
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v1 10/13] drm/edid: do invalid block filtering in-place
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
` (8 preceding siblings ...)
2022-05-24 10:39 ` [PATCH v1 09/13] drm/i915/edid: convert DP, HDMI and LVDS to drm_edid Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-05-24 10:39 ` [PATCH v1 11/13] drm/edid: add HF-EEODB support to EDID read and allocation Jani Nikula
` (2 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula
Rewrite edid_filter_invalid_blocks() to filter invalid blocks
in-place. The main motivation is to not rely on passed in information on
invalid block count or the allocation size, which will be helpful in
follow-up work on HF-EEODB.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_edid.c | 43 ++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 6617ea70ae5c..5e0a91da565e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2020,33 +2020,37 @@ bool drm_edid_is_valid(struct edid *edid)
}
EXPORT_SYMBOL(drm_edid_is_valid);
-static struct edid *edid_filter_invalid_blocks(const struct edid *edid,
- int invalid_blocks,
+static struct edid *edid_filter_invalid_blocks(struct edid *edid,
size_t *alloc_size)
{
- struct edid *new, *dest_block;
- int valid_extensions = edid->extensions - invalid_blocks;
- int i;
+ struct edid *new;
+ int i, valid_blocks = 0;
- *alloc_size = edid_size_by_blocks(valid_extensions + 1);
+ for (i = 0; i < edid_block_count(edid); i++) {
+ const void *src_block = edid_block_data(edid, i);
- new = kmalloc(*alloc_size, GFP_KERNEL);
- if (!new)
- goto out;
+ if (edid_block_valid(src_block, i == 0)) {
+ void *dst_block = (void *)edid_block_data(edid, valid_blocks);
- dest_block = new;
- for (i = 0; i < edid_block_count(edid); i++) {
- const void *block = edid_block_data(edid, i);
+ memmove(dst_block, src_block, EDID_LENGTH);
+ valid_blocks++;
+ }
+ }
- if (edid_block_valid(block, i == 0))
- memcpy(dest_block++, block, EDID_LENGTH);
+ /* We already trusted the base block to be valid here... */
+ if (WARN_ON(!valid_blocks)) {
+ kfree(edid);
+ return NULL;
}
- new->extensions = valid_extensions;
- new->checksum = edid_block_compute_checksum(new);
+ edid->extensions = valid_blocks - 1;
+ edid->checksum = edid_block_compute_checksum(edid);
-out:
- kfree(edid);
+ *alloc_size = edid_size_by_blocks(valid_blocks);
+
+ new = krealloc(edid, *alloc_size, GFP_KERNEL);
+ if (!new)
+ kfree(edid);
return new;
}
@@ -2290,8 +2294,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
if (invalid_blocks) {
connector_bad_edid(connector, edid, edid_block_count(edid));
- edid = edid_filter_invalid_blocks(edid, invalid_blocks,
- &alloc_size);
+ edid = edid_filter_invalid_blocks(edid, &alloc_size);
}
ok:
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v1 11/13] drm/edid: add HF-EEODB support to EDID read and allocation
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
` (9 preceding siblings ...)
2022-05-24 10:39 ` [PATCH v1 10/13] drm/edid: do invalid block filtering in-place Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-06-03 17:13 ` Ville Syrjälä
2022-05-24 10:39 ` [PATCH v1 12/13] drm/edid: take HF-EEODB extension count into account Jani Nikula
2022-05-24 10:39 ` [PATCH v1 13/13] drm/todo: add entry for converting the subsystem to struct drm_edid Jani Nikula
12 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula
HDMI 2.1 section 10.3.6 defines an HDMI Forum EDID Extension Override
Data Block, which may contain a different extension count than the base
block claims. Add support for reading more EDID data if available. The
extra blocks aren't parsed yet, though.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_edid.c | 81 ++++++++++++++++++++++++++++++++++++--
1 file changed, 78 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5e0a91da565e..ba0c880dc133 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1581,6 +1581,15 @@ static bool version_greater(const struct drm_edid *drm_edid,
(edid->version == version && edid->revision > revision);
}
+static int edid_hfeeodb_extension_block_count(const struct edid *edid);
+
+static int edid_hfeeodb_block_count(const struct edid *edid)
+{
+ int eeodb = edid_hfeeodb_extension_block_count(edid);
+
+ return eeodb ? eeodb + 1 : 0;
+}
+
static int edid_extension_block_count(const struct edid *edid)
{
return edid->extensions;
@@ -2026,6 +2035,11 @@ static struct edid *edid_filter_invalid_blocks(struct edid *edid,
struct edid *new;
int i, valid_blocks = 0;
+ /*
+ * Note: If the EDID uses HF-EEODB, but has invalid blocks, we'll revert
+ * back to regular extension count here. We don't want to start
+ * modifying the HF-EEODB extension too.
+ */
for (i = 0; i < edid_block_count(edid); i++) {
const void *src_block = edid_block_data(edid, i);
@@ -2235,7 +2249,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
size_t *size)
{
enum edid_block_status status;
- int i, invalid_blocks = 0;
+ int i, num_blocks, invalid_blocks = 0;
struct edid *edid, *new;
size_t alloc_size = EDID_LENGTH;
@@ -2277,7 +2291,8 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
goto fail;
edid = new;
- for (i = 1; i < edid_block_count(edid); i++) {
+ num_blocks = edid_block_count(edid);
+ for (i = 1; i < num_blocks; i++) {
void *block = (void *)edid_block_data(edid, i);
status = edid_block_read(block, i, read_block, context);
@@ -2288,11 +2303,31 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
if (status == EDID_BLOCK_READ_FAIL)
goto fail;
invalid_blocks++;
+ } else if (i == 1) {
+ /*
+ * If the first EDID extension is a CTA extension, and
+ * the first Data Block is HF-EEODB, override the
+ * extension block count.
+ *
+ * Note: HF-EEODB could specify a smaller extension
+ * count too, but we can't risk allocating a smaller
+ * amount.
+ */
+ int eeodb = edid_hfeeodb_block_count(edid);
+
+ if (eeodb > num_blocks) {
+ num_blocks = eeodb;
+ alloc_size = edid_size_by_blocks(num_blocks);
+ new = krealloc(edid, alloc_size, GFP_KERNEL);
+ if (!new)
+ goto fail;
+ edid = new;
+ }
}
}
if (invalid_blocks) {
- connector_bad_edid(connector, edid, edid_block_count(edid));
+ connector_bad_edid(connector, edid, num_blocks);
edid = edid_filter_invalid_blocks(edid, &alloc_size);
}
@@ -3825,6 +3860,7 @@ static int add_detailed_modes(struct drm_connector *connector,
#define CTA_EXT_DB_HDR_STATIC_METADATA 6
#define CTA_EXT_DB_420_VIDEO_DATA 14
#define CTA_EXT_DB_420_VIDEO_CAP_MAP 15
+#define CTA_EXT_DB_HF_EEODB 0x78
#define CTA_EXT_DB_HF_SCDB 0x79
#define EDID_BASIC_AUDIO (1 << 6)
@@ -4868,6 +4904,12 @@ static bool cea_db_is_hdmi_forum_vsdb(const struct cea_db *db)
cea_db_payload_len(db) >= 7;
}
+static bool cea_db_is_hdmi_forum_eeodb(const void *db)
+{
+ return cea_db_is_extended_tag(db, CTA_EXT_DB_HF_EEODB) &&
+ cea_db_payload_len(db) >= 2;
+}
+
static bool cea_db_is_microsoft_vsdb(const struct cea_db *db)
{
return cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) &&
@@ -4902,6 +4944,39 @@ static bool cea_db_is_hdmi_hdr_metadata_block(const struct cea_db *db)
cea_db_payload_len(db) >= 3;
}
+/*
+ * Get the HF-EEODB override extension block count from EDID.
+ *
+ * The passed in EDID may be partially read, as long as it has at least two
+ * blocks (base block and one extension block) if EDID extension count is > 0.
+ *
+ * References:
+ * - HDMI 2.1 section 10.3.6 HDMI Forum EDID Extension Override Data Block
+ */
+static int edid_hfeeodb_extension_block_count(const struct edid *edid)
+{
+ const u8 *cta;
+
+ /* No extensions according to base block, no HF-EEODB. */
+ if (!edid_extension_block_count(edid))
+ return 0;
+
+ /* HF-EEODB is always in the first EDID extension block only */
+ cta = edid_extension_block_data(edid, 0);
+ if (edid_block_tag(cta) != CEA_EXT || cea_revision(cta) < 3)
+ return 0;
+
+ /*
+ * Sinks that include the HF-EEODB in their E-EDID shall include one and
+ * only one instance of the HF-EEODB in the E-EDID, occupying bytes 4
+ * through 6 of Block 1 of the E-EDID.
+ */
+ if (!cea_db_is_hdmi_forum_eeodb(&cta[4]))
+ return 0;
+
+ return cta[4 + 2];
+}
+
static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector,
const u8 *db)
{
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v1 11/13] drm/edid: add HF-EEODB support to EDID read and allocation
2022-05-24 10:39 ` [PATCH v1 11/13] drm/edid: add HF-EEODB support to EDID read and allocation Jani Nikula
@ 2022-06-03 17:13 ` Ville Syrjälä
2022-06-07 11:26 ` Jani Nikula
0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2022-06-03 17:13 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, dri-devel
On Tue, May 24, 2022 at 01:39:33PM +0300, Jani Nikula wrote:
> HDMI 2.1 section 10.3.6 defines an HDMI Forum EDID Extension Override
> Data Block, which may contain a different extension count than the base
> block claims. Add support for reading more EDID data if available. The
> extra blocks aren't parsed yet, though.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 81 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 78 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 5e0a91da565e..ba0c880dc133 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1581,6 +1581,15 @@ static bool version_greater(const struct drm_edid *drm_edid,
> (edid->version == version && edid->revision > revision);
> }
>
> +static int edid_hfeeodb_extension_block_count(const struct edid *edid);
> +
> +static int edid_hfeeodb_block_count(const struct edid *edid)
> +{
> + int eeodb = edid_hfeeodb_extension_block_count(edid);
> +
> + return eeodb ? eeodb + 1 : 0;
> +}
> +
> static int edid_extension_block_count(const struct edid *edid)
> {
> return edid->extensions;
> @@ -2026,6 +2035,11 @@ static struct edid *edid_filter_invalid_blocks(struct edid *edid,
> struct edid *new;
> int i, valid_blocks = 0;
>
> + /*
> + * Note: If the EDID uses HF-EEODB, but has invalid blocks, we'll revert
> + * back to regular extension count here. We don't want to start
> + * modifying the HF-EEODB extension too.
> + */
> for (i = 0; i < edid_block_count(edid); i++) {
> const void *src_block = edid_block_data(edid, i);
>
> @@ -2235,7 +2249,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
> size_t *size)
> {
> enum edid_block_status status;
> - int i, invalid_blocks = 0;
> + int i, num_blocks, invalid_blocks = 0;
> struct edid *edid, *new;
> size_t alloc_size = EDID_LENGTH;
>
> @@ -2277,7 +2291,8 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
> goto fail;
> edid = new;
>
> - for (i = 1; i < edid_block_count(edid); i++) {
> + num_blocks = edid_block_count(edid);
> + for (i = 1; i < num_blocks; i++) {
> void *block = (void *)edid_block_data(edid, i);
>
> status = edid_block_read(block, i, read_block, context);
> @@ -2288,11 +2303,31 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
> if (status == EDID_BLOCK_READ_FAIL)
> goto fail;
> invalid_blocks++;
> + } else if (i == 1) {
> + /*
> + * If the first EDID extension is a CTA extension, and
> + * the first Data Block is HF-EEODB, override the
> + * extension block count.
> + *
> + * Note: HF-EEODB could specify a smaller extension
> + * count too, but we can't risk allocating a smaller
> + * amount.
> + */
> + int eeodb = edid_hfeeodb_block_count(edid);
> +
> + if (eeodb > num_blocks) {
> + num_blocks = eeodb;
> + alloc_size = edid_size_by_blocks(num_blocks);
> + new = krealloc(edid, alloc_size, GFP_KERNEL);
> + if (!new)
> + goto fail;
> + edid = new;
> + }
> }
> }
>
> if (invalid_blocks) {
> - connector_bad_edid(connector, edid, edid_block_count(edid));
> + connector_bad_edid(connector, edid, num_blocks);
>
> edid = edid_filter_invalid_blocks(edid, &alloc_size);
> }
> @@ -3825,6 +3860,7 @@ static int add_detailed_modes(struct drm_connector *connector,
> #define CTA_EXT_DB_HDR_STATIC_METADATA 6
> #define CTA_EXT_DB_420_VIDEO_DATA 14
> #define CTA_EXT_DB_420_VIDEO_CAP_MAP 15
> +#define CTA_EXT_DB_HF_EEODB 0x78
> #define CTA_EXT_DB_HF_SCDB 0x79
>
> #define EDID_BASIC_AUDIO (1 << 6)
> @@ -4868,6 +4904,12 @@ static bool cea_db_is_hdmi_forum_vsdb(const struct cea_db *db)
> cea_db_payload_len(db) >= 7;
> }
>
> +static bool cea_db_is_hdmi_forum_eeodb(const void *db)
> +{
> + return cea_db_is_extended_tag(db, CTA_EXT_DB_HF_EEODB) &&
> + cea_db_payload_len(db) >= 2;
> +}
> +
> static bool cea_db_is_microsoft_vsdb(const struct cea_db *db)
> {
> return cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) &&
> @@ -4902,6 +4944,39 @@ static bool cea_db_is_hdmi_hdr_metadata_block(const struct cea_db *db)
> cea_db_payload_len(db) >= 3;
> }
>
> +/*
> + * Get the HF-EEODB override extension block count from EDID.
> + *
> + * The passed in EDID may be partially read, as long as it has at least two
> + * blocks (base block and one extension block) if EDID extension count is > 0.
> + *
> + * References:
> + * - HDMI 2.1 section 10.3.6 HDMI Forum EDID Extension Override Data Block
> + */
> +static int edid_hfeeodb_extension_block_count(const struct edid *edid)
> +{
> + const u8 *cta;
> +
> + /* No extensions according to base block, no HF-EEODB. */
> + if (!edid_extension_block_count(edid))
> + return 0;
> +
> + /* HF-EEODB is always in the first EDID extension block only */
> + cta = edid_extension_block_data(edid, 0);
> + if (edid_block_tag(cta) != CEA_EXT || cea_revision(cta) < 3)
> + return 0;
> +
> + /*
> + * Sinks that include the HF-EEODB in their E-EDID shall include one and
> + * only one instance of the HF-EEODB in the E-EDID, occupying bytes 4
> + * through 6 of Block 1 of the E-EDID.
> + */
> + if (!cea_db_is_hdmi_forum_eeodb(&cta[4]))
> + return 0;
Still not a big fan of these hardcoded things. Not sure if there's any
easy way to just use the normal iterators at this point when we don't
really know the full composition of the EDID yet. If not then I guess
we'll have to use some hardcoded stuff. What we definitely seem to be
missing here are size checks, for both the whoe data block collection,
and the specific data block payload.
> +
> + return cta[4 + 2];
> +}
> +
> static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector,
> const u8 *db)
> {
> --
> 2.30.2
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v1 11/13] drm/edid: add HF-EEODB support to EDID read and allocation
2022-06-03 17:13 ` Ville Syrjälä
@ 2022-06-07 11:26 ` Jani Nikula
0 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-06-07 11:26 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Fri, 03 Jun 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, May 24, 2022 at 01:39:33PM +0300, Jani Nikula wrote:
>> HDMI 2.1 section 10.3.6 defines an HDMI Forum EDID Extension Override
>> Data Block, which may contain a different extension count than the base
>> block claims. Add support for reading more EDID data if available. The
>> extra blocks aren't parsed yet, though.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/drm_edid.c | 81 ++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 78 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 5e0a91da565e..ba0c880dc133 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -1581,6 +1581,15 @@ static bool version_greater(const struct drm_edid *drm_edid,
>> (edid->version == version && edid->revision > revision);
>> }
>>
>> +static int edid_hfeeodb_extension_block_count(const struct edid *edid);
>> +
>> +static int edid_hfeeodb_block_count(const struct edid *edid)
>> +{
>> + int eeodb = edid_hfeeodb_extension_block_count(edid);
>> +
>> + return eeodb ? eeodb + 1 : 0;
>> +}
>> +
>> static int edid_extension_block_count(const struct edid *edid)
>> {
>> return edid->extensions;
>> @@ -2026,6 +2035,11 @@ static struct edid *edid_filter_invalid_blocks(struct edid *edid,
>> struct edid *new;
>> int i, valid_blocks = 0;
>>
>> + /*
>> + * Note: If the EDID uses HF-EEODB, but has invalid blocks, we'll revert
>> + * back to regular extension count here. We don't want to start
>> + * modifying the HF-EEODB extension too.
>> + */
>> for (i = 0; i < edid_block_count(edid); i++) {
>> const void *src_block = edid_block_data(edid, i);
>>
>> @@ -2235,7 +2249,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
>> size_t *size)
>> {
>> enum edid_block_status status;
>> - int i, invalid_blocks = 0;
>> + int i, num_blocks, invalid_blocks = 0;
>> struct edid *edid, *new;
>> size_t alloc_size = EDID_LENGTH;
>>
>> @@ -2277,7 +2291,8 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
>> goto fail;
>> edid = new;
>>
>> - for (i = 1; i < edid_block_count(edid); i++) {
>> + num_blocks = edid_block_count(edid);
>> + for (i = 1; i < num_blocks; i++) {
>> void *block = (void *)edid_block_data(edid, i);
>>
>> status = edid_block_read(block, i, read_block, context);
>> @@ -2288,11 +2303,31 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector,
>> if (status == EDID_BLOCK_READ_FAIL)
>> goto fail;
>> invalid_blocks++;
>> + } else if (i == 1) {
>> + /*
>> + * If the first EDID extension is a CTA extension, and
>> + * the first Data Block is HF-EEODB, override the
>> + * extension block count.
>> + *
>> + * Note: HF-EEODB could specify a smaller extension
>> + * count too, but we can't risk allocating a smaller
>> + * amount.
>> + */
>> + int eeodb = edid_hfeeodb_block_count(edid);
>> +
>> + if (eeodb > num_blocks) {
>> + num_blocks = eeodb;
>> + alloc_size = edid_size_by_blocks(num_blocks);
>> + new = krealloc(edid, alloc_size, GFP_KERNEL);
>> + if (!new)
>> + goto fail;
>> + edid = new;
>> + }
>> }
>> }
>>
>> if (invalid_blocks) {
>> - connector_bad_edid(connector, edid, edid_block_count(edid));
>> + connector_bad_edid(connector, edid, num_blocks);
>>
>> edid = edid_filter_invalid_blocks(edid, &alloc_size);
>> }
>> @@ -3825,6 +3860,7 @@ static int add_detailed_modes(struct drm_connector *connector,
>> #define CTA_EXT_DB_HDR_STATIC_METADATA 6
>> #define CTA_EXT_DB_420_VIDEO_DATA 14
>> #define CTA_EXT_DB_420_VIDEO_CAP_MAP 15
>> +#define CTA_EXT_DB_HF_EEODB 0x78
>> #define CTA_EXT_DB_HF_SCDB 0x79
>>
>> #define EDID_BASIC_AUDIO (1 << 6)
>> @@ -4868,6 +4904,12 @@ static bool cea_db_is_hdmi_forum_vsdb(const struct cea_db *db)
>> cea_db_payload_len(db) >= 7;
>> }
>>
>> +static bool cea_db_is_hdmi_forum_eeodb(const void *db)
>> +{
>> + return cea_db_is_extended_tag(db, CTA_EXT_DB_HF_EEODB) &&
>> + cea_db_payload_len(db) >= 2;
>> +}
>> +
>> static bool cea_db_is_microsoft_vsdb(const struct cea_db *db)
>> {
>> return cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) &&
>> @@ -4902,6 +4944,39 @@ static bool cea_db_is_hdmi_hdr_metadata_block(const struct cea_db *db)
>> cea_db_payload_len(db) >= 3;
>> }
>>
>> +/*
>> + * Get the HF-EEODB override extension block count from EDID.
>> + *
>> + * The passed in EDID may be partially read, as long as it has at least two
>> + * blocks (base block and one extension block) if EDID extension count is > 0.
>> + *
>> + * References:
>> + * - HDMI 2.1 section 10.3.6 HDMI Forum EDID Extension Override Data Block
>> + */
>> +static int edid_hfeeodb_extension_block_count(const struct edid *edid)
>> +{
>> + const u8 *cta;
>> +
>> + /* No extensions according to base block, no HF-EEODB. */
>> + if (!edid_extension_block_count(edid))
>> + return 0;
>> +
>> + /* HF-EEODB is always in the first EDID extension block only */
>> + cta = edid_extension_block_data(edid, 0);
>> + if (edid_block_tag(cta) != CEA_EXT || cea_revision(cta) < 3)
>> + return 0;
>> +
>> + /*
>> + * Sinks that include the HF-EEODB in their E-EDID shall include one and
>> + * only one instance of the HF-EEODB in the E-EDID, occupying bytes 4
>> + * through 6 of Block 1 of the E-EDID.
>> + */
>> + if (!cea_db_is_hdmi_forum_eeodb(&cta[4]))
>> + return 0;
>
> Still not a big fan of these hardcoded things. Not sure if there's any
> easy way to just use the normal iterators at this point when we don't
> really know the full composition of the EDID yet. If not then I guess
> we'll have to use some hardcoded stuff. What we definitely seem to be
> missing here are size checks, for both the whoe data block collection,
> and the specific data block payload.
I don't like it either, but it's a chicken and egg problem wrt how far
to iterate. Taking this into account in the iterators looks like making
the iterators harder to understand, so I prefer the hardcoded hack here
in one place. And the spec specifically says where this data block must
be.
The data block collection size check is an oversight, but
cea_db_is_hdmi_forum_eeodb() does check for minimum payload length.
BR,
Jani.
>
>> +
>> + return cta[4 + 2];
>> +}
>> +
>> static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector,
>> const u8 *db)
>> {
>> --
>> 2.30.2
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v1 12/13] drm/edid: take HF-EEODB extension count into account
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
` (10 preceding siblings ...)
2022-05-24 10:39 ` [PATCH v1 11/13] drm/edid: add HF-EEODB support to EDID read and allocation Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
2022-05-24 10:39 ` [PATCH v1 13/13] drm/todo: add entry for converting the subsystem to struct drm_edid Jani Nikula
12 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula
Take the HF-EEODB extension count override into account.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/drm_edid.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index ba0c880dc133..6b1284b895eb 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1629,6 +1629,19 @@ static int drm_edid_block_count(const struct drm_edid *drm_edid)
/* Starting point */
num_blocks = edid_block_count(drm_edid->edid);
+ /* HF-EEODB override */
+ if (drm_edid->size >= edid_size_by_blocks(2)) {
+ int eeodb;
+
+ /*
+ * Note: HF-EEODB may specify a smaller extension count than the
+ * regular one. Unlike in buffer allocation, here we can use it.
+ */
+ eeodb = edid_hfeeodb_block_count(drm_edid->edid);
+ if (eeodb)
+ num_blocks = eeodb;
+ }
+
/* Limit by allocated size */
num_blocks = min(num_blocks, (int)drm_edid->size / EDID_LENGTH);
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v1 13/13] drm/todo: add entry for converting the subsystem to struct drm_edid
2022-05-24 10:39 [PATCH v1 00/13] drm/edid: expand on struct drm_edid usage Jani Nikula
` (11 preceding siblings ...)
2022-05-24 10:39 ` [PATCH v1 12/13] drm/edid: take HF-EEODB extension count into account Jani Nikula
@ 2022-05-24 10:39 ` Jani Nikula
12 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2022-05-24 10:39 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: jani.nikula, David Airlie
We need to stop duplicating EDID validation and parsing all over the
subsystem in various broken ways.
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
Documentation/gpu/todo.rst | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 513b20ccef1e..982e4e29825f 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -480,6 +480,31 @@ Contact: Thomas Zimmermann <tzimmermann@suse.de>
Level: Starter
+Convert core and drivers from struct edid to struct drm_edid
+------------------------------------------------------------
+
+Go through all drivers and drm core KMS code to convert all raw struct edid
+usage to the opaque struct drm_edid. See commit e4ccf9a777d3 ("drm/edid: add
+struct drm_edid container") for rationale.
+
+Convert drm_get_edid() and drm_do_get_edid() usage to drm_edid_read(),
+drm_edid_read_ddc(), or drm_edid_read_custom().
+
+Convert drm_add_edid_modes() and drm_connector_update_edid_property() to
+drm_edid_connector_update(). See drm_helper_probe_get_modes() for reference for
+converting the ->get_modes() hooks.
+
+Convert decentralized, direct struct edid parsing to centralized parsing in
+drm_edid.c. Prefer one-time parsing as part of drm_edid_connector_update() and
+storing the result in drm_connector->display_info over adding individual,
+exported parser functions.
+
+During the transition period, it may be necessary to use drm_edid_raw(), but do
+use it sparingly. Eventually, all of them need to go.
+
+Contact: Jani Nikula <jani.nikula@intel.com>
+
+Level: Intermediate
Core refactorings
=================
--
2.30.2
^ permalink raw reply related [flat|nested] 24+ messages in thread