Re: [PATCH 4/4] drm: remove the raw_edid field from struct drm_display_info

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Acked-by: Inki Dae <inki.dae@xxxxxxxxxxx>

2012/8/15 Jani Nikula <jani.nikula@xxxxxxxxx>:
> Neither the drm core nor any of the drivers really need the raw_edid field
> of struct drm_display_info for anything. Instead of being useful, it
> creates confusion about who is responsible for freeing the memory it points
> to and setting the field to NULL afterwards, leading to memory leaks and
> dangling pointers.
>
> Remove the raw_edid field, and fix drivers as necessary.
>
> Reported-by: Russell King <linux@xxxxxxxxxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_edid.c                    |    3 ---
>  drivers/gpu/drm/drm_edid_load.c               |   23 +++++++++++++----------
>  drivers/gpu/drm/exynos/exynos_drm_connector.c |    4 +---
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c      |   13 -------------
>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c       |    2 --
>  drivers/gpu/drm/gma500/oaktrail_hdmi.c        |    1 -
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c       |    3 ---
>  drivers/gpu/drm/i915/intel_dp.c               |    4 ----
>  drivers/gpu/drm/i915/intel_hdmi.c             |    3 ---
>  drivers/gpu/drm/i915/intel_modes.c            |    1 -
>  drivers/gpu/drm/i915/intel_sdvo.c             |    3 ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c        |    1 -
>  drivers/gpu/drm/udl/udl_connector.c           |    3 ---
>  drivers/staging/omapdrm/omap_connector.c      |    5 +----
>  include/drm/drm_crtc.h                        |    2 --
>  15 files changed, 15 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index a8743c3..bcc4725 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -399,10 +399,7 @@ struct edid *drm_get_edid(struct drm_connector *connector,
>         if (drm_probe_ddc(adapter))
>                 edid = (struct edid *)drm_do_get_edid(connector, adapter);
>
> -       connector->display_info.raw_edid = (char *)edid;
> -
>         return edid;
> -
>  }
>  EXPORT_SYMBOL(drm_get_edid);
>
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 0303935..186832e 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -114,8 +114,8 @@ static u8 generic_edid[GENERIC_EDIDS][128] = {
>         },
>  };
>
> -static int edid_load(struct drm_connector *connector, char *name,
> -                    char *connector_name)
> +static u8 *edid_load(struct drm_connector *connector, char *name,
> +                       char *connector_name)
>  {
>         const struct firmware *fw;
>         struct platform_device *pdev;
> @@ -205,7 +205,6 @@ static int edid_load(struct drm_connector *connector, char *name,
>                 edid = new_edid;
>         }
>
> -       connector->display_info.raw_edid = edid;
>         DRM_INFO("Got %s EDID base block and %d extension%s from "
>             "\"%s\" for connector \"%s\"\n", builtin ? "built-in" :
>             "external", valid_extensions, valid_extensions == 1 ? "" : "s",
> @@ -215,7 +214,10 @@ relfw_out:
>         release_firmware(fw);
>
>  out:
> -       return err;
> +       if (err)
> +               return ERR_PTR(err);
> +
> +       return edid;
>  }
>
>  int drm_load_edid_firmware(struct drm_connector *connector)
> @@ -223,6 +225,7 @@ int drm_load_edid_firmware(struct drm_connector *connector)
>         char *connector_name = drm_get_connector_name(connector);
>         char *edidname = edid_firmware, *last, *colon;
>         int ret;
> +       struct edid *edid;
>
>         if (*edidname == '\0')
>                 return 0;
> @@ -240,13 +243,13 @@ int drm_load_edid_firmware(struct drm_connector *connector)
>         if (*last == '\n')
>                 *last = '\0';
>
> -       ret = edid_load(connector, edidname, connector_name);
> -       if (ret)
> +       edid = (struct edid *) edid_load(connector, edidname, connector_name);
> +       if (IS_ERR_OR_NULL(edid))
>                 return 0;
>
> -       drm_mode_connector_update_edid_property(connector,
> -           (struct edid *) connector->display_info.raw_edid);
> +       drm_mode_connector_update_edid_property(connector, edid);
> +       ret = drm_add_edid_modes(connector, edid);
> +       kfree(edid);
>
> -       return drm_add_edid_modes(connector, (struct edid *)
> -           connector->display_info.raw_edid);
> +       return ret;
>  }
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c
> index d956819..9dce3b9 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_connector.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c
> @@ -147,9 +147,7 @@ static int exynos_drm_connector_get_modes(struct drm_connector *connector)
>
>                 drm_mode_connector_update_edid_property(connector, edid);
>                 count = drm_add_edid_modes(connector, edid);
> -
> -               kfree(connector->display_info.raw_edid);
> -               connector->display_info.raw_edid = edid;
> +               kfree(edid);
>         } else {
>                 struct drm_display_mode *mode = drm_mode_create(connector->dev);
>                 struct exynos_drm_panel_info *panel;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> index bb1550c..9239525 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
> @@ -102,7 +102,6 @@ static int vidi_get_edid(struct device *dev, struct drm_connector *connector,
>                                 u8 *edid, int len)
>  {
>         struct vidi_context *ctx = get_vidi_context(dev);
> -       struct edid *raw_edid;
>
>         DRM_DEBUG_KMS("%s\n", __FILE__);
>
> @@ -115,18 +114,6 @@ static int vidi_get_edid(struct device *dev, struct drm_connector *connector,
>                 return -EFAULT;
>         }
>
> -       raw_edid = kzalloc(len, GFP_KERNEL);
> -       if (!raw_edid) {
> -               DRM_DEBUG_KMS("failed to allocate raw_edid.\n");
> -               return -ENOMEM;
> -       }
> -
> -       memcpy(raw_edid, ctx->raw_edid, min((1 + ctx->raw_edid->extensions)
> -                                               * EDID_LENGTH, len));
> -
> -       /* attach the edid data to connector. */
> -       connector->display_info.raw_edid = (char *)raw_edid;
> -
>         memcpy(edid, ctx->raw_edid, min((1 + ctx->raw_edid->extensions)
>                                         * EDID_LENGTH, len));
>
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
> index a86f87b..9db0ef1 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
> @@ -157,8 +157,6 @@ static enum drm_connector_status cdv_hdmi_detect(
>                         hdmi_priv->has_hdmi_audio =
>                                                 drm_detect_monitor_audio(edid);
>                 }
> -
> -               psb_intel_connector->base.display_info.raw_edid = NULL;
>                 kfree(edid);
>         }
>         return status;
> diff --git a/drivers/gpu/drm/gma500/oaktrail_hdmi.c b/drivers/gpu/drm/gma500/oaktrail_hdmi.c
> index 2eb3dc4..69e51e9 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_hdmi.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_hdmi.c
> @@ -252,7 +252,6 @@ static int oaktrail_hdmi_get_modes(struct drm_connector *connector)
>         if (edid) {
>                 drm_mode_connector_update_edid_property(connector, edid);
>                 ret = drm_add_edid_modes(connector, edid);
> -               connector->display_info.raw_edid = NULL;
>         }
>
>         /*
> diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> index 0466c7b..a453d94 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> @@ -1343,7 +1343,6 @@ psb_intel_sdvo_hdmi_sink_detect(struct drm_connector *connector)
>                         }
>                 } else
>                         status = connector_status_disconnected;
> -               connector->display_info.raw_edid = NULL;
>                 kfree(edid);
>         }
>
> @@ -1404,7 +1403,6 @@ psb_intel_sdvo_detect(struct drm_connector *connector, bool force)
>                                 ret = connector_status_disconnected;
>                         else
>                                 ret = connector_status_connected;
> -                       connector->display_info.raw_edid = NULL;
>                         kfree(edid);
>                 } else
>                         ret = connector_status_connected;
> @@ -1453,7 +1451,6 @@ static void psb_intel_sdvo_get_ddc_modes(struct drm_connector *connector)
>                         drm_add_edid_modes(connector, edid);
>                 }
>
> -               connector->display_info.raw_edid = NULL;
>                 kfree(edid);
>         }
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0a56b9a..bffd8cc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2160,7 +2160,6 @@ intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *ada
>                 ret = drm_add_edid_modes(connector, intel_dp->edid);
>                 drm_edid_to_eld(connector,
>                                 intel_dp->edid);
> -               connector->display_info.raw_edid = NULL;
>                 return intel_dp->edid_mode_count;
>         }
>
> @@ -2206,7 +2205,6 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>                 edid = intel_dp_get_edid(connector, &intel_dp->adapter);
>                 if (edid) {
>                         intel_dp->has_audio = drm_detect_monitor_audio(edid);
> -                       connector->display_info.raw_edid = NULL;
>                         kfree(edid);
>                 }
>         }
> @@ -2271,8 +2269,6 @@ intel_dp_detect_audio(struct drm_connector *connector)
>         edid = intel_dp_get_edid(connector, &intel_dp->adapter);
>         if (edid) {
>                 has_audio = drm_detect_monitor_audio(edid);
> -
> -               connector->display_info.raw_edid = NULL;
>                 kfree(edid);
>         }
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 98f6024..6a29f72 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -737,7 +737,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>                                                 drm_detect_hdmi_monitor(edid);
>                         intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
>                 }
> -               connector->display_info.raw_edid = NULL;
>                 kfree(edid);
>         }
>
> @@ -778,8 +777,6 @@ intel_hdmi_detect_audio(struct drm_connector *connector)
>         if (edid) {
>                 if (edid->input & DRM_EDID_INPUT_DIGITAL)
>                         has_audio = drm_detect_monitor_audio(edid);
> -
> -               connector->display_info.raw_edid = NULL;
>                 kfree(edid);
>         }
>
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index 45848b9..7a5238f 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -50,7 +50,6 @@ int intel_ddc_get_modes(struct drm_connector *connector,
>                 drm_mode_connector_update_edid_property(connector, edid);
>                 ret = drm_add_edid_modes(connector, edid);
>                 drm_edid_to_eld(connector, edid);
> -               connector->display_info.raw_edid = NULL;
>                 kfree(edid);
>         }
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index d81bb0b..7dad271 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1345,7 +1345,6 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
>                         }
>                 } else
>                         status = connector_status_disconnected;
> -               connector->display_info.raw_edid = NULL;
>                 kfree(edid);
>         }
>
> @@ -1419,7 +1418,6 @@ intel_sdvo_detect(struct drm_connector *connector, bool force)
>                         else
>                                 ret = connector_status_disconnected;
>
> -                       connector->display_info.raw_edid = NULL;
>                         kfree(edid);
>                 } else
>                         ret = connector_status_connected;
> @@ -1465,7 +1463,6 @@ static void intel_sdvo_get_ddc_modes(struct drm_connector *connector)
>                         drm_add_edid_modes(connector, edid);
>                 }
>
> -               connector->display_info.raw_edid = NULL;
>                 kfree(edid);
>         }
>  }
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index b69642d..c7420e8 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1399,7 +1399,6 @@ static int mga_vga_get_modes(struct drm_connector *connector)
>         if (edid) {
>                 drm_mode_connector_update_edid_property(connector, edid);
>                 ret = drm_add_edid_modes(connector, edid);
> -               connector->display_info.raw_edid = NULL;
>                 kfree(edid);
>         }
>         return ret;
> diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
> index ba055e9..2d98ff9 100644
> --- a/drivers/gpu/drm/udl/udl_connector.c
> +++ b/drivers/gpu/drm/udl/udl_connector.c
> @@ -57,11 +57,8 @@ static int udl_get_modes(struct drm_connector *connector)
>
>         edid = (struct edid *)udl_get_edid(udl);
>
> -       connector->display_info.raw_edid = (char *)edid;
> -
>         drm_mode_connector_update_edid_property(connector, edid);
>         ret = drm_add_edid_modes(connector, edid);
> -       connector->display_info.raw_edid = NULL;
>         kfree(edid);
>         return ret;
>  }
> diff --git a/drivers/staging/omapdrm/omap_connector.c b/drivers/staging/omapdrm/omap_connector.c
> index 5e2856c..9c2287b 100644
> --- a/drivers/staging/omapdrm/omap_connector.c
> +++ b/drivers/staging/omapdrm/omap_connector.c
> @@ -177,14 +177,11 @@ static int omap_connector_get_modes(struct drm_connector *connector)
>                         drm_mode_connector_update_edid_property(
>                                         connector, edid);
>                         n = drm_add_edid_modes(connector, edid);
> -                       kfree(connector->display_info.raw_edid);
> -                       connector->display_info.raw_edid = edid;
>                 } else {
>                         drm_mode_connector_update_edid_property(
>                                         connector, NULL);
> -                       connector->display_info.raw_edid = NULL;
> -                       kfree(edid);
>                 }
> +               kfree(edid);
>         } else {
>                 struct drm_display_mode *mode = drm_mode_create(dev);
>                 struct omap_video_timings timings;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a1a0386..0522044 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -216,8 +216,6 @@ struct drm_display_info {
>         u32 color_formats;
>
>         u8 cea_rev;
> -
> -       char *raw_edid; /* if any */
>  };
>
>  struct drm_framebuffer_funcs {
> --
> 1.7.4.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux