* [PATCH 2/4] drm/displayid: Iterate over all DisplayID blocks
2016-05-10 1:20 [PATCH 1/4] drm/edid: move displayid tiled block parsing into separate function Dave Airlie
@ 2016-05-10 1:20 ` Dave Airlie
2016-05-10 9:02 ` Jani Nikula
2016-05-10 1:20 ` [PATCH 3/4] drm/edid: move displayid validation to it's own function Dave Airlie
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Dave Airlie @ 2016-05-10 1:20 UTC (permalink / raw)
To: dri-devel
From: Tomas Bzatek <tomas@bzatek.net>
This will iterate over all DisplayID blocks found in the buffer.
Previously only the first block was parsed.
https://bugs.freedesktop.org/show_bug.cgi?id=95207
Signed-off-by: Tomas Bzatek <tomas@bzatek.net>
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
drivers/gpu/drm/drm_edid.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9dbaaf4..3cf17a3 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4237,18 +4237,24 @@ static int drm_parse_display_id(struct drm_connector *connector,
return -EINVAL;
}
- block = (struct displayid_block *)&displayid[idx + 4];
- DRM_DEBUG_KMS("block id %d, rev %d, len %d\n",
- block->tag, block->rev, block->num_bytes);
-
- switch (block->tag) {
- case DATA_BLOCK_TILED_DISPLAY:
- ret = drm_parse_tiled_block(connector, block);
- if (ret)
- return ret;
- default:
- printk("unknown displayid tag %d\n", block->tag);
- break;
+ idx += sizeof(struct displayid_hdr);
+ while (block = (struct displayid_block *)&displayid[idx],
+ idx + sizeof(struct displayid_block) <= length &&
+ idx + sizeof(struct displayid_block) + block->num_bytes <= length &&
+ block->num_bytes > 0) {
+ idx += block->num_bytes + sizeof(struct displayid_block);
+ DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n",
+ block->tag, block->rev, block->num_bytes);
+
+ switch (block->tag) {
+ case DATA_BLOCK_TILED_DISPLAY:
+ ret = drm_parse_tiled_block(connector, block);
+ if (ret)
+ return ret;
+ default:
+ DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n", block->tag);
+ break;
+ }
}
return 0;
}
--
2.5.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/4] drm/displayid: Iterate over all DisplayID blocks
2016-05-10 1:20 ` [PATCH 2/4] drm/displayid: Iterate over all DisplayID blocks Dave Airlie
@ 2016-05-10 9:02 ` Jani Nikula
0 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2016-05-10 9:02 UTC (permalink / raw)
To: Dave Airlie, dri-devel
On Tue, 10 May 2016, Dave Airlie <airlied@gmail.com> wrote:
> From: Tomas Bzatek <tomas@bzatek.net>
>
> This will iterate over all DisplayID blocks found in the buffer.
> Previously only the first block was parsed.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=95207
>
> Signed-off-by: Tomas Bzatek <tomas@bzatek.net>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
> drivers/gpu/drm/drm_edid.c | 30 ++++++++++++++++++------------
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 9dbaaf4..3cf17a3 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4237,18 +4237,24 @@ static int drm_parse_display_id(struct drm_connector *connector,
> return -EINVAL;
> }
>
> - block = (struct displayid_block *)&displayid[idx + 4];
> - DRM_DEBUG_KMS("block id %d, rev %d, len %d\n",
> - block->tag, block->rev, block->num_bytes);
> -
> - switch (block->tag) {
> - case DATA_BLOCK_TILED_DISPLAY:
> - ret = drm_parse_tiled_block(connector, block);
> - if (ret)
> - return ret;
> - default:
> - printk("unknown displayid tag %d\n", block->tag);
> - break;
> + idx += sizeof(struct displayid_hdr);
> + while (block = (struct displayid_block *)&displayid[idx],
> + idx + sizeof(struct displayid_block) <= length &&
> + idx + sizeof(struct displayid_block) + block->num_bytes <= length &&
> + block->num_bytes > 0) {
Ugh, that's a bit ugly, but seems to DTRT.
Do we need to check block->num_bytes > 0? Shouldn't that be block->tag
specific? See below.
> + idx += block->num_bytes + sizeof(struct displayid_block);
> + DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n",
> + block->tag, block->rev, block->num_bytes);
> +
> + switch (block->tag) {
> + case DATA_BLOCK_TILED_DISPLAY:
This could use a check (in a separate patch, perhaps) to verify idx +
sizeof(struct displayid_tiled_block) <= length. (Alternatively,
sizeof(struct displayid_block) + block->num_bytes == sizeof(struct
displayid_tiled_block).)
In any case,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> + ret = drm_parse_tiled_block(connector, block);
> + if (ret)
> + return ret;
> + default:
> + DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n", block->tag);
> + break;
> + }
> }
> return 0;
> }
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/4] drm/edid: move displayid validation to it's own function.
2016-05-10 1:20 [PATCH 1/4] drm/edid: move displayid tiled block parsing into separate function Dave Airlie
2016-05-10 1:20 ` [PATCH 2/4] drm/displayid: Iterate over all DisplayID blocks Dave Airlie
@ 2016-05-10 1:20 ` Dave Airlie
2016-05-10 9:14 ` Jani Nikula
2016-05-10 1:20 ` [PATCH 4/4] drm/edid: add displayid detailed 1 timings to the modelist. (v1.1) Dave Airlie
2016-05-10 8:15 ` [PATCH 1/4] drm/edid: move displayid tiled block parsing into separate function Jani Nikula
3 siblings, 1 reply; 8+ messages in thread
From: Dave Airlie @ 2016-05-10 1:20 UTC (permalink / raw)
To: dri-devel
From: Dave Airlie <airlied@redhat.com>
We need to use this for validating modeline additions.
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
drivers/gpu/drm/drm_edid.c | 44 ++++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3cf17a3..73d4218 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3901,6 +3901,29 @@ static void drm_add_display_info(struct edid *edid,
info->color_formats |= DRM_COLOR_FORMAT_YCRCB422;
}
+static int validate_displayid(u8 *displayid, int length, int idx)
+{
+ int i;
+ u8 csum = 0;
+ struct displayid_hdr *base;
+
+ base = (struct displayid_hdr *)&displayid[idx];
+
+ DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
+ base->rev, base->bytes, base->prod_id, base->ext_count);
+
+ if (base->bytes + 5 > length - idx)
+ return -EINVAL;
+ for (i = idx; i <= base->bytes + 5; i++) {
+ csum += displayid[i];
+ }
+ if (csum) {
+ DRM_ERROR("DisplayID checksum invalid, remainder is %d\n", csum);
+ return -EINVAL;
+ }
+ return 0;
+}
+
/**
* drm_add_edid_modes - add modes from EDID data, if available
* @connector: connector we're probing
@@ -4212,30 +4235,15 @@ static int drm_parse_display_id(struct drm_connector *connector,
{
/* if this is an EDID extension the first byte will be 0x70 */
int idx = 0;
- struct displayid_hdr *base;
struct displayid_block *block;
- u8 csum = 0;
- int i;
int ret;
if (is_edid_extension)
idx = 1;
- base = (struct displayid_hdr *)&displayid[idx];
-
- DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
- base->rev, base->bytes, base->prod_id, base->ext_count);
-
- if (base->bytes + 5 > length - idx)
- return -EINVAL;
-
- for (i = idx; i <= base->bytes + 5; i++) {
- csum += displayid[i];
- }
- if (csum) {
- DRM_ERROR("DisplayID checksum invalid, remainder is %d\n", csum);
- return -EINVAL;
- }
+ ret = validate_displayid(displayid, length, idx);
+ if (ret)
+ return ret;
idx += sizeof(struct displayid_hdr);
while (block = (struct displayid_block *)&displayid[idx],
--
2.5.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 3/4] drm/edid: move displayid validation to it's own function.
2016-05-10 1:20 ` [PATCH 3/4] drm/edid: move displayid validation to it's own function Dave Airlie
@ 2016-05-10 9:14 ` Jani Nikula
0 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2016-05-10 9:14 UTC (permalink / raw)
To: Dave Airlie, dri-devel
On Tue, 10 May 2016, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> We need to use this for validating modeline additions.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
> drivers/gpu/drm/drm_edid.c | 44 ++++++++++++++++++++++++++------------------
> 1 file changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3cf17a3..73d4218 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3901,6 +3901,29 @@ static void drm_add_display_info(struct edid *edid,
> info->color_formats |= DRM_COLOR_FORMAT_YCRCB422;
> }
>
> +static int validate_displayid(u8 *displayid, int length, int idx)
Bikeshed, (u8 *displayid, int idx, int length) would feel like a more
natural order to me.
> +{
> + int i;
> + u8 csum = 0;
> + struct displayid_hdr *base;
> +
> + base = (struct displayid_hdr *)&displayid[idx];
> +
> + DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
> + base->rev, base->bytes, base->prod_id, base->ext_count);
> +
I guess to be pedantic we should check idx + sizeof(struct
displayid_hdr) <= length before looking at base. This patch is about
abstracting the thing, so should be a separate patch anyway.
Other than the bikeshed,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> + if (base->bytes + 5 > length - idx)
> + return -EINVAL;
> + for (i = idx; i <= base->bytes + 5; i++) {
> + csum += displayid[i];
> + }
> + if (csum) {
> + DRM_ERROR("DisplayID checksum invalid, remainder is %d\n", csum);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> /**
> * drm_add_edid_modes - add modes from EDID data, if available
> * @connector: connector we're probing
> @@ -4212,30 +4235,15 @@ static int drm_parse_display_id(struct drm_connector *connector,
> {
> /* if this is an EDID extension the first byte will be 0x70 */
> int idx = 0;
> - struct displayid_hdr *base;
> struct displayid_block *block;
> - u8 csum = 0;
> - int i;
> int ret;
>
> if (is_edid_extension)
> idx = 1;
>
> - base = (struct displayid_hdr *)&displayid[idx];
> -
> - DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
> - base->rev, base->bytes, base->prod_id, base->ext_count);
> -
> - if (base->bytes + 5 > length - idx)
> - return -EINVAL;
> -
> - for (i = idx; i <= base->bytes + 5; i++) {
> - csum += displayid[i];
> - }
> - if (csum) {
> - DRM_ERROR("DisplayID checksum invalid, remainder is %d\n", csum);
> - return -EINVAL;
> - }
> + ret = validate_displayid(displayid, length, idx);
> + if (ret)
> + return ret;
>
> idx += sizeof(struct displayid_hdr);
> while (block = (struct displayid_block *)&displayid[idx],
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/4] drm/edid: add displayid detailed 1 timings to the modelist. (v1.1)
2016-05-10 1:20 [PATCH 1/4] drm/edid: move displayid tiled block parsing into separate function Dave Airlie
2016-05-10 1:20 ` [PATCH 2/4] drm/displayid: Iterate over all DisplayID blocks Dave Airlie
2016-05-10 1:20 ` [PATCH 3/4] drm/edid: move displayid validation to it's own function Dave Airlie
@ 2016-05-10 1:20 ` Dave Airlie
2016-05-10 8:15 ` [PATCH 1/4] drm/edid: move displayid tiled block parsing into separate function Jani Nikula
3 siblings, 0 replies; 8+ messages in thread
From: Dave Airlie @ 2016-05-10 1:20 UTC (permalink / raw)
To: dri-devel
From: Dave Airlie <airlied@redhat.com>
The tiled 5K Dell monitor appears to be hiding it's tiled mode
inside the displayid timings block, this patch parses this
blocks and adds the modes to the modelist.
v1.1: add missing __packed.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95207
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
drivers/gpu/drm/drm_edid.c | 108 ++++++++++++++++++++++++++++++++++++++++++++
include/drm/drm_displayid.h | 17 +++++++
2 files changed, 125 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 73d4218..46ccd60 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3924,6 +3924,110 @@ static int validate_displayid(u8 *displayid, int length, int idx)
return 0;
}
+static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *dev,
+ struct displayid_detailed_timings_1 *timings)
+{
+ struct drm_display_mode *mode;
+ unsigned pixel_clock = (timings->pixel_clock[0] |
+ (timings->pixel_clock[1] << 8) |
+ (timings->pixel_clock[2] << 16));
+ 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;
+ mode = drm_mode_create(dev);
+ if (!mode)
+ return NULL;
+
+ mode->clock = pixel_clock * 10;
+ mode->hdisplay = hactive;
+ mode->hsync_start = mode->hdisplay + hsync;
+ mode->hsync_end = mode->hsync_start + hsync_width;
+ mode->htotal = mode->hdisplay + hblank;
+
+ mode->vdisplay = vactive;
+ mode->vsync_start = mode->vdisplay + vsync;
+ mode->vsync_end = mode->vsync_start + vsync_width;
+ mode->vtotal = mode->vdisplay + vblank;
+
+ mode->flags = 0;
+ mode->flags |= hsync_positive ? DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
+ mode->flags |= vsync_positive ? DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
+ mode->type = DRM_MODE_TYPE_DRIVER;
+
+ if (timings->flags & 0x80)
+ mode->type |= DRM_MODE_TYPE_PREFERRED;
+ mode->vrefresh = drm_mode_vrefresh(mode);
+ drm_mode_set_name(mode);
+
+ return mode;
+}
+
+static int add_displayid_detailed_1_modes(struct drm_connector *connector,
+ struct displayid_block *block)
+{
+ struct displayid_detailed_timing_block *det = (struct displayid_detailed_timing_block *)block;
+ int i;
+ int num_timings;
+ struct drm_display_mode *newmode;
+ int num_modes = 0;
+ /* blocks must be multiple of 20 bytes length */
+ if (block->num_bytes % 20)
+ return 0;
+
+ num_timings = block->num_bytes / 20;
+ for (i = 0; i < num_timings; i++) {
+ struct displayid_detailed_timings_1 *timings = &det->timings[i];
+
+ newmode = drm_mode_displayid_detailed(connector->dev, timings);
+ if (!newmode)
+ continue;
+
+ drm_mode_probed_add(connector, newmode);
+ num_modes++;
+ }
+ return num_modes;
+}
+
+static int add_displayid_detailed_modes(struct drm_connector *connector,
+ struct edid *edid)
+{
+ u8 *displayid;
+ int ret;
+ int idx = 1;
+ int length = EDID_LENGTH;
+ struct displayid_block *block;
+ int num_modes = 0;
+
+ displayid = drm_find_displayid_extension(edid);
+ if (!displayid)
+ return 0;
+
+ ret = validate_displayid(displayid, length, idx);
+ if (ret)
+ return 0;
+
+ idx += sizeof(struct displayid_hdr);
+ while (block = (struct displayid_block *)&displayid[idx],
+ idx + sizeof(struct displayid_block) <= length &&
+ idx + sizeof(struct displayid_block) + block->num_bytes <= length &&
+ block->num_bytes > 0) {
+ idx += block->num_bytes + sizeof(struct displayid_block);
+ switch (block->tag) {
+ case DATA_BLOCK_TYPE_1_DETAILED_TIMING:
+ num_modes += add_displayid_detailed_1_modes(connector, block);
+ break;
+ }
+ }
+ return num_modes;
+}
+
/**
* drm_add_edid_modes - add modes from EDID data, if available
* @connector: connector we're probing
@@ -3969,6 +4073,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
num_modes += add_established_modes(connector, edid);
num_modes += add_cea_modes(connector, edid);
num_modes += add_alternate_cea_modes(connector, edid);
+ num_modes += add_displayid_detailed_modes(connector, edid);
if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF)
num_modes += add_inferred_modes(connector, edid);
@@ -4259,6 +4364,9 @@ static int drm_parse_display_id(struct drm_connector *connector,
ret = drm_parse_tiled_block(connector, block);
if (ret)
return ret;
+ case DATA_BLOCK_TYPE_1_DETAILED_TIMING:
+ /* handled in mode gathering code. */
+ break;
default:
DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n", block->tag);
break;
diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h
index 623b4e9..c0d4df6 100644
--- a/include/drm/drm_displayid.h
+++ b/include/drm/drm_displayid.h
@@ -73,4 +73,21 @@ struct displayid_tiled_block {
u8 topology_id[8];
} __packed;
+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];
+} __packed;
+
+struct displayid_detailed_timing_block {
+ struct displayid_block base;
+ struct displayid_detailed_timings_1 timings[0];
+};
#endif
--
2.5.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/4] drm/edid: move displayid tiled block parsing into separate function.
2016-05-10 1:20 [PATCH 1/4] drm/edid: move displayid tiled block parsing into separate function Dave Airlie
` (2 preceding siblings ...)
2016-05-10 1:20 ` [PATCH 4/4] drm/edid: add displayid detailed 1 timings to the modelist. (v1.1) Dave Airlie
@ 2016-05-10 8:15 ` Jani Nikula
2016-05-10 9:01 ` Jani Nikula
3 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2016-05-10 8:15 UTC (permalink / raw)
To: Dave Airlie, dri-devel
On Tue, 10 May 2016, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> This just makes the code easier to follow.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_edid.c | 111 ++++++++++++++++++++++++---------------------
> 1 file changed, 59 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 96b181a..9dbaaf4 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4152,6 +4152,60 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
> }
> EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
>
> +static int drm_parse_tiled_block(struct drm_connector *connector,
> + struct displayid_block *block)
> +{
> + struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;
> + u16 w, h;
> + u8 tile_v_loc, tile_h_loc;
> + u8 num_v_tile, num_h_tile;
> + struct drm_tile_group *tg;
> +
> + w = tile->tile_size[0] | tile->tile_size[1] << 8;
> + h = tile->tile_size[2] | tile->tile_size[3] << 8;
> +
> + num_v_tile = (tile->topo[0] & 0xf) | (tile->topo[2] & 0x30);
> + num_h_tile = (tile->topo[0] >> 4) | ((tile->topo[2] >> 2) & 0x30);
> + tile_v_loc = (tile->topo[1] & 0xf) | ((tile->topo[2] & 0x3) << 4);
> + tile_h_loc = (tile->topo[1] >> 4) | (((tile->topo[2] >> 2) & 0x3) << 4);
> +
> + connector->has_tile = true;
> + if (tile->tile_cap & 0x80)
> + connector->tile_is_single_monitor = true;
> +
> + connector->num_h_tile = num_h_tile + 1;
> + connector->num_v_tile = num_v_tile + 1;
> + connector->tile_h_loc = tile_h_loc;
> + connector->tile_v_loc = tile_v_loc;
> + connector->tile_h_size = w + 1;
> + connector->tile_v_size = h + 1;
> +
> + DRM_DEBUG_KMS("tile cap 0x%x\n", tile->tile_cap);
> + DRM_DEBUG_KMS("tile_size %d x %d\n", w + 1, h + 1);
> + DRM_DEBUG_KMS("topo num tiles %dx%d, location %dx%d\n",
> + num_h_tile + 1, num_v_tile + 1, tile_h_loc, tile_v_loc);
> + DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]);
> +
> + tg = drm_mode_get_tile_group(connector->dev, tile->topology_id);
> + if (!tg) {
> + tg = drm_mode_create_tile_group(connector->dev, tile->topology_id);
> + }
> + if (!tg)
> + return -ENOMEM;
> +
> + if (connector->tile_group != tg) {
> + /* if we haven't got a pointer,
> + take the reference, drop ref to old tile group */
> + if (connector->tile_group) {
> + drm_mode_put_tile_group(connector->dev, connector->tile_group);
> + }
> + connector->tile_group = tg;
> + } else
> + /* if same tile group, then release the ref we just took. */
> + drm_mode_put_tile_group(connector->dev, tg);
> + return 0;
> +}
> +
> static int drm_parse_display_id(struct drm_connector *connector,
> u8 *displayid, int length,
> bool is_edid_extension)
> @@ -4162,6 +4216,7 @@ static int drm_parse_display_id(struct drm_connector *connector,
> struct displayid_block *block;
> u8 csum = 0;
> int i;
> + int ret;
>
> if (is_edid_extension)
> idx = 1;
> @@ -4187,58 +4242,10 @@ static int drm_parse_display_id(struct drm_connector *connector,
> block->tag, block->rev, block->num_bytes);
>
> switch (block->tag) {
> - case DATA_BLOCK_TILED_DISPLAY: {
> - struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;
> -
> - u16 w, h;
> - u8 tile_v_loc, tile_h_loc;
> - u8 num_v_tile, num_h_tile;
> - struct drm_tile_group *tg;
> -
> - w = tile->tile_size[0] | tile->tile_size[1] << 8;
> - h = tile->tile_size[2] | tile->tile_size[3] << 8;
> -
> - num_v_tile = (tile->topo[0] & 0xf) | (tile->topo[2] & 0x30);
> - num_h_tile = (tile->topo[0] >> 4) | ((tile->topo[2] >> 2) & 0x30);
> - tile_v_loc = (tile->topo[1] & 0xf) | ((tile->topo[2] & 0x3) << 4);
> - tile_h_loc = (tile->topo[1] >> 4) | (((tile->topo[2] >> 2) & 0x3) << 4);
> -
> - connector->has_tile = true;
> - if (tile->tile_cap & 0x80)
> - connector->tile_is_single_monitor = true;
> -
> - connector->num_h_tile = num_h_tile + 1;
> - connector->num_v_tile = num_v_tile + 1;
> - connector->tile_h_loc = tile_h_loc;
> - connector->tile_v_loc = tile_v_loc;
> - connector->tile_h_size = w + 1;
> - connector->tile_v_size = h + 1;
> -
> - DRM_DEBUG_KMS("tile cap 0x%x\n", tile->tile_cap);
> - DRM_DEBUG_KMS("tile_size %d x %d\n", w + 1, h + 1);
> - DRM_DEBUG_KMS("topo num tiles %dx%d, location %dx%d\n",
> - num_h_tile + 1, num_v_tile + 1, tile_h_loc, tile_v_loc);
> - DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]);
> -
> - tg = drm_mode_get_tile_group(connector->dev, tile->topology_id);
> - if (!tg) {
> - tg = drm_mode_create_tile_group(connector->dev, tile->topology_id);
> - }
> - if (!tg)
> - return -ENOMEM;
> -
> - if (connector->tile_group != tg) {
> - /* if we haven't got a pointer,
> - take the reference, drop ref to old tile group */
> - if (connector->tile_group) {
> - drm_mode_put_tile_group(connector->dev, connector->tile_group);
> - }
> - connector->tile_group = tg;
> - } else
> - /* if same tile group, then release the ref we just took. */
> - drm_mode_put_tile_group(connector->dev, tg);
> - }
> - break;
> + case DATA_BLOCK_TILED_DISPLAY:
> + ret = drm_parse_tiled_block(connector, block);
> + if (ret)
> + return ret;
> default:
> printk("unknown displayid tag %d\n", block->tag);
> break;
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/4] drm/edid: move displayid tiled block parsing into separate function.
2016-05-10 8:15 ` [PATCH 1/4] drm/edid: move displayid tiled block parsing into separate function Jani Nikula
@ 2016-05-10 9:01 ` Jani Nikula
0 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2016-05-10 9:01 UTC (permalink / raw)
To: Dave Airlie, dri-devel
On Tue, 10 May 2016, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Tue, 10 May 2016, Dave Airlie <airlied@gmail.com> wrote:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> This just makes the code easier to follow.
>>
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Ooops, one thing needs fixing below.
>
>> ---
>> drivers/gpu/drm/drm_edid.c | 111 ++++++++++++++++++++++++---------------------
>> 1 file changed, 59 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 96b181a..9dbaaf4 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4152,6 +4152,60 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
>> }
>> EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
>>
>> +static int drm_parse_tiled_block(struct drm_connector *connector,
>> + struct displayid_block *block)
>> +{
>> + struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;
>> + u16 w, h;
>> + u8 tile_v_loc, tile_h_loc;
>> + u8 num_v_tile, num_h_tile;
>> + struct drm_tile_group *tg;
>> +
>> + w = tile->tile_size[0] | tile->tile_size[1] << 8;
>> + h = tile->tile_size[2] | tile->tile_size[3] << 8;
>> +
>> + num_v_tile = (tile->topo[0] & 0xf) | (tile->topo[2] & 0x30);
>> + num_h_tile = (tile->topo[0] >> 4) | ((tile->topo[2] >> 2) & 0x30);
>> + tile_v_loc = (tile->topo[1] & 0xf) | ((tile->topo[2] & 0x3) << 4);
>> + tile_h_loc = (tile->topo[1] >> 4) | (((tile->topo[2] >> 2) & 0x3) << 4);
>> +
>> + connector->has_tile = true;
>> + if (tile->tile_cap & 0x80)
>> + connector->tile_is_single_monitor = true;
>> +
>> + connector->num_h_tile = num_h_tile + 1;
>> + connector->num_v_tile = num_v_tile + 1;
>> + connector->tile_h_loc = tile_h_loc;
>> + connector->tile_v_loc = tile_v_loc;
>> + connector->tile_h_size = w + 1;
>> + connector->tile_v_size = h + 1;
>> +
>> + DRM_DEBUG_KMS("tile cap 0x%x\n", tile->tile_cap);
>> + DRM_DEBUG_KMS("tile_size %d x %d\n", w + 1, h + 1);
>> + DRM_DEBUG_KMS("topo num tiles %dx%d, location %dx%d\n",
>> + num_h_tile + 1, num_v_tile + 1, tile_h_loc, tile_v_loc);
>> + DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]);
>> +
>> + tg = drm_mode_get_tile_group(connector->dev, tile->topology_id);
>> + if (!tg) {
>> + tg = drm_mode_create_tile_group(connector->dev, tile->topology_id);
>> + }
>> + if (!tg)
>> + return -ENOMEM;
>> +
>> + if (connector->tile_group != tg) {
>> + /* if we haven't got a pointer,
>> + take the reference, drop ref to old tile group */
>> + if (connector->tile_group) {
>> + drm_mode_put_tile_group(connector->dev, connector->tile_group);
>> + }
>> + connector->tile_group = tg;
>> + } else
>> + /* if same tile group, then release the ref we just took. */
>> + drm_mode_put_tile_group(connector->dev, tg);
>> + return 0;
>> +}
>> +
>> static int drm_parse_display_id(struct drm_connector *connector,
>> u8 *displayid, int length,
>> bool is_edid_extension)
>> @@ -4162,6 +4216,7 @@ static int drm_parse_display_id(struct drm_connector *connector,
>> struct displayid_block *block;
>> u8 csum = 0;
>> int i;
>> + int ret;
>>
>> if (is_edid_extension)
>> idx = 1;
>> @@ -4187,58 +4242,10 @@ static int drm_parse_display_id(struct drm_connector *connector,
>> block->tag, block->rev, block->num_bytes);
>>
>> switch (block->tag) {
>> - case DATA_BLOCK_TILED_DISPLAY: {
>> - struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;
>> -
>> - u16 w, h;
>> - u8 tile_v_loc, tile_h_loc;
>> - u8 num_v_tile, num_h_tile;
>> - struct drm_tile_group *tg;
>> -
>> - w = tile->tile_size[0] | tile->tile_size[1] << 8;
>> - h = tile->tile_size[2] | tile->tile_size[3] << 8;
>> -
>> - num_v_tile = (tile->topo[0] & 0xf) | (tile->topo[2] & 0x30);
>> - num_h_tile = (tile->topo[0] >> 4) | ((tile->topo[2] >> 2) & 0x30);
>> - tile_v_loc = (tile->topo[1] & 0xf) | ((tile->topo[2] & 0x3) << 4);
>> - tile_h_loc = (tile->topo[1] >> 4) | (((tile->topo[2] >> 2) & 0x3) << 4);
>> -
>> - connector->has_tile = true;
>> - if (tile->tile_cap & 0x80)
>> - connector->tile_is_single_monitor = true;
>> -
>> - connector->num_h_tile = num_h_tile + 1;
>> - connector->num_v_tile = num_v_tile + 1;
>> - connector->tile_h_loc = tile_h_loc;
>> - connector->tile_v_loc = tile_v_loc;
>> - connector->tile_h_size = w + 1;
>> - connector->tile_v_size = h + 1;
>> -
>> - DRM_DEBUG_KMS("tile cap 0x%x\n", tile->tile_cap);
>> - DRM_DEBUG_KMS("tile_size %d x %d\n", w + 1, h + 1);
>> - DRM_DEBUG_KMS("topo num tiles %dx%d, location %dx%d\n",
>> - num_h_tile + 1, num_v_tile + 1, tile_h_loc, tile_v_loc);
>> - DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]);
>> -
>> - tg = drm_mode_get_tile_group(connector->dev, tile->topology_id);
>> - if (!tg) {
>> - tg = drm_mode_create_tile_group(connector->dev, tile->topology_id);
>> - }
>> - if (!tg)
>> - return -ENOMEM;
>> -
>> - if (connector->tile_group != tg) {
>> - /* if we haven't got a pointer,
>> - take the reference, drop ref to old tile group */
>> - if (connector->tile_group) {
>> - drm_mode_put_tile_group(connector->dev, connector->tile_group);
>> - }
>> - connector->tile_group = tg;
>> - } else
>> - /* if same tile group, then release the ref we just took. */
>> - drm_mode_put_tile_group(connector->dev, tg);
>> - }
>> - break;
>> + case DATA_BLOCK_TILED_DISPLAY:
>> + ret = drm_parse_tiled_block(connector, block);
>> + if (ret)
>> + return ret;
break; here.
>> default:
>> printk("unknown displayid tag %d\n", block->tag);
>> break;
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread