[PATCH weston 6/7] compositor-drm: Support creating drm_fb backed by dumb buffers

[Ander Conselvan de Oliveira]

On 01/23/2013 11:50 AM, Pekka Paalanen wrote:
> On Tue, 22 Jan 2013 18:07:14 +0200
> Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
> wrote:
>
>> ---
>>   src/compositor-drm.c |  106 +++++++++++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 96 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
>> index 4868db0..7c11af1 100644
>> --- a/src/compositor-drm.c
>> +++ b/src/compositor-drm.c
>> @@ -30,6 +30,7 @@
>>   #include <unistd.h>
>>   #include <linux/input.h>
>>   #include <assert.h>
>> +#include <sys/mman.h>
>>
>>   #include <xf86drm.h>
>>   #include <xf86drmMode.h>
>> @@ -116,9 +117,12 @@ struct drm_output;
>>   struct drm_fb {
>>   	struct gbm_bo *bo;
>>   	struct drm_output *output;
>> +	int fd;
>>   	uint32_t fb_id;
>>   	int is_client_buffer;
>>   	struct weston_buffer_reference buffer_ref;
>> +	uint32_t stride, handle, size;
>> +	void *map;
>>   };
>
> Just for clarity, the new fields in struct drm_fb are only ever used
> for dumb buffers, right?

Fields fd, handle, stride and size are valid for both. Only map is 
exclusive for dumb buffers. Granted most of them might not be used for 
the gbm case.

I know stride might be necessary in general, since we can't change the 
stride of the scan out bo with the page flip ioctl with i915, so if we 
have a dumb buffer and want to page flip to a gbm bo, we need to check
if the stride is different and if it is, do a SetCrtc() instead.

> Regrouping all fields into three by used for both, used only for GBM
> buffers, and used only for dumb buffers might be nice. Or maybe even an
> anonymous embedded struct.

I think this is just map for dumb and bo for gbm.


>> @@ -213,27 +217,106 @@ drm_fb_destroy_callback(struct gbm_bo *bo, void *data)

[...]

>> +static void
>> +drm_fb_destroy_dumb(struct drm_fb *fb)
>> +{
>> +	struct drm_mode_destroy_dumb destroy_arg;
>> +
>> +	if (!fb->map)
>> +		return;
>> +
>> +	if (fb->fb_id)
>> +		drmModeRmFB(fb->fd, fb->fb_id);
>> +
>> +	munmap(fb->map, fb->size);
>> +
>> +	memset(&destroy_arg, 0, sizeof(destroy_arg));
>> +	destroy_arg.handle = fb->handle;
>> +	drmIoctl(fb->fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_arg);
>> +
>
> What about fb->buffer_ref? Is it only for GBM case, where a client
> buffer is scanned out directly, and so impossible for dumb buffers?

That's the case right now, but nothing prevents someone from calling 
drm_fb_set_buffer() with a dumb buffer for whatever reason. I think we 
should check for that here too. Good catch!


Thanks,
Ander
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.