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

[Thierry Reding]

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.reding at 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.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130218/ced5735c/attachment.pgp>