| Message ID | 20200527130310.27099-2-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/edid: Allow looking for ext blocks starting from a specified index | expand |
On Wed, 2020-05-27 at 16:03 +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Apparently there are EDIDs in the wild with multiple DispID extension > blocks. Iterate through them all. > > In one particular case the tile information is specicied in the > second DispID ext block, and since the current parser only looks > at the first DispID ext block we don't notice that we're dealing > with a tiled display. > > While at it change a few functions to return void since we have > no use for the errno. With the change in the previous patch: Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > References: https://gitlab.freedesktop.org/drm/intel/-/issues/27 > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/drm_edid.c | 84 +++++++++++++++++--------------------- > 1 file changed, 38 insertions(+), 46 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index f2531d51dfa2..dcb23563d29d 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -3248,6 +3248,7 @@ static u8 *drm_find_cea_extension(const struct edid *edid) > int ext_index; > > /* Look for a top level CEA extension block */ > + /* FIXME: make callers iterate through multiple CEA ext blocks? */ > ext_index = 0; > cea = drm_find_edid_extension(edid, CEA_EXT, &ext_index); > if (cea) > @@ -3255,20 +3256,20 @@ static u8 *drm_find_cea_extension(const struct edid *edid) > > /* CEA blocks can also be found embedded in a DisplayID block */ > ext_index = 0; > - displayid = drm_find_displayid_extension(edid, &length, &idx, > - &ext_index); > - if (!displayid) > - return NULL; > + for (;;) { > + displayid = drm_find_displayid_extension(edid, &length, &idx, > + &ext_index); > + if (!displayid) > + return NULL; > > - idx += sizeof(struct displayid_hdr); > - for_each_displayid_db(displayid, block, idx, length) { > - if (block->tag == DATA_BLOCK_CTA) { > - cea = (u8 *)block; > - break; > + idx += sizeof(struct displayid_hdr); > + for_each_displayid_db(displayid, block, idx, length) { > + if (block->tag == DATA_BLOCK_CTA) > + return (u8 *)block; > } > } > > - return cea; > + return NULL; > } > > static __always_inline const struct drm_display_mode *cea_mode_for_vic(u8 vic) > @@ -5205,19 +5206,22 @@ static int add_displayid_detailed_modes(struct drm_connector *connector, > int num_modes = 0; > int ext_index = 0; > > - displayid = drm_find_displayid_extension(edid, &length, &idx, > - &ext_index); > - if (!displayid) > - return 0; > - > - idx += sizeof(struct displayid_hdr); > - for_each_displayid_db(displayid, block, idx, length) { > - switch (block->tag) { > - case DATA_BLOCK_TYPE_1_DETAILED_TIMING: > - num_modes += add_displayid_detailed_1_modes(connector, block); > + for (;;) { > + displayid = drm_find_displayid_extension(edid, &length, &idx, > + &ext_index); > + if (!displayid) > break; > + > + idx += sizeof(struct displayid_hdr); > + for_each_displayid_db(displayid, block, idx, length) { > + switch (block->tag) { > + case DATA_BLOCK_TYPE_1_DETAILED_TIMING: > + num_modes += add_displayid_detailed_1_modes(connector, block); > + break; > + } > } > } > + > return num_modes; > } > > @@ -5797,8 +5801,8 @@ 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, > - const struct displayid_block *block) > +static void drm_parse_tiled_block(struct drm_connector *connector, > + const struct displayid_block *block) > { > const struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block; > u16 w, h; > @@ -5836,7 +5840,7 @@ static int drm_parse_tiled_block(struct drm_connector *connector, > tg = drm_mode_create_tile_group(connector->dev, tile->topology_id); > } > if (!tg) > - return -ENOMEM; > + return; > > if (connector->tile_group != tg) { > /* if we haven't got a pointer, > @@ -5848,14 +5852,12 @@ static int drm_parse_tiled_block(struct drm_connector *connector, > } 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_displayid_parse_tiled(struct drm_connector *connector, > - const u8 *displayid, int length, int idx) > +static void drm_displayid_parse_tiled(struct drm_connector *connector, > + const u8 *displayid, int length, int idx) > { > const struct displayid_block *block; > - int ret; > > idx += sizeof(struct displayid_hdr); > for_each_displayid_db(displayid, block, idx, length) { > @@ -5864,16 +5866,13 @@ static int drm_displayid_parse_tiled(struct drm_connector *connector, > > switch (block->tag) { > case DATA_BLOCK_TILED_DISPLAY: > - ret = drm_parse_tiled_block(connector, block); > - if (ret) > - return ret; > + drm_parse_tiled_block(connector, block); > break; > default: > DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n", block->tag); > break; > } > } > - return 0; > } > > void drm_update_tile_info(struct drm_connector *connector, > @@ -5882,26 +5881,19 @@ void drm_update_tile_info(struct drm_connector *connector, > const void *displayid = NULL; > int ext_index = 0; > int length, idx; > - int ret; > > connector->has_tile = false; > - displayid = drm_find_displayid_extension(edid, &length, &idx, > - &ext_index); > - if (!displayid) { > - /* drop reference to any tile group we had */ > - goto out_drop_ref; > + for (;;) { > + displayid = drm_find_displayid_extension(edid, &length, &idx, > + &ext_index); > + if (!displayid) > + break; > + > + drm_displayid_parse_tiled(connector, displayid, length, idx); > } > > - ret = drm_displayid_parse_tiled(connector, displayid, length, idx); > - if (ret < 0) > - goto out_drop_ref; > - if (!connector->has_tile) > - goto out_drop_ref; > - return; > -out_drop_ref: > - if (connector->tile_group) { > + if (!connector->has_tile && connector->tile_group) { > drm_mode_put_tile_group(connector->dev, connector->tile_group); > connector->tile_group = NULL; > } > - return; > }
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f2531d51dfa2..dcb23563d29d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3248,6 +3248,7 @@ static u8 *drm_find_cea_extension(const struct edid *edid) int ext_index; /* Look for a top level CEA extension block */ + /* FIXME: make callers iterate through multiple CEA ext blocks? */ ext_index = 0; cea = drm_find_edid_extension(edid, CEA_EXT, &ext_index); if (cea) @@ -3255,20 +3256,20 @@ static u8 *drm_find_cea_extension(const struct edid *edid) /* CEA blocks can also be found embedded in a DisplayID block */ ext_index = 0; - displayid = drm_find_displayid_extension(edid, &length, &idx, - &ext_index); - if (!displayid) - return NULL; + for (;;) { + displayid = drm_find_displayid_extension(edid, &length, &idx, + &ext_index); + if (!displayid) + return NULL; - idx += sizeof(struct displayid_hdr); - for_each_displayid_db(displayid, block, idx, length) { - if (block->tag == DATA_BLOCK_CTA) { - cea = (u8 *)block; - break; + idx += sizeof(struct displayid_hdr); + for_each_displayid_db(displayid, block, idx, length) { + if (block->tag == DATA_BLOCK_CTA) + return (u8 *)block; } } - return cea; + return NULL; } static __always_inline const struct drm_display_mode *cea_mode_for_vic(u8 vic) @@ -5205,19 +5206,22 @@ static int add_displayid_detailed_modes(struct drm_connector *connector, int num_modes = 0; int ext_index = 0; - displayid = drm_find_displayid_extension(edid, &length, &idx, - &ext_index); - if (!displayid) - return 0; - - idx += sizeof(struct displayid_hdr); - for_each_displayid_db(displayid, block, idx, length) { - switch (block->tag) { - case DATA_BLOCK_TYPE_1_DETAILED_TIMING: - num_modes += add_displayid_detailed_1_modes(connector, block); + for (;;) { + displayid = drm_find_displayid_extension(edid, &length, &idx, + &ext_index); + if (!displayid) break; + + idx += sizeof(struct displayid_hdr); + for_each_displayid_db(displayid, block, idx, length) { + switch (block->tag) { + case DATA_BLOCK_TYPE_1_DETAILED_TIMING: + num_modes += add_displayid_detailed_1_modes(connector, block); + break; + } } } + return num_modes; } @@ -5797,8 +5801,8 @@ 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, - const struct displayid_block *block) +static void drm_parse_tiled_block(struct drm_connector *connector, + const struct displayid_block *block) { const struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block; u16 w, h; @@ -5836,7 +5840,7 @@ static int drm_parse_tiled_block(struct drm_connector *connector, tg = drm_mode_create_tile_group(connector->dev, tile->topology_id); } if (!tg) - return -ENOMEM; + return; if (connector->tile_group != tg) { /* if we haven't got a pointer, @@ -5848,14 +5852,12 @@ static int drm_parse_tiled_block(struct drm_connector *connector, } 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_displayid_parse_tiled(struct drm_connector *connector, - const u8 *displayid, int length, int idx) +static void drm_displayid_parse_tiled(struct drm_connector *connector, + const u8 *displayid, int length, int idx) { const struct displayid_block *block; - int ret; idx += sizeof(struct displayid_hdr); for_each_displayid_db(displayid, block, idx, length) { @@ -5864,16 +5866,13 @@ static int drm_displayid_parse_tiled(struct drm_connector *connector, switch (block->tag) { case DATA_BLOCK_TILED_DISPLAY: - ret = drm_parse_tiled_block(connector, block); - if (ret) - return ret; + drm_parse_tiled_block(connector, block); break; default: DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n", block->tag); break; } } - return 0; } void drm_update_tile_info(struct drm_connector *connector, @@ -5882,26 +5881,19 @@ void drm_update_tile_info(struct drm_connector *connector, const void *displayid = NULL; int ext_index = 0; int length, idx; - int ret; connector->has_tile = false; - displayid = drm_find_displayid_extension(edid, &length, &idx, - &ext_index); - if (!displayid) { - /* drop reference to any tile group we had */ - goto out_drop_ref; + for (;;) { + displayid = drm_find_displayid_extension(edid, &length, &idx, + &ext_index); + if (!displayid) + break; + + drm_displayid_parse_tiled(connector, displayid, length, idx); } - ret = drm_displayid_parse_tiled(connector, displayid, length, idx); - if (ret < 0) - goto out_drop_ref; - if (!connector->has_tile) - goto out_drop_ref; - return; -out_drop_ref: - if (connector->tile_group) { + if (!connector->has_tile && connector->tile_group) { drm_mode_put_tile_group(connector->dev, connector->tile_group); connector->tile_group = NULL; } - return; }