[PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[yakui_zhao]

Hi, All
    the following is the patch set that can weaken the dependency between ACPI video and i915
driver.
    Patch 1/2:  Check whether the acpi video is deferrable only when KMS is enabled
    When the KMS is enabled in kernel configuration, OS will check whether  the acpi video 
driver is deferrable. Otherwise it will continue the current flowchart.

    Patch 2/2:  weaken the tight dependency between the ACPI video driver and i915 driver
    Now there exists the tight dependency between the ACPI video driver and i915 driver. 
If the I915 driver is compiled as built-in module and the acpi video is compiled as module, 
it will fail in the kernel compilation.
    At the same time the acpi video driver must be loaded in order to load
the I915 driver. It is very hack.

    This patch is to weaken the dependency between the ACPI video driver and
i915 driver. But it still can do what we expected.
    a. the i915/acpi_video_register is defined in the video_detect.When the video is selected, 
it will be complied as built-in functions. If the video is not selected, it is defined as 
inline function.
    b. i915_video_register is called by i915 driver. And i915 video flag is set.
    c. acpi_video_register/unregister is called by acpi video driver. The acpi video bus driver
is hooked and the ACPI video flag is set.
    d. When the both flag bits are set, the acpi video bus driver will be registered.


Welcome the comments.
 
Best regards
   Yakui

Re: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[Matthew Garrett]

On Tue, Apr 14, 2009 at 11:04:35AM +0800, yakui_zhao wrote:

>     Patch 2/2:  weaken the tight dependency between the ACPI video driver and i915 driver
>     Now there exists the tight dependency between the ACPI video driver and i915 driver. 
> If the I915 driver is compiled as built-in module and the acpi video is compiled as module, 
> it will fail in the kernel compilation.
>     At the same time the acpi video driver must be loaded in order to load
> the I915 driver. It is very hack.

No, it's *correct*. In a modesetting world, i915 requires ACPI.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[yakui_zhao]

On Wed, 2009-04-15 at 03:35 +0800, Matthew Garrett wrote:
> On Tue, Apr 14, 2009 at 11:04:35AM +0800, yakui_zhao wrote:
> 
> >     Patch 2/2:  weaken the tight dependency between the ACPI video driver and i915 driver
> >     Now there exists the tight dependency between the ACPI video driver and i915 driver. 
> > If the I915 driver is compiled as built-in module and the acpi video is compiled as module, 
> > it will fail in the kernel compilation.
> >     At the same time the acpi video driver must be loaded in order to load
> > the I915 driver. It is very hack.
> 
> No, it's *correct*. In a modesetting world, i915 requires ACPI.
Why is the ACPI required by i915 when KMS is used? 
It seems that we still can use the KMS with ACPI disabled. 
   In such case it is unnecessary to load the acpi video driver. 

In fact what I have done is to weaken the tight dependency. When the
i915 driver is compiled as built-in and the acpi video is compiled as
module, the kernel compilation will be OK.  
    Before the acpi video driver is loaded, the system is still OK
except that there is no backlight control.

Is this reasonable?

Thanks.   
> 

Re: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[Matthew Garrett]

On Wed, Apr 15, 2009 at 09:12:41AM +0800, yakui_zhao wrote:
> > No, it's *correct*. In a modesetting world, i915 requires ACPI.
> Why is the ACPI required by i915 when KMS is used? 
> It seems that we still can use the KMS with ACPI disabled. 
>    In such case it is unnecessary to load the acpi video driver. 

You can, but things will break.

> In fact what I have done is to weaken the tight dependency. When the
> i915 driver is compiled as built-in and the acpi video is compiled as
> module, the kernel compilation will be OK.  
>     Before the acpi video driver is loaded, the system is still OK
> except that there is no backlight control.

I know that that's what you've done. I just don't think it's useful. 
Weakening the dependency means that you're driving the hardware outside 
of its design parameters, and I really don't see any reason to do that.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[Zhang Rui]

On Wed, 2009-04-15 at 03:35 +0800, Matthew Garrett wrote:
> On Tue, Apr 14, 2009 at 11:04:35AM +0800, yakui_zhao wrote:
> 
> >     Patch 2/2:  weaken the tight dependency between the ACPI video driver and i915 driver
> >     Now there exists the tight dependency between the ACPI video driver and i915 driver. 
> > If the I915 driver is compiled as built-in module and the acpi video is compiled as module, 
> > it will fail in the kernel compilation.
> >     At the same time the acpi video driver must be loaded in order to load
> > the I915 driver. It is very hack.
> 
> No, it's *correct*. In a modesetting world, i915 requires ACPI.

then I think the i915 driver should select ACPI video driver.

thanks,
rui

RE: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[Zhao, Yakui]

Will you please explain more clearly why ACPI video driver is required when KMS is used?
 

-----Original Message-----
From: Matthew Garrett [mailto:mjg59@srcf.ucam.org] 
Sent: 2009年4月15日 12:54
To: Zhao, Yakui
Cc: lenb@kernel.org; linux-acpi@vger.kernel.org
Subject: Re: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

On Wed, Apr 15, 2009 at 09:12:41AM +0800, yakui_zhao wrote:
> > No, it's *correct*. In a modesetting world, i915 requires ACPI.
> Why is the ACPI required by i915 when KMS is used? 
> It seems that we still can use the KMS with ACPI disabled. 
>    In such case it is unnecessary to load the acpi video driver. 

You can, but things will break.

> In fact what I have done is to weaken the tight dependency. When the
> i915 driver is compiled as built-in and the acpi video is compiled as
> module, the kernel compilation will be OK.  
>     Before the acpi video driver is loaded, the system is still OK
> except that there is no backlight control.

I know that that's what you've done. I just don't think it's useful. 
Weakening the dependency means that you're driving the hardware outside 
of its design parameters, and I really don't see any reason to do that.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[Matthew Garrett]

On Wed, Apr 15, 2009 at 09:21:16PM +0800, Zhao, Yakui wrote:
> Will you please explain more clearly why ACPI video driver is required when KMS is used?

Sorry, I should have been clearer. ACPI is required if the hardware 
supports opregion. Now that I've thought about it more, I think the 
patch I've just sent should remove the need to change the #ifdefs in the 
video driver.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[Matthew Garrett]

How about we just replace these with the following?

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 3a22eb9..cf7e7c2 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -71,6 +71,7 @@ config DRM_I915
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
 	select FB
+	select ACPI_VIDEO if ACPI
 	tristate "i915 driver"
 	help
 	  Choose this option if you have a system that has Intel 830M, 845G,

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[yakui_zhao]

On Thu, 2009-04-16 at 00:54 +0800, Matthew Garrett wrote:
> Sorry, I should have been clearer. ACPI is required if the hardware 
> supports opregion. Now that I've thought about it more, I think the 
> patch I've just sent should remove the need to change the #ifdefs in
> the 
> video driver
Agree.    If another patch from you is applied, it is unnecessary to
change the #ifdef in the acpi video driver.
    Please go ahead. Thanks.

Another issue is about the ACPI opregion. For the boxes that supports
ACPI opregion the brightness still can't be controlled even when the
acpi video driver is loaded correctly. Only after the i915 driver is
loaded, the ACPI backlight I/F can work well. In such case whether the
ACPI video driver can work well depends on the i915 driver.
    But whether the i915 driver can work well doesn't depend on the acpi
video driver. Right? 

At the same time a lot of people complain fail in the kernel compilation
if the i915 is compiled as builtin module and acpi video is compiled as
module. 

So we had better make some changes so that there is no failure about
kernel compilation.

thanks.

Re: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[Len Brown]

applied

thanks,
Len Brown, Intel Open Source Technology Center

On Wed, 15 Apr 2009, Matthew Garrett wrote:

> How about we just replace these with the following?
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 3a22eb9..cf7e7c2 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -71,6 +71,7 @@ config DRM_I915
>  	select FB_CFB_COPYAREA
>  	select FB_CFB_IMAGEBLIT
>  	select FB
> +	select ACPI_VIDEO if ACPI
>  	tristate "i915 driver"
>  	help
>  	  Choose this option if you have a system that has Intel 830M, 845G,
> 
> -- 
> Matthew Garrett | mjg59@srcf.ucam.org
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 0/2]: the patch set to weaken the dependency between ACPI video driver and i915 driver

[Andrew Morton]

(cc dri-devel)

On Thu, 23 Apr 2009 15:56:24 -0400 (EDT)
Len Brown <lenb@kernel.org> wrote:

> applied
> 
> thanks,
> Len Brown, Intel Open Source Technology Center
> 
> On Wed, 15 Apr 2009, Matthew Garrett wrote:
> 
> > How about we just replace these with the following?
> > 
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 3a22eb9..cf7e7c2 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -71,6 +71,7 @@ config DRM_I915
> >  	select FB_CFB_COPYAREA
> >  	select FB_CFB_IMAGEBLIT
> >  	select FB
> > +	select ACPI_VIDEO if ACPI
> >  	tristate "i915 driver"
> >  	help
> >  	  Choose this option if you have a system that has Intel 830M, 845G,

You applied this a week ago, but it isn't in mainline or linux-next.
We La described it as fixing a regression and David John said

: Can you forward this patch to Linus?  This regression breaks quite a
: few configs where ACPI Video is compiled as a module and i915 is built
: in.  This should have been in stable, but I guess it's too late now.

So...  wazzup?