* [PATCH v2] drm/edid: Implement DisplayID Type IX & X timing blocks parsing
@ 2025-02-12 5:05 Egor Vorontsov
2025-02-12 9:35 ` Jani Nikula
2025-02-14 1:05 ` [PATCH v2 2/2] drm/edid: Refactor DisplayID timing block structs Egor Vorontsov
0 siblings, 2 replies; 6+ messages in thread
From: Egor Vorontsov @ 2025-02-12 5:05 UTC (permalink / raw)
To: linux-kernel
Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Jani Nikula, Egor Vorontsov
Some newer high refresh rate consumer monitors (including those by Samsung)
make use of DisplayID 2.1 timing blocks in their EDID data, notably for
their highest refresh rate modes. Such modes won't be available as of now.
Implement partial support for such blocks in order to enable native
support of HRR modes of most such monitors for users without having to rely
on EDID patching/override (or need thereof).
Closes: https://gitlab.freedesktop.org/drm/misc/kernel/-/issues/55
Suggested-by: Maximilian Boße <max@bosse.io>
Signed-off-by: Egor Vorontsov <sdoregor@sdore.me>
---
drivers/gpu/drm/drm_displayid_internal.h | 13 +++++
drivers/gpu/drm/drm_edid.c | 63 ++++++++++++++++++++++++
2 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/drm_displayid_internal.h b/drivers/gpu/drm/drm_displayid_internal.h
index aee1b86a73c1..88220c107822 100644
--- a/drivers/gpu/drm/drm_displayid_internal.h
+++ b/drivers/gpu/drm/drm_displayid_internal.h
@@ -66,6 +66,7 @@ struct drm_edid;
#define DATA_BLOCK_2_STEREO_DISPLAY_INTERFACE 0x27
#define DATA_BLOCK_2_TILED_DISPLAY_TOPOLOGY 0x28
#define DATA_BLOCK_2_CONTAINER_ID 0x29
+#define DATA_BLOCK_2_TYPE_10_FORMULA_TIMING 0x2a
#define DATA_BLOCK_2_VENDOR_SPECIFIC 0x7e
#define DATA_BLOCK_2_CTA_DISPLAY_ID 0x81
@@ -129,6 +130,18 @@ struct displayid_detailed_timing_block {
struct displayid_detailed_timings_1 timings[];
};
+struct displayid_formula_timings_9 {
+ u8 flags;
+ __be16 hactive;
+ __be16 vactive;
+ u8 vrefresh;
+} __packed;
+
+struct displayid_formula_timing_block {
+ struct displayid_block base;
+ struct displayid_formula_timings_9 timings[];
+} __packed;
+
#define DISPLAYID_VESA_MSO_OVERLAP GENMASK(3, 0)
#define DISPLAYID_VESA_MSO_MODE GENMASK(6, 5)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 13bc4c290b17..9c363df5af9a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6833,6 +6833,66 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector,
return num_modes;
}
+static struct drm_display_mode *drm_mode_displayid_formula(struct drm_device *dev,
+ const struct displayid_formula_timings_9 *timings,
+ bool type_10)
+{
+ struct drm_display_mode *mode;
+ u16 hactive = be16_to_cpu(timings->hactive) + 1;
+ u16 vactive = be16_to_cpu(timings->vactive) + 1;
+ u8 timing_formula = timings->flags & 0x7;
+
+ /* TODO: support RB-v2 & RB-v3 */
+ if (timing_formula > 1)
+ return NULL;
+
+ /* TODO: support video-optimized refresh rate */
+ if (timings->flags & (1 << 4))
+ return NULL;
+
+ mode = drm_cvt_mode(dev, hactive, vactive, timings->vrefresh + 1, timing_formula == 1, false, false);
+ if (!mode)
+ return NULL;
+
+ /* TODO: interpret S3D flags */
+
+ mode->type = DRM_MODE_TYPE_DRIVER;
+ drm_mode_set_name(mode);
+
+ return mode;
+}
+
+static int add_displayid_formula_modes(struct drm_connector *connector,
+ const struct displayid_block *block)
+{
+ const struct displayid_formula_timing_block *formula_block = (struct displayid_formula_timing_block *)block;
+ int num_timings;
+ struct drm_display_mode *newmode;
+ int num_modes = 0;
+ bool type_10 = block->tag == DATA_BLOCK_2_TYPE_10_FORMULA_TIMING;
+ int timing_size = 6 + ((formula_block->base.rev & 0x70) >> 4);
+
+ /* extended blocks are not supported yet */
+ if (timing_size != 6)
+ return 0;
+
+ if (block->num_bytes % timing_size)
+ return 0;
+
+ num_timings = block->num_bytes / timing_size;
+ for (int i = 0; i < num_timings; i++) {
+ const struct displayid_formula_timings_9 *timings = &formula_block->timings[i];
+
+ newmode = drm_mode_displayid_formula(connector->dev, timings, type_10);
+ if (!newmode)
+ continue;
+
+ drm_mode_probed_add(connector, newmode);
+ num_modes++;
+ }
+ return num_modes;
+}
+
static int add_displayid_detailed_modes(struct drm_connector *connector,
const struct drm_edid *drm_edid)
{
@@ -6845,6 +6905,9 @@ static int add_displayid_detailed_modes(struct drm_connector *connector,
if (block->tag == DATA_BLOCK_TYPE_1_DETAILED_TIMING ||
block->tag == DATA_BLOCK_2_TYPE_7_DETAILED_TIMING)
num_modes += add_displayid_detailed_1_modes(connector, block);
+ else if (block->tag == DATA_BLOCK_2_TYPE_9_FORMULA_TIMING ||
+ block->tag == DATA_BLOCK_2_TYPE_10_FORMULA_TIMING)
+ num_modes += add_displayid_formula_modes(connector, block);
}
displayid_iter_end(&iter);
--
2.48.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/edid: Implement DisplayID Type IX & X timing blocks parsing
2025-02-12 5:05 [PATCH v2] drm/edid: Implement DisplayID Type IX & X timing blocks parsing Egor Vorontsov
@ 2025-02-12 9:35 ` Jani Nikula
2025-02-12 22:30 ` Egor Vorontsov
2025-02-14 1:05 ` [PATCH v2 2/2] drm/edid: Refactor DisplayID timing block structs Egor Vorontsov
1 sibling, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2025-02-12 9:35 UTC (permalink / raw)
To: Egor Vorontsov, linux-kernel
Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Egor Vorontsov
On Wed, 12 Feb 2025, Egor Vorontsov <sdoregor@sdore.me> wrote:
> Some newer high refresh rate consumer monitors (including those by Samsung)
> make use of DisplayID 2.1 timing blocks in their EDID data, notably for
> their highest refresh rate modes. Such modes won't be available as of now.
>
> Implement partial support for such blocks in order to enable native
> support of HRR modes of most such monitors for users without having to rely
> on EDID patching/override (or need thereof).
>
> Closes: https://gitlab.freedesktop.org/drm/misc/kernel/-/issues/55
> Suggested-by: Maximilian Boße <max@bosse.io>
> Signed-off-by: Egor Vorontsov <sdoregor@sdore.me>
> ---
> drivers/gpu/drm/drm_displayid_internal.h | 13 +++++
> drivers/gpu/drm/drm_edid.c | 63 ++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_displayid_internal.h b/drivers/gpu/drm/drm_displayid_internal.h
> index aee1b86a73c1..88220c107822 100644
> --- a/drivers/gpu/drm/drm_displayid_internal.h
> +++ b/drivers/gpu/drm/drm_displayid_internal.h
> @@ -66,6 +66,7 @@ struct drm_edid;
> #define DATA_BLOCK_2_STEREO_DISPLAY_INTERFACE 0x27
> #define DATA_BLOCK_2_TILED_DISPLAY_TOPOLOGY 0x28
> #define DATA_BLOCK_2_CONTAINER_ID 0x29
> +#define DATA_BLOCK_2_TYPE_10_FORMULA_TIMING 0x2a
> #define DATA_BLOCK_2_VENDOR_SPECIFIC 0x7e
> #define DATA_BLOCK_2_CTA_DISPLAY_ID 0x81
>
> @@ -129,6 +130,18 @@ struct displayid_detailed_timing_block {
> struct displayid_detailed_timings_1 timings[];
> };
>
> +struct displayid_formula_timings_9 {
> + u8 flags;
> + __be16 hactive;
> + __be16 vactive;
> + u8 vrefresh;
> +} __packed;
> +
> +struct displayid_formula_timing_block {
> + struct displayid_block base;
> + struct displayid_formula_timings_9 timings[];
> +} __packed;
> +
> #define DISPLAYID_VESA_MSO_OVERLAP GENMASK(3, 0)
> #define DISPLAYID_VESA_MSO_MODE GENMASK(6, 5)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 13bc4c290b17..9c363df5af9a 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6833,6 +6833,66 @@ static int add_displayid_detailed_1_modes(struct drm_connector *connector,
> return num_modes;
> }
>
> +static struct drm_display_mode *drm_mode_displayid_formula(struct drm_device *dev,
> + const struct displayid_formula_timings_9 *timings,
> + bool type_10)
> +{
> + struct drm_display_mode *mode;
> + u16 hactive = be16_to_cpu(timings->hactive) + 1;
> + u16 vactive = be16_to_cpu(timings->vactive) + 1;
> + u8 timing_formula = timings->flags & 0x7;
> +
> + /* TODO: support RB-v2 & RB-v3 */
> + if (timing_formula > 1)
> + return NULL;
> +
> + /* TODO: support video-optimized refresh rate */
> + if (timings->flags & (1 << 4))
> + return NULL;
Mmh. I'm not sure I'd go this far. The bit indicates *two* timings, one
for which the below *is* correct, and another additional one with
vrefresh * (1000/1001).
We could just add a drm_dbg_kms(dev, "<message>") here about missing
fractional refresh rate, and proceed with the one non-fractional rate?
Or just have the TODO comment with no checks.
And I'm not asking you to just implement the fractional refresh rate
here, because we can't simply do it on the vrefresh due to losing
precision. Needs to be done on the clock. But it could be a follow-up,
using the above bit to do something similar to what
add_alternate_cea_modes() does.
Either way,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Are you up for the follow-ups too? And since you've got the hang of it,
maybe fix struct displayid_formula_timings_9 hactive/vactive to __be16
as well?
BR,
Jani.
> +
> + mode = drm_cvt_mode(dev, hactive, vactive, timings->vrefresh + 1, timing_formula == 1, false, false);
> + if (!mode)
> + return NULL;
> +
> + /* TODO: interpret S3D flags */
> +
> + mode->type = DRM_MODE_TYPE_DRIVER;
> + drm_mode_set_name(mode);
> +
> + return mode;
> +}
> +
> +static int add_displayid_formula_modes(struct drm_connector *connector,
> + const struct displayid_block *block)
> +{
> + const struct displayid_formula_timing_block *formula_block = (struct displayid_formula_timing_block *)block;
> + int num_timings;
> + struct drm_display_mode *newmode;
> + int num_modes = 0;
> + bool type_10 = block->tag == DATA_BLOCK_2_TYPE_10_FORMULA_TIMING;
> + int timing_size = 6 + ((formula_block->base.rev & 0x70) >> 4);
> +
> + /* extended blocks are not supported yet */
> + if (timing_size != 6)
> + return 0;
> +
> + if (block->num_bytes % timing_size)
> + return 0;
> +
> + num_timings = block->num_bytes / timing_size;
> + for (int i = 0; i < num_timings; i++) {
> + const struct displayid_formula_timings_9 *timings = &formula_block->timings[i];
> +
> + newmode = drm_mode_displayid_formula(connector->dev, timings, type_10);
> + if (!newmode)
> + continue;
> +
> + drm_mode_probed_add(connector, newmode);
> + num_modes++;
> + }
> + return num_modes;
> +}
> +
> static int add_displayid_detailed_modes(struct drm_connector *connector,
> const struct drm_edid *drm_edid)
> {
> @@ -6845,6 +6905,9 @@ static int add_displayid_detailed_modes(struct drm_connector *connector,
> if (block->tag == DATA_BLOCK_TYPE_1_DETAILED_TIMING ||
> block->tag == DATA_BLOCK_2_TYPE_7_DETAILED_TIMING)
> num_modes += add_displayid_detailed_1_modes(connector, block);
> + else if (block->tag == DATA_BLOCK_2_TYPE_9_FORMULA_TIMING ||
> + block->tag == DATA_BLOCK_2_TYPE_10_FORMULA_TIMING)
> + num_modes += add_displayid_formula_modes(connector, block);
> }
> displayid_iter_end(&iter);
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/edid: Implement DisplayID Type IX & X timing blocks parsing
2025-02-12 9:35 ` Jani Nikula
@ 2025-02-12 22:30 ` Egor Vorontsov
2025-02-14 8:22 ` Jani Nikula
0 siblings, 1 reply; 6+ messages in thread
From: Egor Vorontsov @ 2025-02-12 22:30 UTC (permalink / raw)
To: Jani Nikula, linux-kernel
Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Egor Vorontsov
On Wed, 2025-02-12 at 11:35 +0200, Jani Nikula wrote:
> > + /* TODO: support video-optimized refresh rate */
> > + if (timings->flags & (1 << 4))
> > + return NULL;
>
> Mmh. I'm not sure I'd go this far. The bit indicates *two* timings, one
> for which the below *is* correct, and another additional one with
> vrefresh * (1000/1001).
>
> We could just add a drm_dbg_kms(dev, "<message>") here about missing
> fractional refresh rate, and proceed with the one non-fractional rate?
> Or just have the TODO comment with no checks.
I'll go with the former, for now.
> Either way,
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Thank you. ... But!
> Are you up for the follow-ups too? And since you've got the hang of it,
> maybe fix struct displayid_formula_timings_9 hactive/vactive to __be16
> as well?
... at this moment I realised that both the specs and the legacy code
actually indicate it's indeed *little*-endian shorts!
I.e. `x[0] | x[1] << 8' -- that's LSB-first.
Also, virtually no code in `drm_edid.c' uses big-endian.
Thus, I'm fixing both my code and `displayid_detailed_timings_1' (I
suppose you meant this struct instead) to use __le16.
Egor
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/edid: Implement DisplayID Type IX & X timing blocks parsing
2025-02-12 22:30 ` Egor Vorontsov
@ 2025-02-14 8:22 ` Jani Nikula
0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2025-02-14 8:22 UTC (permalink / raw)
To: Egor Vorontsov, linux-kernel
Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Egor Vorontsov
On Thu, 13 Feb 2025, Egor Vorontsov <sdoregor@sdore.me> wrote:
> On Wed, 2025-02-12 at 11:35 +0200, Jani Nikula wrote:
>> > + /* TODO: support video-optimized refresh rate */
>> > + if (timings->flags & (1 << 4))
>> > + return NULL;
>>
>> Mmh. I'm not sure I'd go this far. The bit indicates *two* timings, one
>> for which the below *is* correct, and another additional one with
>> vrefresh * (1000/1001).
>>
>> We could just add a drm_dbg_kms(dev, "<message>") here about missing
>> fractional refresh rate, and proceed with the one non-fractional rate?
>> Or just have the TODO comment with no checks.
> I'll go with the former, for now.
>
>> Either way,
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Thank you. ... But!
>
>> Are you up for the follow-ups too? And since you've got the hang of it,
>> maybe fix struct displayid_formula_timings_9 hactive/vactive to __be16
>> as well?
> ... at this moment I realised that both the specs and the legacy code
> actually indicate it's indeed *little*-endian shorts!
> I.e. `x[0] | x[1] << 8' -- that's LSB-first.
>
> Also, virtually no code in `drm_edid.c' uses big-endian.
Yes, I *obviously* meant __be16 and be16_to_cpu(). ;D
Good catch, and sorry about that, quite the *facepalm* for me.
> Thus, I'm fixing both my code and `displayid_detailed_timings_1' (I
> suppose you meant this struct instead) to use __le16.
Indeed.
Thanks,
Jani.
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] drm/edid: Refactor DisplayID timing block structs
2025-02-12 5:05 [PATCH v2] drm/edid: Implement DisplayID Type IX & X timing blocks parsing Egor Vorontsov
2025-02-12 9:35 ` Jani Nikula
@ 2025-02-14 1:05 ` Egor Vorontsov
2025-02-14 8:28 ` Jani Nikula
1 sibling, 1 reply; 6+ messages in thread
From: Egor Vorontsov @ 2025-02-14 1:05 UTC (permalink / raw)
To: linux-kernel
Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Jani Nikula, Egor Vorontsov
Using le16 instead of u8[2].
Replaced an error with a printed warning as well.
Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Egor Vorontsov <sdoregor@sdore.me>
---
drivers/gpu/drm/drm_displayid_internal.h | 22 ++++++++--------
drivers/gpu/drm/drm_edid.c | 32 ++++++++++++------------
2 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_displayid_internal.h b/drivers/gpu/drm/drm_displayid_internal.h
index 88220c107822..957dd0619f5c 100644
--- a/drivers/gpu/drm/drm_displayid_internal.h
+++ b/drivers/gpu/drm/drm_displayid_internal.h
@@ -115,25 +115,25 @@ struct displayid_tiled_block {
struct displayid_detailed_timings_1 {
u8 pixel_clock[3];
u8 flags;
- u8 hactive[2];
- u8 hblank[2];
- u8 hsync[2];
- u8 hsw[2];
- u8 vactive[2];
- u8 vblank[2];
- u8 vsync[2];
- u8 vsw[2];
+ __le16 hactive;
+ __le16 hblank;
+ __le16 hsync;
+ __le16 hsw;
+ __le16 vactive;
+ __le16 vblank;
+ __le16 vsync;
+ __le16 vsw;
} __packed;
struct displayid_detailed_timing_block {
struct displayid_block base;
struct displayid_detailed_timings_1 timings[];
-};
+} __packed;
struct displayid_formula_timings_9 {
u8 flags;
- __be16 hactive;
- __be16 vactive;
+ __le16 hactive;
+ __le16 vactive;
u8 vrefresh;
} __packed;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9c363df5af9a..54122a12a24f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6764,19 +6764,19 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d
bool type_7)
{
struct drm_display_mode *mode;
- unsigned pixel_clock = (timings->pixel_clock[0] |
- (timings->pixel_clock[1] << 8) |
- (timings->pixel_clock[2] << 16)) + 1;
- unsigned hactive = (timings->hactive[0] | timings->hactive[1] << 8) + 1;
- unsigned hblank = (timings->hblank[0] | timings->hblank[1] << 8) + 1;
- unsigned hsync = (timings->hsync[0] | (timings->hsync[1] & 0x7f) << 8) + 1;
- unsigned hsync_width = (timings->hsw[0] | timings->hsw[1] << 8) + 1;
- unsigned vactive = (timings->vactive[0] | timings->vactive[1] << 8) + 1;
- unsigned vblank = (timings->vblank[0] | timings->vblank[1] << 8) + 1;
- unsigned vsync = (timings->vsync[0] | (timings->vsync[1] & 0x7f) << 8) + 1;
- unsigned vsync_width = (timings->vsw[0] | timings->vsw[1] << 8) + 1;
- bool hsync_positive = (timings->hsync[1] >> 7) & 0x1;
- bool vsync_positive = (timings->vsync[1] >> 7) & 0x1;
+ unsigned int pixel_clock = (timings->pixel_clock[0] |
+ (timings->pixel_clock[1] << 8) |
+ (timings->pixel_clock[2] << 16)) + 1;
+ unsigned int hactive = le16_to_cpu(timings->hactive) + 1;
+ unsigned int hblank = le16_to_cpu(timings->hblank) + 1;
+ unsigned int hsync = (le16_to_cpu(timings->hsync) & 0x7fff) + 1;
+ unsigned int hsync_width = le16_to_cpu(timings->hsw) + 1;
+ unsigned int vactive = le16_to_cpu(timings->vactive) + 1;
+ unsigned int vblank = le16_to_cpu(timings->vblank) + 1;
+ unsigned int vsync = (le16_to_cpu(timings->vsync) & 0x7fff) + 1;
+ unsigned int vsync_width = le16_to_cpu(timings->vsw) + 1;
+ bool hsync_positive = le16_to_cpu(timings->hsync) & (1 << 15);
+ bool vsync_positive = le16_to_cpu(timings->vsync) & (1 << 15);
mode = drm_mode_create(dev);
if (!mode)
@@ -6838,8 +6838,8 @@ static struct drm_display_mode *drm_mode_displayid_formula(struct drm_device *de
bool type_10)
{
struct drm_display_mode *mode;
- u16 hactive = be16_to_cpu(timings->hactive) + 1;
- u16 vactive = be16_to_cpu(timings->vactive) + 1;
+ u16 hactive = le16_to_cpu(timings->hactive) + 1;
+ u16 vactive = le16_to_cpu(timings->vactive) + 1;
u8 timing_formula = timings->flags & 0x7;
/* TODO: support RB-v2 & RB-v3 */
@@ -6848,7 +6848,7 @@ static struct drm_display_mode *drm_mode_displayid_formula(struct drm_device *de
/* TODO: support video-optimized refresh rate */
if (timings->flags & (1 << 4))
- return NULL;
+ drm_dbg_kms(dev, "Fractional vrefresh is not implemented, proceeding with non-video-optimized refresh rate");
mode = drm_cvt_mode(dev, hactive, vactive, timings->vrefresh + 1, timing_formula == 1, false, false);
if (!mode)
--
2.48.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] drm/edid: Refactor DisplayID timing block structs
2025-02-14 1:05 ` [PATCH v2 2/2] drm/edid: Refactor DisplayID timing block structs Egor Vorontsov
@ 2025-02-14 8:28 ` Jani Nikula
0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2025-02-14 8:28 UTC (permalink / raw)
To: Egor Vorontsov, linux-kernel
Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Egor Vorontsov
On Fri, 14 Feb 2025, Egor Vorontsov <sdoregor@sdore.me> wrote:
> Using le16 instead of u8[2].
> Replaced an error with a printed warning as well.
>
> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Egor Vorontsov <sdoregor@sdore.me>
So the two patches should each stand on their own, instead of this one
fixing the issues in the first.
BR,
Jani.
> ---
> drivers/gpu/drm/drm_displayid_internal.h | 22 ++++++++--------
> drivers/gpu/drm/drm_edid.c | 32 ++++++++++++------------
> 2 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_displayid_internal.h b/drivers/gpu/drm/drm_displayid_internal.h
> index 88220c107822..957dd0619f5c 100644
> --- a/drivers/gpu/drm/drm_displayid_internal.h
> +++ b/drivers/gpu/drm/drm_displayid_internal.h
> @@ -115,25 +115,25 @@ struct displayid_tiled_block {
> struct displayid_detailed_timings_1 {
> u8 pixel_clock[3];
> u8 flags;
> - u8 hactive[2];
> - u8 hblank[2];
> - u8 hsync[2];
> - u8 hsw[2];
> - u8 vactive[2];
> - u8 vblank[2];
> - u8 vsync[2];
> - u8 vsw[2];
> + __le16 hactive;
> + __le16 hblank;
> + __le16 hsync;
> + __le16 hsw;
> + __le16 vactive;
> + __le16 vblank;
> + __le16 vsync;
> + __le16 vsw;
> } __packed;
>
> struct displayid_detailed_timing_block {
> struct displayid_block base;
> struct displayid_detailed_timings_1 timings[];
> -};
> +} __packed;
The above belong in patch 2.
>
> struct displayid_formula_timings_9 {
> u8 flags;
> - __be16 hactive;
> - __be16 vactive;
> + __le16 hactive;
> + __le16 vactive;
The above belong in patch 1.
> u8 vrefresh;
> } __packed;
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 9c363df5af9a..54122a12a24f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6764,19 +6764,19 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d
> bool type_7)
> {
> struct drm_display_mode *mode;
> - unsigned pixel_clock = (timings->pixel_clock[0] |
> - (timings->pixel_clock[1] << 8) |
> - (timings->pixel_clock[2] << 16)) + 1;
> - unsigned hactive = (timings->hactive[0] | timings->hactive[1] << 8) + 1;
> - unsigned hblank = (timings->hblank[0] | timings->hblank[1] << 8) + 1;
> - unsigned hsync = (timings->hsync[0] | (timings->hsync[1] & 0x7f) << 8) + 1;
> - unsigned hsync_width = (timings->hsw[0] | timings->hsw[1] << 8) + 1;
> - unsigned vactive = (timings->vactive[0] | timings->vactive[1] << 8) + 1;
> - unsigned vblank = (timings->vblank[0] | timings->vblank[1] << 8) + 1;
> - unsigned vsync = (timings->vsync[0] | (timings->vsync[1] & 0x7f) << 8) + 1;
> - unsigned vsync_width = (timings->vsw[0] | timings->vsw[1] << 8) + 1;
> - bool hsync_positive = (timings->hsync[1] >> 7) & 0x1;
> - bool vsync_positive = (timings->vsync[1] >> 7) & 0x1;
> + unsigned int pixel_clock = (timings->pixel_clock[0] |
> + (timings->pixel_clock[1] << 8) |
> + (timings->pixel_clock[2] << 16)) + 1;
> + unsigned int hactive = le16_to_cpu(timings->hactive) + 1;
> + unsigned int hblank = le16_to_cpu(timings->hblank) + 1;
> + unsigned int hsync = (le16_to_cpu(timings->hsync) & 0x7fff) + 1;
> + unsigned int hsync_width = le16_to_cpu(timings->hsw) + 1;
> + unsigned int vactive = le16_to_cpu(timings->vactive) + 1;
> + unsigned int vblank = le16_to_cpu(timings->vblank) + 1;
> + unsigned int vsync = (le16_to_cpu(timings->vsync) & 0x7fff) + 1;
> + unsigned int vsync_width = le16_to_cpu(timings->vsw) + 1;
> + bool hsync_positive = le16_to_cpu(timings->hsync) & (1 << 15);
> + bool vsync_positive = le16_to_cpu(timings->vsync) & (1 << 15);
The above belong in patch 2.
>
> mode = drm_mode_create(dev);
> if (!mode)
> @@ -6838,8 +6838,8 @@ static struct drm_display_mode *drm_mode_displayid_formula(struct drm_device *de
> bool type_10)
> {
> struct drm_display_mode *mode;
> - u16 hactive = be16_to_cpu(timings->hactive) + 1;
> - u16 vactive = be16_to_cpu(timings->vactive) + 1;
> + u16 hactive = le16_to_cpu(timings->hactive) + 1;
> + u16 vactive = le16_to_cpu(timings->vactive) + 1;
> u8 timing_formula = timings->flags & 0x7;
>
> /* TODO: support RB-v2 & RB-v3 */
> @@ -6848,7 +6848,7 @@ static struct drm_display_mode *drm_mode_displayid_formula(struct drm_device *de
>
> /* TODO: support video-optimized refresh rate */
> if (timings->flags & (1 << 4))
> - return NULL;
> + drm_dbg_kms(dev, "Fractional vrefresh is not implemented, proceeding with non-video-optimized refresh rate");
The above belong in patch 1.
>
> mode = drm_cvt_mode(dev, hactive, vactive, timings->vrefresh + 1, timing_formula == 1, false, false);
> if (!mode)
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-14 8:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-12 5:05 [PATCH v2] drm/edid: Implement DisplayID Type IX & X timing blocks parsing Egor Vorontsov
2025-02-12 9:35 ` Jani Nikula
2025-02-12 22:30 ` Egor Vorontsov
2025-02-14 8:22 ` Jani Nikula
2025-02-14 1:05 ` [PATCH v2 2/2] drm/edid: Refactor DisplayID timing block structs Egor Vorontsov
2025-02-14 8:28 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox