On 02/18/2013 03:01 PM, Thierry Reding wrote: > On Mon, Feb 18, 2013 at 02:55:04PM +0800, Mark Zhang wrote: >> On 02/18/2013 02:40 PM, Thierry Reding wrote: >>> On Mon, Feb 18, 2013 at 02:06:56PM +0800, Mark Zhang wrote: >>>> 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. >>> >>> Why would we want to do that? I don't think doing so would even work, >>> since the planes are resources that need to be registered with the DRM >>> core and therefore can't be allocated on-demand. >>> >> >> I'm wondering if we add power management codes in the future, the >> crtc/plane will be disabled/enabled frequently, e.g: system going to >> suspend state then resume. > > In that case it makes even less sense to allocate and deallocate the > plane every time around. However, any optimizations aside, the allocated > memory is passed into the core via drm_plane_init(), which registers > that plane with the given CRTC. So if we deallocate the memory when the > plane when it is disabled, we'd have to make sure to remove it from the > core as well. However that would also mean that it disappears from the > list of planes supported by the CRTC and therefore we wouldn't be able > to use it again. >
Alright, I mixed up the "disable" & "remove" of planes and crtcs. You're right. Just forget what I said in this thread. Mark > Thierry > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel