On Tue, Feb 27, 2018 at 02:56:54PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Now that we have split the display info into static and dynamic parts,
> we can just zero out the entire dynamic part with memset(). Previously
> we were just clearing parts of it, leaving stale data in other parts
> (eg. HDMI SCDC capabilities).
>
> Also when the edid is NULL drm_add_edid_modes() bails out early skipping
> the call to drm_add_display_info(). Thus we would again leave stale
> data behind. To avoid that let's clear out the display info at the
> very start of drm_add_edid_modes().
>
> Cc: Keith Packard <keithp@xxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
I like the idea of this, but I think we need to refine it a bit. What
about only doing this for external screens, but not panels? That would
avoid a lot of surprises (there's really no need to reset the display info
for fixed panels ever), and also avoid the need for the first 3 patches in
your series.
-Daniel
> ---
> drivers/gpu/drm/drm_connector.c | 3 +--
> drivers/gpu/drm/drm_edid.c | 23 +++--------------------
> 2 files changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index d73e97ed7dff..ddd7d978f462 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1389,10 +1389,9 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector,
> * duplicate it rather than attempt to ensure some arbitrary
> * ordering of calls.
> */
> + drm_reset_display_info(connector);
> if (edid)
> drm_add_display_info(connector, edid);
> - else
> - drm_reset_display_info(connector);
>
> drm_object_property_set_value(&connector->base,
> dev->mode_config.non_desktop_property,
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 788fee4b4bf9..78c1f37be3db 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4443,37 +4443,18 @@ drm_reset_display_info(struct drm_connector *connector)
> {
> struct drm_display_info *info = &connector->display_info;
>
> - info->width_mm = 0;
> - info->height_mm = 0;
> -
> - info->bpc = 0;
> - info->color_formats = 0;
> - info->cea_rev = 0;
> - info->max_tmds_clock = 0;
> - info->dvi_dual = false;
> - info->has_hdmi_infoframe = false;
> -
> - info->non_desktop = 0;
> + memset(info, 0, sizeof(*info));
> }
> EXPORT_SYMBOL_GPL(drm_reset_display_info);
>
> u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid)
> {
> struct drm_display_info *info = &connector->display_info;
> -
> u32 quirks = edid_get_quirks(edid);
>
> info->width_mm = edid->width_cm * 10;
> info->height_mm = edid->height_cm * 10;
>
> - /* driver figures it out in this case */
> - info->bpc = 0;
> - info->color_formats = 0;
> - info->cea_rev = 0;
> - info->max_tmds_clock = 0;
> - info->dvi_dual = false;
> - info->has_hdmi_infoframe = false;
> -
> info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
>
> DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop);
> @@ -4684,6 +4665,8 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> int num_modes = 0;
> u32 quirks;
>
> + drm_reset_display_info(connector);
> +
> if (edid == NULL) {
> clear_eld(connector);
> return 0;
> --
> 2.13.6
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
