On 02/14/2013 12:05 AM, Thierry Reding wrote:
> Add support for the B and C planes which support RGB and YUV pixel
> formats and can be used as overlays or hardware cursor. Currently
> only 32-bit RGBA pixel formats are advertised.
> 
> Signed-off-by: Thierry Reding <thierry.red...@avionic-design.de>
[...]
> +static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
> +{
> +     unsigned int i;
> +     int err = 0;
> +
> +     for (i = 0; i < 2; i++) {
> +             struct tegra_plane *plane;
> +
> +             plane = devm_kzalloc(drm->dev, sizeof(*plane), GFP_KERNEL);

Using "devm_kzalloc" here seems like not a good idea. Everytime plane
disable or crtc disable, we should free "struct tegra_plane" which is
allocated here. But the memory which devm_kzalloc allocates is only
freed when the driver detach. This makes lots of memory can't be
recycled when the plane enable/disable frequently.

> +             if (!plane)
> +                     return -ENOMEM;
> +
> +             plane->index = 1 + i;
> +
> +             err = drm_plane_init(drm, &plane->base, 1 << dc->pipe,
> +                                  &tegra_plane_funcs, plane_formats,
> +                                  ARRAY_SIZE(plane_formats), false);
> +             if (err < 0)
> +                     return err;
> +     }
> +
> +     return 0;
> +}
> +
>  static const struct drm_crtc_funcs tegra_crtc_funcs = {
>       .set_config = drm_crtc_helper_set_config,
>       .destroy = drm_crtc_cleanup,
>  };
>  
> -static void tegra_crtc_dpms(struct drm_crtc *crtc, int mode)
> +static void tegra_crtc_disable(struct drm_crtc *crtc)
>  {
> +     struct drm_device *drm = crtc->dev;
> +     struct drm_plane *plane;
> +
> +     list_for_each_entry(plane, &drm->mode_config.plane_list, head) {
> +             if (plane->crtc == crtc) {
> +                     tegra_plane_disable(plane);
> +                     plane->crtc = NULL;
> +
> +                     if (plane->fb) {
> +                             drm_framebuffer_unreference(plane->fb);
> +                             plane->fb = NULL;
> +                     }
> +             }
> +     }

If what I mentioned above(about using devm_kzalloc to allocate "struct
tegra_plane") is correct, we need to free "struct tegra_plane" here.

>  }
>  
>  static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
> @@ -46,10 +144,11 @@ static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
>       return true;
>  }
>
[...]
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to