Re: [PATCH v3 2/7] drm/tegra: Add plane support

[Mark Zhang <nvmarkzhang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

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.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
>> [...]
>>> +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.

> Thierry
>