* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-28 21:03 [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization Mario Limonciello
@ 2024-05-29 13:33 ` Alex Deucher
2024-05-29 13:51 ` Jani Nikula
2024-05-29 14:14 ` Ville Syrjälä
2024-06-04 2:02 ` kernel test robot
2 siblings, 1 reply; 17+ messages in thread
From: Alex Deucher @ 2024-05-29 13:33 UTC (permalink / raw)
To: Mario Limonciello
Cc: dri-devel, amd-gfx, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, linux-kernel,
Chris Bainbridge, hughsient
On Tue, May 28, 2024 at 5:03 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> If the lid on a laptop is closed when eDP connectors are populated
> then it remains enabled when the initial framebuffer configuration
> is built.
>
> When creating the initial framebuffer configuration detect the ACPI
> lid status and if it's closed disable any eDP connectors.
>
> Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Do you have drm-misc access or do you need someone to apply this for you?
Alex
> ---
> Cc: hughsient@gmail.com
> v1->v2:
> * Match LVDS as well
> ---
> drivers/gpu/drm/drm_client_modeset.c | 30 ++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 31af5cf37a09..0b0411086e76 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -8,6 +8,7 @@
> */
>
> #include "drm/drm_modeset_lock.h"
> +#include <acpi/button.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> @@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
> enabled[i] = drm_connector_enabled(connectors[i], false);
> }
>
> +static void drm_client_match_edp_lid(struct drm_device *dev,
> + struct drm_connector **connectors,
> + unsigned int connector_count,
> + bool *enabled)
> +{
> + int i;
> +
> + for (i = 0; i < connector_count; i++) {
> + struct drm_connector *connector = connectors[i];
> +
> + switch (connector->connector_type) {
> + case DRM_MODE_CONNECTOR_LVDS:
> + case DRM_MODE_CONNECTOR_eDP:
> + if (!enabled[i])
> + continue;
> + break;
> + default:
> + continue;
> + }
> +
> + if (!acpi_lid_open()) {
> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
> + connector->base.id, connector->name);
> + enabled[i] = false;
> + }
> + }
> +}
> +
> static bool drm_client_target_cloned(struct drm_device *dev,
> struct drm_connector **connectors,
> unsigned int connector_count,
> @@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
> memset(crtcs, 0, connector_count * sizeof(*crtcs));
> memset(offsets, 0, connector_count * sizeof(*offsets));
>
> + drm_client_match_edp_lid(dev, connectors, connector_count, enabled);
> if (!drm_client_target_cloned(dev, connectors, connector_count, modes,
> offsets, enabled, width, height) &&
> !drm_client_target_preferred(dev, connectors, connector_count, modes,
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-29 13:33 ` Alex Deucher
@ 2024-05-29 13:51 ` Jani Nikula
2024-05-29 13:55 ` Alex Deucher
0 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2024-05-29 13:51 UTC (permalink / raw)
To: Alex Deucher, Mario Limonciello
Cc: dri-devel, amd-gfx, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, linux-kernel,
Chris Bainbridge, hughsient
On Wed, 29 May 2024, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Tue, May 28, 2024 at 5:03 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>> If the lid on a laptop is closed when eDP connectors are populated
>> then it remains enabled when the initial framebuffer configuration
>> is built.
>>
>> When creating the initial framebuffer configuration detect the ACPI
>> lid status and if it's closed disable any eDP connectors.
>>
>> Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> Do you have drm-misc access or do you need someone to apply this for you?
I've bounced this to intel-gfx and intel-xe lists to get CI testing. I'd
appreciate holding off on merging until we have results.
Thanks,
Jani.
>
> Alex
>
>> ---
>> Cc: hughsient@gmail.com
>> v1->v2:
>> * Match LVDS as well
>> ---
>> drivers/gpu/drm/drm_client_modeset.c | 30 ++++++++++++++++++++++++++++
>> 1 file changed, 30 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
>> index 31af5cf37a09..0b0411086e76 100644
>> --- a/drivers/gpu/drm/drm_client_modeset.c
>> +++ b/drivers/gpu/drm/drm_client_modeset.c
>> @@ -8,6 +8,7 @@
>> */
>>
>> #include "drm/drm_modeset_lock.h"
>> +#include <acpi/button.h>
>> #include <linux/module.h>
>> #include <linux/mutex.h>
>> #include <linux/slab.h>
>> @@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
>> enabled[i] = drm_connector_enabled(connectors[i], false);
>> }
>>
>> +static void drm_client_match_edp_lid(struct drm_device *dev,
>> + struct drm_connector **connectors,
>> + unsigned int connector_count,
>> + bool *enabled)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < connector_count; i++) {
>> + struct drm_connector *connector = connectors[i];
>> +
>> + switch (connector->connector_type) {
>> + case DRM_MODE_CONNECTOR_LVDS:
>> + case DRM_MODE_CONNECTOR_eDP:
>> + if (!enabled[i])
>> + continue;
>> + break;
>> + default:
>> + continue;
>> + }
>> +
>> + if (!acpi_lid_open()) {
>> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
>> + connector->base.id, connector->name);
>> + enabled[i] = false;
>> + }
>> + }
>> +}
>> +
>> static bool drm_client_target_cloned(struct drm_device *dev,
>> struct drm_connector **connectors,
>> unsigned int connector_count,
>> @@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
>> memset(crtcs, 0, connector_count * sizeof(*crtcs));
>> memset(offsets, 0, connector_count * sizeof(*offsets));
>>
>> + drm_client_match_edp_lid(dev, connectors, connector_count, enabled);
>> if (!drm_client_target_cloned(dev, connectors, connector_count, modes,
>> offsets, enabled, width, height) &&
>> !drm_client_target_preferred(dev, connectors, connector_count, modes,
>> --
>> 2.43.0
>>
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-29 13:51 ` Jani Nikula
@ 2024-05-29 13:55 ` Alex Deucher
2024-05-29 14:34 ` Mario Limonciello
0 siblings, 1 reply; 17+ messages in thread
From: Alex Deucher @ 2024-05-29 13:55 UTC (permalink / raw)
To: Jani Nikula
Cc: Mario Limonciello, dri-devel, amd-gfx, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
linux-kernel, Chris Bainbridge, hughsient
On Wed, May 29, 2024 at 9:51 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Wed, 29 May 2024, Alex Deucher <alexdeucher@gmail.com> wrote:
> > On Tue, May 28, 2024 at 5:03 PM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> >>
> >> If the lid on a laptop is closed when eDP connectors are populated
> >> then it remains enabled when the initial framebuffer configuration
> >> is built.
> >>
> >> When creating the initial framebuffer configuration detect the ACPI
> >> lid status and if it's closed disable any eDP connectors.
> >>
> >> Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
> >> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> >
> > Do you have drm-misc access or do you need someone to apply this for you?
>
> I've bounced this to intel-gfx and intel-xe lists to get CI testing. I'd
> appreciate holding off on merging until we have results.
Sure.
Alex
>
> Thanks,
> Jani.
>
> >
> > Alex
> >
> >> ---
> >> Cc: hughsient@gmail.com
> >> v1->v2:
> >> * Match LVDS as well
> >> ---
> >> drivers/gpu/drm/drm_client_modeset.c | 30 ++++++++++++++++++++++++++++
> >> 1 file changed, 30 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> >> index 31af5cf37a09..0b0411086e76 100644
> >> --- a/drivers/gpu/drm/drm_client_modeset.c
> >> +++ b/drivers/gpu/drm/drm_client_modeset.c
> >> @@ -8,6 +8,7 @@
> >> */
> >>
> >> #include "drm/drm_modeset_lock.h"
> >> +#include <acpi/button.h>
> >> #include <linux/module.h>
> >> #include <linux/mutex.h>
> >> #include <linux/slab.h>
> >> @@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
> >> enabled[i] = drm_connector_enabled(connectors[i], false);
> >> }
> >>
> >> +static void drm_client_match_edp_lid(struct drm_device *dev,
> >> + struct drm_connector **connectors,
> >> + unsigned int connector_count,
> >> + bool *enabled)
> >> +{
> >> + int i;
> >> +
> >> + for (i = 0; i < connector_count; i++) {
> >> + struct drm_connector *connector = connectors[i];
> >> +
> >> + switch (connector->connector_type) {
> >> + case DRM_MODE_CONNECTOR_LVDS:
> >> + case DRM_MODE_CONNECTOR_eDP:
> >> + if (!enabled[i])
> >> + continue;
> >> + break;
> >> + default:
> >> + continue;
> >> + }
> >> +
> >> + if (!acpi_lid_open()) {
> >> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
> >> + connector->base.id, connector->name);
> >> + enabled[i] = false;
> >> + }
> >> + }
> >> +}
> >> +
> >> static bool drm_client_target_cloned(struct drm_device *dev,
> >> struct drm_connector **connectors,
> >> unsigned int connector_count,
> >> @@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
> >> memset(crtcs, 0, connector_count * sizeof(*crtcs));
> >> memset(offsets, 0, connector_count * sizeof(*offsets));
> >>
> >> + drm_client_match_edp_lid(dev, connectors, connector_count, enabled);
> >> if (!drm_client_target_cloned(dev, connectors, connector_count, modes,
> >> offsets, enabled, width, height) &&
> >> !drm_client_target_preferred(dev, connectors, connector_count, modes,
> >> --
> >> 2.43.0
> >>
>
> --
> Jani Nikula, Intel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-29 13:55 ` Alex Deucher
@ 2024-05-29 14:34 ` Mario Limonciello
0 siblings, 0 replies; 17+ messages in thread
From: Mario Limonciello @ 2024-05-29 14:34 UTC (permalink / raw)
To: Alex Deucher, Jani Nikula
Cc: dri-devel, amd-gfx, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, linux-kernel,
Chris Bainbridge, hughsient
On 5/29/2024 08:55, Alex Deucher wrote:
> On Wed, May 29, 2024 at 9:51 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>>
>> On Wed, 29 May 2024, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> On Tue, May 28, 2024 at 5:03 PM Mario Limonciello
>>> <mario.limonciello@amd.com> wrote:
>>>>
>>>> If the lid on a laptop is closed when eDP connectors are populated
>>>> then it remains enabled when the initial framebuffer configuration
>>>> is built.
>>>>
>>>> When creating the initial framebuffer configuration detect the ACPI
>>>> lid status and if it's closed disable any eDP connectors.
>>>>
>>>> Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
>>>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>>
>>> Do you have drm-misc access or do you need someone to apply this for you?
>>
>> I've bounced this to intel-gfx and intel-xe lists to get CI testing. I'd
>> appreciate holding off on merging until we have results.
>
> Sure.
Thanks for the review and pushing it to CI testing infra.
I don't have any drm-misc access so if everything looks good then one of
you guys please merge it for me.
Thanks!
>
> Alex
>
>>
>> Thanks,
>> Jani.
>>
>>>
>>> Alex
>>>
>>>> ---
>>>> Cc: hughsient@gmail.com
>>>> v1->v2:
>>>> * Match LVDS as well
>>>> ---
>>>> drivers/gpu/drm/drm_client_modeset.c | 30 ++++++++++++++++++++++++++++
>>>> 1 file changed, 30 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
>>>> index 31af5cf37a09..0b0411086e76 100644
>>>> --- a/drivers/gpu/drm/drm_client_modeset.c
>>>> +++ b/drivers/gpu/drm/drm_client_modeset.c
>>>> @@ -8,6 +8,7 @@
>>>> */
>>>>
>>>> #include "drm/drm_modeset_lock.h"
>>>> +#include <acpi/button.h>
>>>> #include <linux/module.h>
>>>> #include <linux/mutex.h>
>>>> #include <linux/slab.h>
>>>> @@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
>>>> enabled[i] = drm_connector_enabled(connectors[i], false);
>>>> }
>>>>
>>>> +static void drm_client_match_edp_lid(struct drm_device *dev,
>>>> + struct drm_connector **connectors,
>>>> + unsigned int connector_count,
>>>> + bool *enabled)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < connector_count; i++) {
>>>> + struct drm_connector *connector = connectors[i];
>>>> +
>>>> + switch (connector->connector_type) {
>>>> + case DRM_MODE_CONNECTOR_LVDS:
>>>> + case DRM_MODE_CONNECTOR_eDP:
>>>> + if (!enabled[i])
>>>> + continue;
>>>> + break;
>>>> + default:
>>>> + continue;
>>>> + }
>>>> +
>>>> + if (!acpi_lid_open()) {
>>>> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
>>>> + connector->base.id, connector->name);
>>>> + enabled[i] = false;
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> static bool drm_client_target_cloned(struct drm_device *dev,
>>>> struct drm_connector **connectors,
>>>> unsigned int connector_count,
>>>> @@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
>>>> memset(crtcs, 0, connector_count * sizeof(*crtcs));
>>>> memset(offsets, 0, connector_count * sizeof(*offsets));
>>>>
>>>> + drm_client_match_edp_lid(dev, connectors, connector_count, enabled);
>>>> if (!drm_client_target_cloned(dev, connectors, connector_count, modes,
>>>> offsets, enabled, width, height) &&
>>>> !drm_client_target_preferred(dev, connectors, connector_count, modes,
>>>> --
>>>> 2.43.0
>>>>
>>
>> --
>> Jani Nikula, Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-28 21:03 [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization Mario Limonciello
2024-05-29 13:33 ` Alex Deucher
@ 2024-05-29 14:14 ` Ville Syrjälä
2024-05-29 14:45 ` Mario Limonciello
2024-06-04 2:02 ` kernel test robot
2 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2024-05-29 14:14 UTC (permalink / raw)
To: Mario Limonciello
Cc: dri-devel, amd-gfx, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, linux-kernel,
Chris Bainbridge, hughsient
On Tue, May 28, 2024 at 04:03:19PM -0500, Mario Limonciello wrote:
> If the lid on a laptop is closed when eDP connectors are populated
> then it remains enabled when the initial framebuffer configuration
> is built.
>
> When creating the initial framebuffer configuration detect the ACPI
> lid status and if it's closed disable any eDP connectors.
>
> Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> Cc: hughsient@gmail.com
> v1->v2:
> * Match LVDS as well
> ---
> drivers/gpu/drm/drm_client_modeset.c | 30 ++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 31af5cf37a09..0b0411086e76 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -8,6 +8,7 @@
> */
>
> #include "drm/drm_modeset_lock.h"
> +#include <acpi/button.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> @@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
> enabled[i] = drm_connector_enabled(connectors[i], false);
> }
>
> +static void drm_client_match_edp_lid(struct drm_device *dev,
> + struct drm_connector **connectors,
> + unsigned int connector_count,
> + bool *enabled)
> +{
> + int i;
> +
> + for (i = 0; i < connector_count; i++) {
> + struct drm_connector *connector = connectors[i];
> +
> + switch (connector->connector_type) {
> + case DRM_MODE_CONNECTOR_LVDS:
> + case DRM_MODE_CONNECTOR_eDP:
> + if (!enabled[i])
> + continue;
> + break;
> + default:
> + continue;
> + }
> +
> + if (!acpi_lid_open()) {
> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
> + connector->base.id, connector->name);
> + enabled[i] = false;
> + }
> + }
> +}
If you don't hook into some lid notify event how is one supposed to get
the display back to life after opening the lid?
> +
> static bool drm_client_target_cloned(struct drm_device *dev,
> struct drm_connector **connectors,
> unsigned int connector_count,
> @@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
> memset(crtcs, 0, connector_count * sizeof(*crtcs));
> memset(offsets, 0, connector_count * sizeof(*offsets));
>
> + drm_client_match_edp_lid(dev, connectors, connector_count, enabled);
> if (!drm_client_target_cloned(dev, connectors, connector_count, modes,
> offsets, enabled, width, height) &&
> !drm_client_target_preferred(dev, connectors, connector_count, modes,
> --
> 2.43.0
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-29 14:14 ` Ville Syrjälä
@ 2024-05-29 14:45 ` Mario Limonciello
2024-05-29 15:39 ` Ville Syrjälä
0 siblings, 1 reply; 17+ messages in thread
From: Mario Limonciello @ 2024-05-29 14:45 UTC (permalink / raw)
To: Ville Syrjälä
Cc: dri-devel, amd-gfx, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, linux-kernel,
Chris Bainbridge, hughsient
On 5/29/2024 09:14, Ville Syrjälä wrote:
> On Tue, May 28, 2024 at 04:03:19PM -0500, Mario Limonciello wrote:
>> If the lid on a laptop is closed when eDP connectors are populated
>> then it remains enabled when the initial framebuffer configuration
>> is built.
>>
>> When creating the initial framebuffer configuration detect the ACPI
>> lid status and if it's closed disable any eDP connectors.
>>
>> Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> Cc: hughsient@gmail.com
>> v1->v2:
>> * Match LVDS as well
>> ---
>> drivers/gpu/drm/drm_client_modeset.c | 30 ++++++++++++++++++++++++++++
>> 1 file changed, 30 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
>> index 31af5cf37a09..0b0411086e76 100644
>> --- a/drivers/gpu/drm/drm_client_modeset.c
>> +++ b/drivers/gpu/drm/drm_client_modeset.c
>> @@ -8,6 +8,7 @@
>> */
>>
>> #include "drm/drm_modeset_lock.h"
>> +#include <acpi/button.h>
>> #include <linux/module.h>
>> #include <linux/mutex.h>
>> #include <linux/slab.h>
>> @@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
>> enabled[i] = drm_connector_enabled(connectors[i], false);
>> }
>>
>> +static void drm_client_match_edp_lid(struct drm_device *dev,
>> + struct drm_connector **connectors,
>> + unsigned int connector_count,
>> + bool *enabled)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < connector_count; i++) {
>> + struct drm_connector *connector = connectors[i];
>> +
>> + switch (connector->connector_type) {
>> + case DRM_MODE_CONNECTOR_LVDS:
>> + case DRM_MODE_CONNECTOR_eDP:
>> + if (!enabled[i])
>> + continue;
>> + break;
>> + default:
>> + continue;
>> + }
>> +
>> + if (!acpi_lid_open()) {
>> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
>> + connector->base.id, connector->name);
>> + enabled[i] = false;
>> + }
>> + }
>> +}
>
> If you don't hook into some lid notify event how is one supposed to get
> the display back to life after opening the lid?
I guess in my mind it's a tangential to the "initial modeset". The DRM
master can issue a modeset to enable the combination as desired.
When I tested I did confirm that with mutter such an event is received
and it does the modeset to enable the eDP when lid is opened.
Let me ask this - what happens if no DRM master running and you hotplug
a DP cable? Does a "new" clone configuration get done?
>
>> +
>> static bool drm_client_target_cloned(struct drm_device *dev,
>> struct drm_connector **connectors,
>> unsigned int connector_count,
>> @@ -844,6 +873,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
>> memset(crtcs, 0, connector_count * sizeof(*crtcs));
>> memset(offsets, 0, connector_count * sizeof(*offsets));
>>
>> + drm_client_match_edp_lid(dev, connectors, connector_count, enabled);
>> if (!drm_client_target_cloned(dev, connectors, connector_count, modes,
>> offsets, enabled, width, height) &&
>> !drm_client_target_preferred(dev, connectors, connector_count, modes,
>> --
>> 2.43.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-29 14:45 ` Mario Limonciello
@ 2024-05-29 15:39 ` Ville Syrjälä
2024-05-29 16:26 ` Mario Limonciello
2024-05-29 17:55 ` Dmitry Baryshkov
0 siblings, 2 replies; 17+ messages in thread
From: Ville Syrjälä @ 2024-05-29 15:39 UTC (permalink / raw)
To: Mario Limonciello
Cc: dri-devel, amd-gfx, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, linux-kernel,
Chris Bainbridge, hughsient
On Wed, May 29, 2024 at 09:45:55AM -0500, Mario Limonciello wrote:
> On 5/29/2024 09:14, Ville Syrjälä wrote:
> > On Tue, May 28, 2024 at 04:03:19PM -0500, Mario Limonciello wrote:
> >> If the lid on a laptop is closed when eDP connectors are populated
> >> then it remains enabled when the initial framebuffer configuration
> >> is built.
> >>
> >> When creating the initial framebuffer configuration detect the ACPI
> >> lid status and if it's closed disable any eDP connectors.
> >>
> >> Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
> >> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >> ---
> >> Cc: hughsient@gmail.com
> >> v1->v2:
> >> * Match LVDS as well
> >> ---
> >> drivers/gpu/drm/drm_client_modeset.c | 30 ++++++++++++++++++++++++++++
> >> 1 file changed, 30 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> >> index 31af5cf37a09..0b0411086e76 100644
> >> --- a/drivers/gpu/drm/drm_client_modeset.c
> >> +++ b/drivers/gpu/drm/drm_client_modeset.c
> >> @@ -8,6 +8,7 @@
> >> */
> >>
> >> #include "drm/drm_modeset_lock.h"
> >> +#include <acpi/button.h>
> >> #include <linux/module.h>
> >> #include <linux/mutex.h>
> >> #include <linux/slab.h>
> >> @@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
> >> enabled[i] = drm_connector_enabled(connectors[i], false);
> >> }
> >>
> >> +static void drm_client_match_edp_lid(struct drm_device *dev,
> >> + struct drm_connector **connectors,
> >> + unsigned int connector_count,
> >> + bool *enabled)
> >> +{
> >> + int i;
> >> +
> >> + for (i = 0; i < connector_count; i++) {
> >> + struct drm_connector *connector = connectors[i];
> >> +
> >> + switch (connector->connector_type) {
> >> + case DRM_MODE_CONNECTOR_LVDS:
> >> + case DRM_MODE_CONNECTOR_eDP:
> >> + if (!enabled[i])
> >> + continue;
> >> + break;
> >> + default:
> >> + continue;
> >> + }
> >> +
> >> + if (!acpi_lid_open()) {
> >> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
> >> + connector->base.id, connector->name);
> >> + enabled[i] = false;
> >> + }
> >> + }
> >> +}
> >
> > If you don't hook into some lid notify event how is one supposed to get
> > the display back to life after opening the lid?
>
> I guess in my mind it's a tangential to the "initial modeset". The DRM
> master can issue a modeset to enable the combination as desired.
This code is run whenever there's a hotplug/etc. Not sure why you're
only thinking about the initial modeset.
>
> When I tested I did confirm that with mutter such an event is received
> and it does the modeset to enable the eDP when lid is opened.
This code isn't relevant when you have a userspace drm master
calling the shots.
>
> Let me ask this - what happens if no DRM master running and you hotplug
> a DP cable? Does a "new" clone configuration get done?
Yes, this code reprobes the displays and comes up with a new
config to suit the new situation.
The other potential issue here is whether acpi_lid_open() is actually
trustworthy. Some kms drivers have/had some lid handling in their own
code, and I'm pretty sure those have often needed quirks/modparams
to actually do sensible things on certain machines.
FWIW I ripped out all the lid crap from i915 long ago since it was
half backed, mostly broken, and ugly, and I'm not looking to add it
back there. But I do think handling that in drm_client does seem
somewhat sane, as that should more or less match what userspace
clients would do. Just a question of how bad the quirk situation
will get...
Also a direct acpi_lid_open() call seems a bit iffy. But I guess if
someone needs this to work on non-ACPI system they get to figure out
how to abstract it better. acpi_lid_open() does seem to return != 0
when ACPI is not supported, so at least it would err on the side
of enabling everything.
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-29 15:39 ` Ville Syrjälä
@ 2024-05-29 16:26 ` Mario Limonciello
2024-05-29 17:55 ` Dmitry Baryshkov
1 sibling, 0 replies; 17+ messages in thread
From: Mario Limonciello @ 2024-05-29 16:26 UTC (permalink / raw)
To: Ville Syrjälä
Cc: dri-devel, amd-gfx, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, linux-kernel,
Chris Bainbridge, hughsient
>>>
>>> If you don't hook into some lid notify event how is one supposed to get
>>> the display back to life after opening the lid?
>>
>> I guess in my mind it's a tangential to the "initial modeset". The DRM
>> master can issue a modeset to enable the combination as desired.
>
> This code is run whenever there's a hotplug/etc. Not sure why you're
> only thinking about the initial modeset.
Got it; so in that case adding a notification chain for lid events to
run it again should do the trick.
>
>>
>> When I tested I did confirm that with mutter such an event is received
>> and it does the modeset to enable the eDP when lid is opened.
>
> This code isn't relevant when you have a userspace drm master
> calling the shots.
Right.
>
>>
>> Let me ask this - what happens if no DRM master running and you hotplug
>> a DP cable? Does a "new" clone configuration get done?
>
> Yes, this code reprobes the displays and comes up with a new
> config to suit the new situation.
Got it; in this case you're right we should have some notification
chain. Do you think it should be in the initial patch or a follow up?
>
> The other potential issue here is whether acpi_lid_open() is actually
> trustworthy. Some kms drivers have/had some lid handling in their own
> code, and I'm pretty sure those have often needed quirks/modparams
> to actually do sensible things on certain machines.
>
> FWIW I ripped out all the lid crap from i915 long ago since it was
> half backed, mostly broken, and ugly, and I'm not looking to add it
> back there. But I do think handling that in drm_client does seem
> somewhat sane, as that should more or less match what userspace
> clients would do. Just a question of how bad the quirk situation
> will get...
>
If the lid reporting is wrong it's not just drm_client that would
falter. There are other parts of the kernel that rely upon
acpi_lid_open() being accurate and IMO it would be best to put any
quirks to the effect in drivers/acpi/button.c.
If it can't be relied upon then it's best to just report -EINVAL or -ENODEV.
>
> Also a direct acpi_lid_open() call seems a bit iffy. But I guess if
> someone needs this to work on non-ACPI system they get to figure out
> how to abstract it better. acpi_lid_open() does seem to return != 0
> when ACPI is not supported, so at least it would err on the side
> of enabling everything.
>
Yeah acpi_lid_open() seemed fine to me specifically because non ACPI
hardcodes to open.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-29 15:39 ` Ville Syrjälä
2024-05-29 16:26 ` Mario Limonciello
@ 2024-05-29 17:55 ` Dmitry Baryshkov
2024-05-30 4:41 ` Limonciello, Mario
1 sibling, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2024-05-29 17:55 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Mario Limonciello, dri-devel, amd-gfx, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
linux-kernel, Chris Bainbridge, hughsient
On Wed, May 29, 2024 at 06:39:21PM +0300, Ville Syrjälä wrote:
> On Wed, May 29, 2024 at 09:45:55AM -0500, Mario Limonciello wrote:
> > On 5/29/2024 09:14, Ville Syrjälä wrote:
> > > On Tue, May 28, 2024 at 04:03:19PM -0500, Mario Limonciello wrote:
> > >> If the lid on a laptop is closed when eDP connectors are populated
> > >> then it remains enabled when the initial framebuffer configuration
> > >> is built.
> > >>
> > >> When creating the initial framebuffer configuration detect the ACPI
> > >> lid status and if it's closed disable any eDP connectors.
> > >>
> > >> Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
> > >> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3349
> > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > >> ---
> > >> Cc: hughsient@gmail.com
> > >> v1->v2:
> > >> * Match LVDS as well
> > >> ---
> > >> drivers/gpu/drm/drm_client_modeset.c | 30 ++++++++++++++++++++++++++++
> > >> 1 file changed, 30 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> > >> index 31af5cf37a09..0b0411086e76 100644
> > >> --- a/drivers/gpu/drm/drm_client_modeset.c
> > >> +++ b/drivers/gpu/drm/drm_client_modeset.c
> > >> @@ -8,6 +8,7 @@
> > >> */
> > >>
> > >> #include "drm/drm_modeset_lock.h"
> > >> +#include <acpi/button.h>
> > >> #include <linux/module.h>
> > >> #include <linux/mutex.h>
> > >> #include <linux/slab.h>
> > >> @@ -257,6 +258,34 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
> > >> enabled[i] = drm_connector_enabled(connectors[i], false);
> > >> }
> > >>
> > >> +static void drm_client_match_edp_lid(struct drm_device *dev,
> > >> + struct drm_connector **connectors,
> > >> + unsigned int connector_count,
> > >> + bool *enabled)
> > >> +{
> > >> + int i;
> > >> +
> > >> + for (i = 0; i < connector_count; i++) {
> > >> + struct drm_connector *connector = connectors[i];
> > >> +
> > >> + switch (connector->connector_type) {
> > >> + case DRM_MODE_CONNECTOR_LVDS:
> > >> + case DRM_MODE_CONNECTOR_eDP:
What about DPI or DSI panels? I think they should also be handled in a
similar way. Not sure regarding the SPI.
> > >> + if (!enabled[i])
> > >> + continue;
> > >> + break;
> > >> + default:
> > >> + continue;
> > >> + }
> > >> +
> > >> + if (!acpi_lid_open()) {
> > >> + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
> > >> + connector->base.id, connector->name);
> > >> + enabled[i] = false;
> > >> + }
> > >> + }
> > >> +}
> > >
[trimmed]
>
> The other potential issue here is whether acpi_lid_open() is actually
> trustworthy. Some kms drivers have/had some lid handling in their own
> code, and I'm pretty sure those have often needed quirks/modparams
> to actually do sensible things on certain machines.
>
> FWIW I ripped out all the lid crap from i915 long ago since it was
> half backed, mostly broken, and ugly, and I'm not looking to add it
> back there. But I do think handling that in drm_client does seem
> somewhat sane, as that should more or less match what userspace
> clients would do. Just a question of how bad the quirk situation
> will get...
Looking at drivers/acpi/button.c, the button driver handles some of the
quirks when posting the data to the input subsystem.
>
>
> Also a direct acpi_lid_open() call seems a bit iffy. But I guess if
> someone needs this to work on non-ACPI system they get to figure out
> how to abstract it better. acpi_lid_open() does seem to return != 0
> when ACPI is not supported, so at least it would err on the side
> of enabling everything.
Thanks. I was going to comment, but you got it first. I think a proper
implementation should check for SW_LID input device instead of simply
using acpi_lid_open(). This will handle the issue for other,
non-ACPI-based laptops.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-29 17:55 ` Dmitry Baryshkov
@ 2024-05-30 4:41 ` Limonciello, Mario
2024-05-30 8:07 ` Dmitry Baryshkov
0 siblings, 1 reply; 17+ messages in thread
From: Limonciello, Mario @ 2024-05-30 4:41 UTC (permalink / raw)
To: Dmitry Baryshkov, Ville Syrjälä
Cc: dri-devel, amd-gfx, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, linux-kernel,
Chris Bainbridge, hughsient
>> Also a direct acpi_lid_open() call seems a bit iffy. But I guess if
>> someone needs this to work on non-ACPI system they get to figure out
>> how to abstract it better. acpi_lid_open() does seem to return != 0
>> when ACPI is not supported, so at least it would err on the side
>> of enabling everything.
>
> Thanks. I was going to comment, but you got it first. I think a proper
> implementation should check for SW_LID input device instead of simply
> using acpi_lid_open(). This will handle the issue for other,
> non-ACPI-based laptops.
>
Can you suggest how this would actually work? AFAICT the only way to
discover if input devices support SW_LID would be to iterate all the
input devices in the kernel and look for whether ->swbit has SW_LID set.
This then turns into a dependency problem of whether any myriad of
drivers have started to report SW_LID. It's also a state machine
problem because other drivers can be unloaded at will.
And then what do you if more than one sets SW_LID?
IOW - a lot of complexity for a non-ACPI system. Does such a problem
exist in non-ACPI systems?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-30 4:41 ` Limonciello, Mario
@ 2024-05-30 8:07 ` Dmitry Baryshkov
2024-05-30 20:44 ` Dmitry Torokhov
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2024-05-30 8:07 UTC (permalink / raw)
To: Limonciello, Mario, Dmitry Torokhov
Cc: Ville Syrjälä, dri-devel, amd-gfx, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
linux-kernel, Chris Bainbridge, hughsient, linux-input
On Thu, 30 May 2024 at 07:41, Limonciello, Mario
<mario.limonciello@amd.com> wrote:
>
>
> >> Also a direct acpi_lid_open() call seems a bit iffy. But I guess if
> >> someone needs this to work on non-ACPI system they get to figure out
> >> how to abstract it better. acpi_lid_open() does seem to return != 0
> >> when ACPI is not supported, so at least it would err on the side
> >> of enabling everything.
> >
> > Thanks. I was going to comment, but you got it first. I think a proper
> > implementation should check for SW_LID input device instead of simply
> > using acpi_lid_open(). This will handle the issue for other,
> > non-ACPI-based laptops.
> >
>
> Can you suggest how this would actually work? AFAICT the only way to
> discover if input devices support SW_LID would be to iterate all the
> input devices in the kernel and look for whether ->swbit has SW_LID set.
>
> This then turns into a dependency problem of whether any myriad of
> drivers have started to report SW_LID. It's also a state machine
> problem because other drivers can be unloaded at will.
>
> And then what do you if more than one sets SW_LID?
It might be easier to handle this in the input subsystem. For example
by using a refcount-like variable which handles all the LIDs and
counts if all of them are closed. Or if any of the LIDs is closed.
>
> IOW - a lot of complexity for a non-ACPI system. Does such a problem
> exist in non-ACPI systems?
There are non-ACPI laptops. For example Chromebooks. Or Lenovo X13s,
Lenovo Yoga C630, Lenovo Flex5G, etc. We are expecting more to come in
the next few months. And I don't see why they won't have the same
problem.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-30 8:07 ` Dmitry Baryshkov
@ 2024-05-30 20:44 ` Dmitry Torokhov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2024-05-30 20:44 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Limonciello, Mario, Ville Syrjälä, dri-devel, amd-gfx,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, linux-kernel, Chris Bainbridge, hughsient,
linux-input
On Thu, May 30, 2024 at 11:07:53AM +0300, Dmitry Baryshkov wrote:
> On Thu, 30 May 2024 at 07:41, Limonciello, Mario
> <mario.limonciello@amd.com> wrote:
> >
> >
> > >> Also a direct acpi_lid_open() call seems a bit iffy. But I guess if
> > >> someone needs this to work on non-ACPI system they get to figure out
> > >> how to abstract it better. acpi_lid_open() does seem to return != 0
> > >> when ACPI is not supported, so at least it would err on the side
> > >> of enabling everything.
> > >
> > > Thanks. I was going to comment, but you got it first. I think a proper
> > > implementation should check for SW_LID input device instead of simply
> > > using acpi_lid_open(). This will handle the issue for other,
> > > non-ACPI-based laptops.
> > >
> >
> > Can you suggest how this would actually work? AFAICT the only way to
> > discover if input devices support SW_LID would be to iterate all the
> > input devices in the kernel and look for whether ->swbit has SW_LID set.
> >
> > This then turns into a dependency problem of whether any myriad of
> > drivers have started to report SW_LID. It's also a state machine
> > problem because other drivers can be unloaded at will.
> >
> > And then what do you if more than one sets SW_LID?
>
> It might be easier to handle this in the input subsystem. For example
> by using a refcount-like variable which handles all the LIDs and
> counts if all of them are closed. Or if any of the LIDs is closed.
Yes, install an input handler matching on EV_SW/SW_LID so you will get
notified when input devices capable of reporting SW_LID appear and
disappear and also when SW_LID event is being generated, and handle as
you wish. Something like
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/40e9f6a991856ee7d504ac1ccd587e435775cfc4%5E%21/#F0
In practice I think it is pretty safe to assume only 1 lid for a
laptop/device.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-05-28 21:03 [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization Mario Limonciello
2024-05-29 13:33 ` Alex Deucher
2024-05-29 14:14 ` Ville Syrjälä
@ 2024-06-04 2:02 ` kernel test robot
2024-06-05 15:08 ` Chris Bainbridge
2 siblings, 1 reply; 17+ messages in thread
From: kernel test robot @ 2024-06-04 2:02 UTC (permalink / raw)
To: Mario Limonciello, dri-devel, amd-gfx, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter
Cc: oe-kbuild-all, linux-kernel, Mario Limonciello, Chris Bainbridge,
hughsient
Hi Mario,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.10-rc2 next-20240603]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/drm-client-Detect-when-ACPI-lid-is-closed-during-initialization/20240529-050440
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20240528210319.1242-1-mario.limonciello%40amd.com
patch subject: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
config: i386-randconfig-053-20240604 (https://download.01.org/0day-ci/archive/20240604/202406040928.Eu1gRIWv-lkp@intel.com/config)
compiler: gcc-9 (Ubuntu 9.5.0-4ubuntu2) 9.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240604/202406040928.Eu1gRIWv-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406040928.Eu1gRIWv-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: drivers/gpu/drm/drm_client_modeset.o: in function `drm_client_match_edp_lid':
>> drivers/gpu/drm/drm_client_modeset.c:281:(.text+0x221b): undefined reference to `acpi_lid_open'
vim +281 drivers/gpu/drm/drm_client_modeset.c
260
261 static void drm_client_match_edp_lid(struct drm_device *dev,
262 struct drm_connector **connectors,
263 unsigned int connector_count,
264 bool *enabled)
265 {
266 int i;
267
268 for (i = 0; i < connector_count; i++) {
269 struct drm_connector *connector = connectors[i];
270
271 switch (connector->connector_type) {
272 case DRM_MODE_CONNECTOR_LVDS:
273 case DRM_MODE_CONNECTOR_eDP:
274 if (!enabled[i])
275 continue;
276 break;
277 default:
278 continue;
279 }
280
> 281 if (!acpi_lid_open()) {
282 drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
283 connector->base.id, connector->name);
284 enabled[i] = false;
285 }
286 }
287 }
288
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-06-04 2:02 ` kernel test robot
@ 2024-06-05 15:08 ` Chris Bainbridge
2024-06-06 7:21 ` Jani Nikula
0 siblings, 1 reply; 17+ messages in thread
From: Chris Bainbridge @ 2024-06-05 15:08 UTC (permalink / raw)
To: kernel test robot
Cc: Mario Limonciello, dri-devel, amd-gfx, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
oe-kbuild-all, linux-kernel, hughsient
On Tue, Jun 04, 2024 at 10:02:29AM +0800, kernel test robot wrote:
> Hi Mario,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on drm-misc/drm-misc-next]
> [also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.10-rc2 next-20240603]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/drm-client-Detect-when-ACPI-lid-is-closed-during-initialization/20240529-050440
> base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> patch link: https://lore.kernel.org/r/20240528210319.1242-1-mario.limonciello%40amd.com
> patch subject: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
> config: i386-randconfig-053-20240604 (https://download.01.org/0day-ci/archive/20240604/202406040928.Eu1gRIWv-lkp@intel.com/config)
> compiler: gcc-9 (Ubuntu 9.5.0-4ubuntu2) 9.5.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240604/202406040928.Eu1gRIWv-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202406040928.Eu1gRIWv-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> ld: drivers/gpu/drm/drm_client_modeset.o: in function `drm_client_match_edp_lid':
> >> drivers/gpu/drm/drm_client_modeset.c:281:(.text+0x221b): undefined reference to `acpi_lid_open'
>
>
> vim +281 drivers/gpu/drm/drm_client_modeset.c
>
> 260
> 261 static void drm_client_match_edp_lid(struct drm_device *dev,
> 262 struct drm_connector **connectors,
> 263 unsigned int connector_count,
> 264 bool *enabled)
> 265 {
> 266 int i;
> 267
> 268 for (i = 0; i < connector_count; i++) {
> 269 struct drm_connector *connector = connectors[i];
> 270
> 271 switch (connector->connector_type) {
> 272 case DRM_MODE_CONNECTOR_LVDS:
> 273 case DRM_MODE_CONNECTOR_eDP:
> 274 if (!enabled[i])
> 275 continue;
> 276 break;
> 277 default:
> 278 continue;
> 279 }
> 280
> > 281 if (!acpi_lid_open()) {
> 282 drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
> 283 connector->base.id, connector->name);
> 284 enabled[i] = false;
> 285 }
> 286 }
> 287 }
> 288
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
The failed config has CONFIG_ACPI_BUTTON=m. The build failure can be
fixed with:
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index b76438c31761..0271e66f44f8 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -271,11 +271,13 @@ static void drm_client_match_edp_lid(struct drm_device *dev,
if (connector->connector_type != DRM_MODE_CONNECTOR_eDP || !enabled[i])
continue;
+#if defined(CONFIG_ACPI_BUTTON)
if (!acpi_lid_open()) {
drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
connector->base.id, connector->name);
enabled[i] = false;
}
+#endif
}
}
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-06-05 15:08 ` Chris Bainbridge
@ 2024-06-06 7:21 ` Jani Nikula
2024-06-06 12:31 ` Ville Syrjälä
0 siblings, 1 reply; 17+ messages in thread
From: Jani Nikula @ 2024-06-06 7:21 UTC (permalink / raw)
To: Chris Bainbridge, kernel test robot
Cc: Mario Limonciello, dri-devel, amd-gfx, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
oe-kbuild-all, linux-kernel, hughsient
On Wed, 05 Jun 2024, Chris Bainbridge <chris.bainbridge@gmail.com> wrote:
> On Tue, Jun 04, 2024 at 10:02:29AM +0800, kernel test robot wrote:
>> Hi Mario,
>>
>> kernel test robot noticed the following build errors:
>>
>> [auto build test ERROR on drm-misc/drm-misc-next]
>> [also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.10-rc2 next-20240603]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/drm-client-Detect-when-ACPI-lid-is-closed-during-initialization/20240529-050440
>> base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
>> patch link: https://lore.kernel.org/r/20240528210319.1242-1-mario.limonciello%40amd.com
>> patch subject: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
>> config: i386-randconfig-053-20240604 (https://download.01.org/0day-ci/archive/20240604/202406040928.Eu1gRIWv-lkp@intel.com/config)
>> compiler: gcc-9 (Ubuntu 9.5.0-4ubuntu2) 9.5.0
>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240604/202406040928.Eu1gRIWv-lkp@intel.com/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202406040928.Eu1gRIWv-lkp@intel.com/
>>
>> All errors (new ones prefixed by >>):
>>
>> ld: drivers/gpu/drm/drm_client_modeset.o: in function `drm_client_match_edp_lid':
>> >> drivers/gpu/drm/drm_client_modeset.c:281:(.text+0x221b): undefined reference to `acpi_lid_open'
>>
>>
>> vim +281 drivers/gpu/drm/drm_client_modeset.c
>>
>> 260
>> 261 static void drm_client_match_edp_lid(struct drm_device *dev,
>> 262 struct drm_connector **connectors,
>> 263 unsigned int connector_count,
>> 264 bool *enabled)
>> 265 {
>> 266 int i;
>> 267
>> 268 for (i = 0; i < connector_count; i++) {
>> 269 struct drm_connector *connector = connectors[i];
>> 270
>> 271 switch (connector->connector_type) {
>> 272 case DRM_MODE_CONNECTOR_LVDS:
>> 273 case DRM_MODE_CONNECTOR_eDP:
>> 274 if (!enabled[i])
>> 275 continue;
>> 276 break;
>> 277 default:
>> 278 continue;
>> 279 }
>> 280
>> > 281 if (!acpi_lid_open()) {
>> 282 drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
>> 283 connector->base.id, connector->name);
>> 284 enabled[i] = false;
>> 285 }
>> 286 }
>> 287 }
>> 288
>>
>> --
>> 0-DAY CI Kernel Test Service
>> https://github.com/intel/lkp-tests/wiki
>
> The failed config has CONFIG_ACPI_BUTTON=m. The build failure can be
> fixed with:
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index b76438c31761..0271e66f44f8 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -271,11 +271,13 @@ static void drm_client_match_edp_lid(struct drm_device *dev,
> if (connector->connector_type != DRM_MODE_CONNECTOR_eDP || !enabled[i])
> continue;
>
> +#if defined(CONFIG_ACPI_BUTTON)
> if (!acpi_lid_open()) {
> drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
> connector->base.id, connector->name);
> enabled[i] = false;
> }
> +#endif
> }
> }
No. This is because
CONFIG_DRM=y
CONFIG_ACPI_BUTTON=m
The pedantically correct fix is probably having DRM
depends on ACPI_BUTTON || ACPI_BUTTON=n
but seeing how i915 and xe just
select ACPI_BUTTON if ACPI
and nouveau basically uses acpi_lid_open() it if it's reachable with no
regard to kconfig, it's anyone's guess what will work.
BR,
Jani.
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
2024-06-06 7:21 ` Jani Nikula
@ 2024-06-06 12:31 ` Ville Syrjälä
0 siblings, 0 replies; 17+ messages in thread
From: Ville Syrjälä @ 2024-06-06 12:31 UTC (permalink / raw)
To: Jani Nikula
Cc: Chris Bainbridge, kernel test robot, Mario Limonciello, dri-devel,
amd-gfx, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, oe-kbuild-all, linux-kernel,
hughsient
On Thu, Jun 06, 2024 at 10:21:07AM +0300, Jani Nikula wrote:
> On Wed, 05 Jun 2024, Chris Bainbridge <chris.bainbridge@gmail.com> wrote:
> > On Tue, Jun 04, 2024 at 10:02:29AM +0800, kernel test robot wrote:
> >> Hi Mario,
> >>
> >> kernel test robot noticed the following build errors:
> >>
> >> [auto build test ERROR on drm-misc/drm-misc-next]
> >> [also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.10-rc2 next-20240603]
> >> [If your patch is applied to the wrong git tree, kindly drop us a note.
> >> And when submitting patch, we suggest to use '--base' as documented in
> >> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >>
> >> url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/drm-client-Detect-when-ACPI-lid-is-closed-during-initialization/20240529-050440
> >> base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> >> patch link: https://lore.kernel.org/r/20240528210319.1242-1-mario.limonciello%40amd.com
> >> patch subject: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
> >> config: i386-randconfig-053-20240604 (https://download.01.org/0day-ci/archive/20240604/202406040928.Eu1gRIWv-lkp@intel.com/config)
> >> compiler: gcc-9 (Ubuntu 9.5.0-4ubuntu2) 9.5.0
> >> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240604/202406040928.Eu1gRIWv-lkp@intel.com/reproduce)
> >>
> >> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> >> the same patch/commit), kindly add following tags
> >> | Reported-by: kernel test robot <lkp@intel.com>
> >> | Closes: https://lore.kernel.org/oe-kbuild-all/202406040928.Eu1gRIWv-lkp@intel.com/
> >>
> >> All errors (new ones prefixed by >>):
> >>
> >> ld: drivers/gpu/drm/drm_client_modeset.o: in function `drm_client_match_edp_lid':
> >> >> drivers/gpu/drm/drm_client_modeset.c:281:(.text+0x221b): undefined reference to `acpi_lid_open'
> >>
> >>
> >> vim +281 drivers/gpu/drm/drm_client_modeset.c
> >>
> >> 260
> >> 261 static void drm_client_match_edp_lid(struct drm_device *dev,
> >> 262 struct drm_connector **connectors,
> >> 263 unsigned int connector_count,
> >> 264 bool *enabled)
> >> 265 {
> >> 266 int i;
> >> 267
> >> 268 for (i = 0; i < connector_count; i++) {
> >> 269 struct drm_connector *connector = connectors[i];
> >> 270
> >> 271 switch (connector->connector_type) {
> >> 272 case DRM_MODE_CONNECTOR_LVDS:
> >> 273 case DRM_MODE_CONNECTOR_eDP:
> >> 274 if (!enabled[i])
> >> 275 continue;
> >> 276 break;
> >> 277 default:
> >> 278 continue;
> >> 279 }
> >> 280
> >> > 281 if (!acpi_lid_open()) {
> >> 282 drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
> >> 283 connector->base.id, connector->name);
> >> 284 enabled[i] = false;
> >> 285 }
> >> 286 }
> >> 287 }
> >> 288
> >>
> >> --
> >> 0-DAY CI Kernel Test Service
> >> https://github.com/intel/lkp-tests/wiki
> >
> > The failed config has CONFIG_ACPI_BUTTON=m. The build failure can be
> > fixed with:
> >
> > diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> > index b76438c31761..0271e66f44f8 100644
> > --- a/drivers/gpu/drm/drm_client_modeset.c
> > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > @@ -271,11 +271,13 @@ static void drm_client_match_edp_lid(struct drm_device *dev,
> > if (connector->connector_type != DRM_MODE_CONNECTOR_eDP || !enabled[i])
> > continue;
> >
> > +#if defined(CONFIG_ACPI_BUTTON)
> > if (!acpi_lid_open()) {
> > drm_dbg_kms(dev, "[CONNECTOR:%d:%s] lid is closed, disabling\n",
> > connector->base.id, connector->name);
> > enabled[i] = false;
> > }
> > +#endif
> > }
> > }
>
> No. This is because
>
> CONFIG_DRM=y
> CONFIG_ACPI_BUTTON=m
>
> The pedantically correct fix is probably having DRM
>
> depends on ACPI_BUTTON || ACPI_BUTTON=n
>
> but seeing how i915 and xe just
>
> select ACPI_BUTTON if ACPI
Huh. We should nuke that as we haven't used this lid stuff in ages.
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 17+ messages in thread