Re: [PATCH weston 2/2] compositor-drm: Restore use-current-mode functionality

[Daniel Stone]

Hi Armin,

On 10 November 2016 at 10:20, Armin Krezović  wrote:
> On 09.11.2016 15:43, Daniel Stone wrote:
>> On 9 October 2016 at 22:48, Armin Krezović  wrote:
>>> diff --git a/compositor/main.c b/compositor/main.c
>>> index 320305c..ffeadfb 100644
>>> --- a/compositor/main.c
>>> +++ b/compositor/main.c
>>> @@ -78,6 +78,7 @@ struct wet_compositor {
>>> struct weston_config *config;
>>> struct wet_output_config *parsed_options;
>>> struct wl_listener pending_output_listener;
>>> +   bool drm_use_current_mode;
>>>  };
>>
>> I'm fairly confused about this one, though I freely admit I didn't
>> track the libweston config work, so may have missed something.
>>
>> What makes --use-current-mode special enough that it should be the
>> only such option inside struct wet_compositor? What makes it different
>> to, say, use_pixman, which lives in the DRM backend?
>
> The thing is, --use-pixman flag is used in many places throughout the backend,
> while --use-current-mode is only used by function that sets the mode. That
> function is now called by the user, and is required to be called before
> enabling the output, whereas it was previously done in the backend itself, and
> it got the necessary configuration values by calling into an user-defined 
> function
> (configure output -> user defined function -> set mode).

We were both completely right: everything you've said above is
correct, and I was right when I said that I may have missed something.
;)

To ssh://git.freedesktop.org/git/wayland/weston
   e23697f59..605ac8e68  push -> master

Thanks!

Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 2/2] compositor-drm: Restore use-current-mode functionality

[Armin Krezović]

On 10.11.2016 11:20, Armin Krezović wrote:
> On 09.11.2016 15:43, Daniel Stone wrote:
>> Hi Armin,
>>
> 
> Hi,
> 
>> On 9 October 2016 at 22:48, Armin Krezović  wrote:
>>> diff --git a/compositor/main.c b/compositor/main.c
>>> index 320305c..ffeadfb 100644
>>> --- a/compositor/main.c
>>> +++ b/compositor/main.c
>>> @@ -78,6 +78,7 @@ struct wet_compositor {
>>> struct weston_config *config;
>>> struct wet_output_config *parsed_options;
>>> struct wl_listener pending_output_listener;
>>> +   bool drm_use_current_mode;
>>>  };
>>
>> I'm fairly confused about this one, though I freely admit I didn't
>> track the libweston config work, so may have missed something.
>>
>> What makes --use-current-mode special enough that it should be the
>> only such option inside struct wet_compositor? What makes it different
>> to, say, use_pixman, which lives in the DRM backend?
>>
> 
> The thing is, --use-pixman flag is used in many places throughout the backend,
> while --use-current-mode is only used by function that sets the mode. That
> function is now called by the user, and is required to be called before
> enabling the output, whereas it was previously done in the backend itself, and
> it got the necessary configuration values by calling into an user-defined 
> function
> (configure output -> user defined function -> set mode).
> 
>>> @@ -1138,7 +1140,7 @@ drm_backend_output_configure(struct wl_listener 
>>> *listener, void *data)
>>> weston_output_disable(output);
>>> free(s);
>>> return;
>>> -   } else if (strcmp(s, "current") == 0) {
>>> +   } else if (wet->drm_use_current_mode || strcmp(s, "current") == 0) {
>>> mode = WESTON_DRM_BACKEND_OUTPUT_CURRENT;
>>> } else if (strcmp(s, "preferred") != 0) {
>>> modeline = s;
>>
>> What would the difference be to making this check be 'else if
>> (b->use_current_mode || strcmp(s, "current") == 0)'?
>>
>> Cheers,
>> Daniel
>>
> 
> struct drm_backend is private to compositor-drm.c as far as I'm aware. We 
> can't
> use it here.
> 

Ping. I'd like to know what's acceptable solution for this, to fix it
in time for next release. Otherwise, it's a regression, and
--current-mode would be unusable.



signature.asc
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 2/2] compositor-drm: Restore use-current-mode functionality

[Armin Krezović]

On 09.11.2016 15:43, Daniel Stone wrote:
> Hi Armin,
> 

Hi,

> On 9 October 2016 at 22:48, Armin Krezović  wrote:
>> diff --git a/compositor/main.c b/compositor/main.c
>> index 320305c..ffeadfb 100644
>> --- a/compositor/main.c
>> +++ b/compositor/main.c
>> @@ -78,6 +78,7 @@ struct wet_compositor {
>> struct weston_config *config;
>> struct wet_output_config *parsed_options;
>> struct wl_listener pending_output_listener;
>> +   bool drm_use_current_mode;
>>  };
> 
> I'm fairly confused about this one, though I freely admit I didn't
> track the libweston config work, so may have missed something.
> 
> What makes --use-current-mode special enough that it should be the
> only such option inside struct wet_compositor? What makes it different
> to, say, use_pixman, which lives in the DRM backend?
> 

The thing is, --use-pixman flag is used in many places throughout the backend,
while --use-current-mode is only used by function that sets the mode. That
function is now called by the user, and is required to be called before
enabling the output, whereas it was previously done in the backend itself, and
it got the necessary configuration values by calling into an user-defined 
function
(configure output -> user defined function -> set mode).

>> @@ -1138,7 +1140,7 @@ drm_backend_output_configure(struct wl_listener 
>> *listener, void *data)
>> weston_output_disable(output);
>> free(s);
>> return;
>> -   } else if (strcmp(s, "current") == 0) {
>> +   } else if (wet->drm_use_current_mode || strcmp(s, "current") == 0) {
>> mode = WESTON_DRM_BACKEND_OUTPUT_CURRENT;
>> } else if (strcmp(s, "preferred") != 0) {
>> modeline = s;
> 
> What would the difference be to making this check be 'else if
> (b->use_current_mode || strcmp(s, "current") == 0)'?
> 
> Cheers,
> Daniel
> 

struct drm_backend is private to compositor-drm.c as far as I'm aware. We can't
use it here.



signature.asc
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 2/2] compositor-drm: Restore use-current-mode functionality

[Daniel Stone]

Hi Armin,

On 9 October 2016 at 22:48, Armin Krezović  wrote:
> diff --git a/compositor/main.c b/compositor/main.c
> index 320305c..ffeadfb 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -78,6 +78,7 @@ struct wet_compositor {
> struct weston_config *config;
> struct wet_output_config *parsed_options;
> struct wl_listener pending_output_listener;
> +   bool drm_use_current_mode;
>  };

I'm fairly confused about this one, though I freely admit I didn't
track the libweston config work, so may have missed something.

What makes --use-current-mode special enough that it should be the
only such option inside struct wet_compositor? What makes it different
to, say, use_pixman, which lives in the DRM backend?

> @@ -1138,7 +1140,7 @@ drm_backend_output_configure(struct wl_listener 
> *listener, void *data)
> weston_output_disable(output);
> free(s);
> return;
> -   } else if (strcmp(s, "current") == 0) {
> +   } else if (wet->drm_use_current_mode || strcmp(s, "current") == 0) {
> mode = WESTON_DRM_BACKEND_OUTPUT_CURRENT;
> } else if (strcmp(s, "preferred") != 0) {
> modeline = s;

What would the difference be to making this check be 'else if
(b->use_current_mode || strcmp(s, "current") == 0)'?

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel