Re: [Intel-gfx] [PATCH v7 04/22] drm/edid: Use the pre-parsed VICs
On Wed, Jan 04, 2023 at 12:05:19PM +0200, Jani Nikula wrote:
> Now that we have all the VICs in info->vics, use them to simplify access
> based on VIC index, i.e. on the order of VICs in the EDID, and avoid
> passing CTA VDB pointers around.
>
> This also fixes the highly unlikely scenarios of a) multiple HDMI VSDBs,
> and b) HDMI VSDB 3D modes using VIC indexes that span across multiple
> CTA VDBs, and the combination of the two.
>
> Cc: Ville Syrjälä
> Signed-off-by: Jani Nikula
Cool. Takes care of my previously stated concern of
multiple CTA/HDMI data blocks.
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/drm_edid.c | 92 +-
> 1 file changed, 32 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 90846dc69061..7f0386175230 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4468,28 +4468,20 @@ static u8 svd_to_vic(u8 svd)
> return svd;
> }
>
> +/*
> + * Return a display mode for the 0-based vic_index'th VIC across all CTA
> VDBs in
> + * the EDID, or NULL on errors.
> + */
> static struct drm_display_mode *
> -drm_display_mode_from_vic_index(struct drm_connector *connector,
> - const u8 *video_db, u8 video_len,
> - u8 video_index)
> +drm_display_mode_from_vic_index(struct drm_connector *connector, int
> vic_index)
> {
> + const struct drm_display_info *info = >display_info;
> struct drm_device *dev = connector->dev;
> - struct drm_display_mode *newmode;
> - u8 vic;
>
> - if (video_db == NULL || video_index >= video_len)
> + if (!info->vics || vic_index >= info->vics_len ||
> !info->vics[vic_index])
> return NULL;
>
> - /* CEA modes are numbered 1..127 */
> - vic = svd_to_vic(video_db[video_index]);
> - if (!drm_valid_cea_vic(vic))
> - return NULL;
> -
> - newmode = drm_mode_duplicate(dev, cea_mode_for_vic(vic));
> - if (!newmode)
> - return NULL;
> -
> - return newmode;
> + return drm_display_mode_from_cea_vic(dev, info->vics[vic_index]);
> }
>
> /*
> @@ -4538,9 +4530,8 @@ static int do_y420vdb_modes(struct drm_connector
> *connector,
> * Makes an entry for a videomode in the YCBCR 420 bitmap
> */
> static void
> -drm_add_cmdb_modes(struct drm_connector *connector, u8 svd)
> +drm_add_cmdb_modes(struct drm_connector *connector, u8 vic)
> {
> - u8 vic = svd_to_vic(svd);
> struct drm_hdmi_info *hdmi = >display_info.hdmi;
>
> if (!drm_valid_cea_vic(vic))
> @@ -4577,16 +4568,20 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_display_mode_from_cea_vic);
>
> -static int
> -do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
> +/* Add modes based on VICs parsed in parse_cta_vdb() */
> +static int add_cta_vdb_modes(struct drm_connector *connector)
> {
> + const struct drm_display_info *info = >display_info;
> + const struct drm_hdmi_info *hdmi = >hdmi;
> int i, modes = 0;
> - struct drm_hdmi_info *hdmi = >display_info.hdmi;
>
> - for (i = 0; i < len; i++) {
> + if (!info->vics)
> + return 0;
> +
> + for (i = 0; i < info->vics_len; i++) {
> struct drm_display_mode *mode;
>
> - mode = drm_display_mode_from_vic_index(connector, db, len, i);
> + mode = drm_display_mode_from_vic_index(connector, i);
> if (mode) {
> /*
>* YCBCR420 capability block contains a bitmap which
> @@ -4598,7 +4593,7 @@ do_cea_modes(struct drm_connector *connector, const u8
> *db, u8 len)
>* Add YCBCR420 modes only if sink is HDMI 2.0 capable.
>*/
> if (i < 64 && hdmi->y420_cmdb_map & (1ULL << i))
> - drm_add_cmdb_modes(connector, db[i]);
> + drm_add_cmdb_modes(connector, info->vics[i]);
>
> drm_mode_probed_add(connector, mode);
> modes++;
> @@ -4693,15 +4688,13 @@ static int add_hdmi_mode(struct drm_connector
> *connector, u8 vic)
> }
>
> static int add_3d_struct_modes(struct drm_connector *connector, u16
> structure,
> -const u8 *video_db, u8 video_len, u8 video_index)
> +int vic_index)
> {
> struct drm_display_mode *newmode;
> int modes = 0;
>
> if (structure & (1 << 0)) {
> - newmode = drm_display_mode_from_vic_index(connector, video_db,
> - video_len,
> - video_index);
> + newmode = drm_display_mode_from_vic_index(connector, vic_index);
> if (newmode) {
> newmode->flags |=
[Intel-gfx] [PATCH v7 04/22] drm/edid: Use the pre-parsed VICs
Now that we have all the VICs in info->vics, use them to simplify access
based on VIC index, i.e. on the order of VICs in the EDID, and avoid
passing CTA VDB pointers around.
This also fixes the highly unlikely scenarios of a) multiple HDMI VSDBs,
and b) HDMI VSDB 3D modes using VIC indexes that span across multiple
CTA VDBs, and the combination of the two.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 92 +-
1 file changed, 32 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 90846dc69061..7f0386175230 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4468,28 +4468,20 @@ static u8 svd_to_vic(u8 svd)
return svd;
}
+/*
+ * Return a display mode for the 0-based vic_index'th VIC across all CTA VDBs
in
+ * the EDID, or NULL on errors.
+ */
static struct drm_display_mode *
-drm_display_mode_from_vic_index(struct drm_connector *connector,
- const u8 *video_db, u8 video_len,
- u8 video_index)
+drm_display_mode_from_vic_index(struct drm_connector *connector, int vic_index)
{
+ const struct drm_display_info *info = >display_info;
struct drm_device *dev = connector->dev;
- struct drm_display_mode *newmode;
- u8 vic;
- if (video_db == NULL || video_index >= video_len)
+ if (!info->vics || vic_index >= info->vics_len ||
!info->vics[vic_index])
return NULL;
- /* CEA modes are numbered 1..127 */
- vic = svd_to_vic(video_db[video_index]);
- if (!drm_valid_cea_vic(vic))
- return NULL;
-
- newmode = drm_mode_duplicate(dev, cea_mode_for_vic(vic));
- if (!newmode)
- return NULL;
-
- return newmode;
+ return drm_display_mode_from_cea_vic(dev, info->vics[vic_index]);
}
/*
@@ -4538,9 +4530,8 @@ static int do_y420vdb_modes(struct drm_connector
*connector,
* Makes an entry for a videomode in the YCBCR 420 bitmap
*/
static void
-drm_add_cmdb_modes(struct drm_connector *connector, u8 svd)
+drm_add_cmdb_modes(struct drm_connector *connector, u8 vic)
{
- u8 vic = svd_to_vic(svd);
struct drm_hdmi_info *hdmi = >display_info.hdmi;
if (!drm_valid_cea_vic(vic))
@@ -4577,16 +4568,20 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_display_mode_from_cea_vic);
-static int
-do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len)
+/* Add modes based on VICs parsed in parse_cta_vdb() */
+static int add_cta_vdb_modes(struct drm_connector *connector)
{
+ const struct drm_display_info *info = >display_info;
+ const struct drm_hdmi_info *hdmi = >hdmi;
int i, modes = 0;
- struct drm_hdmi_info *hdmi = >display_info.hdmi;
- for (i = 0; i < len; i++) {
+ if (!info->vics)
+ return 0;
+
+ for (i = 0; i < info->vics_len; i++) {
struct drm_display_mode *mode;
- mode = drm_display_mode_from_vic_index(connector, db, len, i);
+ mode = drm_display_mode_from_vic_index(connector, i);
if (mode) {
/*
* YCBCR420 capability block contains a bitmap which
@@ -4598,7 +4593,7 @@ do_cea_modes(struct drm_connector *connector, const u8
*db, u8 len)
* Add YCBCR420 modes only if sink is HDMI 2.0 capable.
*/
if (i < 64 && hdmi->y420_cmdb_map & (1ULL << i))
- drm_add_cmdb_modes(connector, db[i]);
+ drm_add_cmdb_modes(connector, info->vics[i]);
drm_mode_probed_add(connector, mode);
modes++;
@@ -4693,15 +4688,13 @@ static int add_hdmi_mode(struct drm_connector
*connector, u8 vic)
}
static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
- const u8 *video_db, u8 video_len, u8 video_index)
+ int vic_index)
{
struct drm_display_mode *newmode;
int modes = 0;
if (structure & (1 << 0)) {
- newmode = drm_display_mode_from_vic_index(connector, video_db,
- video_len,
- video_index);
+ newmode = drm_display_mode_from_vic_index(connector, vic_index);
if (newmode) {
newmode->flags |= DRM_MODE_FLAG_3D_FRAME_PACKING;
drm_mode_probed_add(connector, newmode);
@@ -4709,9 +4702,7 @@ static int add_3d_struct_modes(struct drm_connector
*connector, u16 structure,
}
}
if (structure & (1 << 6)) {
- newmode = drm_display_mode_from_vic_index(connector, video_db,
-
