[PATCH weston v6 10/12] drm: Pass gbm_format as enum rather than string in output config

[Pekka Paalanen]

On Mon, 18 Apr 2016 17:52:55 +0200
Benoit Gschwind <gschwind at gnu-log.net> wrote:

> Le 18/04/2016 16:50, Pekka Paalanen a écrit :
> > On Fri, 15 Apr 2016 20:28:35 -0700
> > Bryce Harrington <bryce at osg.samsung.com> wrote:
> >   
> >> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> >> ---
> >>  src/compositor-drm.c | 23 +++++++++++++----------
> >>  1 file changed, 13 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> >> index d129adc..a896785 100644
> >> --- a/src/compositor-drm.c
> >> +++ b/src/compositor-drm.c
> >> @@ -93,13 +93,13 @@ enum weston_drm_backend_output_mode {
> >>  struct weston_drm_backend_output_config {
> >>  	struct weston_backend_output_config base;
> >>  
> >> -	/** The pixel format to be used by the output. Valid values are:
> >> -	 * - NULL - The format set at backend creation time will be used
> >> -	 * - "xrgb8888"
> >> -	 * - "rgb565"
> >> -	 * - "xrgb2101010"
> >> +	/** The pixel format to be used by the output. Supported values are:
> >> +	 * - 0: The format set at backend creation time will be used
> >> +	 * - GBM_FORMAT_XRGB8888
> >> +	 * - GBM_FORMAT_RGB565
> >> +	 * - GBM_FORMAT_XRGB2101010
> >>  	 */
> >> -	char *gbm_format;
> >> +	uint32_t gbm_format;
> >>  
> >>  	/** The seat to be used by the output. Set to NULL to use the
> >>  	 * default seat. */
> >> @@ -2336,8 +2336,12 @@ drm_configure_output(struct weston_compositor *c,
> >>  			   s, name);
> >>  	free(s);
> >>  
> >> -	weston_config_section_get_string(section,
> >> -					 "gbm-format", &config->gbm_format, NULL);
> >> +	weston_config_section_get_string(section, "gbm-format", &s, NULL);
> >> +        if (parse_gbm_format(s, GBM_FORMAT_XRGB8888, &config->gbm_format) < 0)
> >> +		weston_log("Invalid gbm-format \"%s\" for output %s\n",
> >> +			   s, name);
> >> +	free(s);
> >> +
> >>  	weston_config_section_get_string(section, "seat", &config->seat, "");
> >>  	return mode;
> >>  }
> >> @@ -2391,8 +2395,7 @@ create_output_for_connector(struct drm_backend *b,
> >>  
> >>  	mode = drm_configure_output(b->compositor, b->use_current_mode,
> >>  				    output->base.name, &config);
> >> -	if (parse_gbm_format(config.gbm_format, b->gbm_format, &output->gbm_format) == -1)
> >> -		output->gbm_format = b->gbm_format;
> >> +	output->gbm_format = config.gbm_format;
> >>  
> >>  	setup_output_seat_constraint(b, &output->base,
> >>  				     config.seat ? config.seat : "");  
> > 
> > Hi,
> > 
> > we cannot do this, because the backend or anything in libweston will
> > not have access to the weston_config_*() API at all.
> > 
> > I would not like to #include <gbm.h> in main.c either, if we just can
> > reasonably avoid it.
> > 
> > Drawing the line here is vague, and we might improve later. OTOH, I
> > certainly don't want libweston to have its own enumeration of pixel
> > formats either, though time will tell if that is avoidable.
> > 
> > 
> > Thanks,
> > pq
> >   
> 
> Hello Pekka,
> 
> I think what ever you try in that case, you have 2 choice when using a
> library under your own one:
>  1. you choose to hide anything under your library
>  2. or, you leave the user to know that your are using those library and
> just include this library in your own libfoo.h
> 
> IMO, it's a waste time to try to hide library dependency. In particular
> in the case of libweston, that will be more or less a glue library.

That's true, but let's not be hasty. We don't have to do that in
this series right now.

Another common pixel format enumeration is that of Pixman's, and
that is not backend-specific, while libdrm and GBM are. But I still
would not shove Pixman formats just because of that. Wayland core
protocol also has a huge list of pixel formats that could do as
well.

The most logical would be for DRM backend to use DRM pixel formats.

I just don't want to mix that discussion into this basic conversion
series.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160418/8b32d3e3/attachment.sig>