* Re: [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
From: Luis R. Rodriguez @ 2015-04-02 21:49 UTC (permalink / raw)
To: Toshi Kani
Cc: Luis R. Rodriguez, luto, mingo, tglx, hpa, jgross, JBeulich, bp,
suresh.b.siddha, venkatesh.pallipadi, airlied, linux-kernel,
linux-fbdev, x86, xen-devel, Ingo Molnar, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Dave Hansen, Stefan Bader, konrad.wilk, ville.syrjala,
david.vrabel, bhelgaas, Roger Pau Monné, xen-devel
In-Reply-To: <20150327235630.GR5622@wotan.suse.de>
On Sat, Mar 28, 2015 at 12:56:30AM +0100, Luis R. Rodriguez wrote:
> On Fri, Mar 27, 2015 at 02:40:17PM -0600, Toshi Kani wrote:
> > On Fri, 2015-03-20 at 16:17 -0700, Luis R. Rodriguez wrote:
> > :
> > > @@ -734,6 +742,7 @@ void __init mtrr_bp_init(void)
> > > }
> > >
> > > if (mtrr_if) {
> > > + mtrr_enabled = true;
> > > set_num_var_ranges();
> > > init_table();
> > > if (use_intel()) {
> > get_mtrr_state();
> >
> > After setting mtrr_enabled to true, get_mtrr_state() reads
> > MSR_MTRRdefType and sets 'mtrr_state.enabled', which also indicates if
> > MTRRs are enabled or not on the system. So, potentially, we could have
> > a case that mtrr_enabled is set to true, but mtrr_state.enabled is set
> > to disabled when MTRRs are disabled by BIOS.
>
> Thanks for the review, in this case then we should update mtrr_enabled to false.
>
> > ps.
> > I recently cleaned up this part of the MTRR code in the patch below,
> > which is currently available in the -mm & -next trees.
> > https://lkml.org/lkml/2015/3/24/1063
>
> Great I will rebase and work with that and try to address this
> consideration you have raised.
OK I'll mesh in this change as well in my next respin:
diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index a83f27a..ecf7cb9 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -438,7 +438,7 @@ static void __init print_mtrr_state(void)
}
/* Grab all of the MTRR state for this CPU into *state */
-void __init get_mtrr_state(void)
+bool __init get_mtrr_state(void)
{
struct mtrr_var_range *vrs;
unsigned long flags;
@@ -482,6 +482,8 @@ void __init get_mtrr_state(void)
post_set();
local_irq_restore(flags);
+
+ return !!mtrr_state.enabled;
}
/* Some BIOS's are messed up and don't set all MTRRs the same! */
diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index ea5f363..f96195e 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -734,22 +742,25 @@ void __init mtrr_bp_init(void)
}
if (mtrr_if) {
+ mtrr_enabled = true;
set_num_var_ranges();
init_table();
if (use_intel()) {
- get_mtrr_state();
+ /* BIOS may override */
+ mtrr_enabled = get_mtrr_state();
if (mtrr_cleanup(phys_addr)) {
changed_by_mtrr_cleanup = 1;
@@ -745,11 +755,14 @@ void __init mtrr_bp_init(void)
}
}
}
+
+ if (!mtrr_enabled)
+ pr_info("mtrr: system does not support MTRR\n");
}
diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.h b/arch/x86/kernel/cpu/mtrr/mtrr.h
index df5e41f..951884d 100644
--- a/arch/x86/kernel/cpu/mtrr/mtrr.h
+++ b/arch/x86/kernel/cpu/mtrr/mtrr.h
@@ -51,7 +51,7 @@ void set_mtrr_prepare_save(struct set_mtrr_context *ctxt);
void fill_mtrr_var_range(unsigned int index,
u32 base_lo, u32 base_hi, u32 mask_lo, u32 mask_hi);
-void get_mtrr_state(void);
+bool get_mtrr_state(void);
extern void set_mtrr_ops(const struct mtrr_ops *ops);
^ permalink raw reply related
* Re: [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
From: Bjorn Helgaas @ 2015-04-02 22:09 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Konrad Rzeszutek Wilk, Andy Lutomirski, Ingo Molnar,
Thomas Gleixner, H. Peter Anvin, Juergen Gross, Jan Beulich,
Borislav Petkov, Suresh Siddha, venkatesh.pallipadi, Dave Airlie,
linux-kernel@vger.kernel.org, linux-fbdev, x86@kernel.org,
xen-devel@lists.xenproject.org, Ingo Molnar, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Dave Hansen, Stefan Bader, Ville Syrjälä, David Vrabel,
Toshi Kani, Roger Pau Monné, xen-devel
In-Reply-To: <20150402210253.GY5622@wotan.suse.de>
On Thu, Apr 2, 2015 at 4:02 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> ---
> It is possible to enable CONFIG_MTRR and CONFIG_X86_PAT
> and end up with a system with MTRR functionality disabled
> PAT functionality enabled.
This is missing a conjunction or something in "MTRR functionality
disabled PAT functionality."
Bjorn
^ permalink raw reply
* Re: [Xen-devel] [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
From: Luis R. Rodriguez @ 2015-04-02 22:12 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-fbdev, Daniel Vetter, Dave Hansen, Jan Beulich,
H. Peter Anvin, Ville Syrjälä, xen-devel, Suresh Siddha,
x86@kernel.org, Tomi Valkeinen, xen-devel@lists.xenproject.org,
Ingo Molnar, Borislav Petkov, Jean-Christophe Plagniol-Villard,
Antonino Daplas, Stefan Bader, Dave Airlie, Thomas Gleixner,
Ingo Molnar, Juergen Gross, Toshi Kani,
linux-kernel@vger.kernel.org, Andy Lutomirski, David Vrabel,
venkatesh.pallipadi, Roger Pau Monné
In-Reply-To: <CAErSpo5S7=CRyH0TKDfXzs-LWyP9kx_Xd=iFvEKyQZ5-humpxA@mail.gmail.com>
On Thu, Apr 2, 2015 at 3:09 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Apr 2, 2015 at 4:02 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>
>> ---
>> It is possible to enable CONFIG_MTRR and CONFIG_X86_PAT
>> and end up with a system with MTRR functionality disabled
>> PAT functionality enabled.
>
> This is missing a conjunction or something in "MTRR functionality
> disabled PAT functionality."
"and PAT functionality" -- fixed. Thanks.
Luis
^ permalink raw reply
* Re: [PATCH v1 06/47] mtrr: add __arch_phys_wc_add()
From: Bjorn Helgaas @ 2015-04-02 22:35 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Luis R. Rodriguez, Andy Lutomirski, Ingo Molnar, Thomas Gleixner,
H. Peter Anvin, Juergen Gross, Jan Beulich, Borislav Petkov,
Dave Airlie, linux-kernel@vger.kernel.org, linux-fbdev,
x86@kernel.org, xen-devel@lists.xenproject.org, Ingo Molnar,
Daniel Vetter, Antonino Daplas, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen
In-Reply-To: <20150402205520.GX5622@wotan.suse.de>
[-cc Venkatesh, Suresh]
On Thu, Apr 2, 2015 at 3:55 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Thu, Apr 02, 2015 at 03:21:22PM -0500, Bjorn Helgaas wrote:
>> On Fri, Mar 20, 2015 at 6:17 PM, Luis R. Rodriguez
>> <mcgrof@do-not-panic.com> wrote:
>> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
>> > This is *only* a transitive API -- and as such no new
>> > drivers are ever expected to use this.
>>
>> "transient"? Do you mean you intend to remove this API in the near future?
>
> That's correct, the problem is that in order to use PAT cleanly we'd need to
> change these drivers ...
I was just trying to ask whether you intended to write "transient"
instead of "transitive." But I'm not doing a very good job :)
Bjorn
^ permalink raw reply
* Re: [PATCH v1 06/47] mtrr: add __arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-04-02 22:54 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Andy Lutomirski, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
Juergen Gross, Jan Beulich, Borislav Petkov, Dave Airlie,
linux-kernel@vger.kernel.org, linux-fbdev, x86@kernel.org,
xen-devel@lists.xenproject.org, Ingo Molnar, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <CAErSpo5mBtLuMcRJBYuxXqGBJRkqzQDQiimVgeLk2k+ztyCCCw@mail.gmail.com>
On Thu, Apr 2, 2015 at 3:35 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> [-cc Venkatesh, Suresh]
>
> On Thu, Apr 2, 2015 at 3:55 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> On Thu, Apr 02, 2015 at 03:21:22PM -0500, Bjorn Helgaas wrote:
>>> On Fri, Mar 20, 2015 at 6:17 PM, Luis R. Rodriguez
>>> <mcgrof@do-not-panic.com> wrote:
>>> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
>>> > This is *only* a transitive API -- and as such no new
>>> > drivers are ever expected to use this.
>>>
>>> "transient"? Do you mean you intend to remove this API in the near future?
>>
>> That's correct, the problem is that in order to use PAT cleanly we'd need to
>> change these drivers ...
>
> I was just trying to ask whether you intended to write "transient"
> instead of "transitive." But I'm not doing a very good job :)
Yes, corrected, thanks.
Luis
^ permalink raw reply
* Re: [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
From: Toshi Kani @ 2015-04-02 23:52 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Luis R. Rodriguez, luto, mingo, tglx, hpa, jgross, JBeulich, bp,
suresh.b.siddha, venkatesh.pallipadi, airlied, linux-kernel,
linux-fbdev, x86, xen-devel, Ingo Molnar, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Dave Hansen, Stefan Bader, konrad.wilk, ville.syrjala,
david.vrabel, bhelgaas, Roger Pau Monné, xen-devel
In-Reply-To: <20150402214912.GZ5622@wotan.suse.de>
On Thu, 2015-04-02 at 23:49 +0200, Luis R. Rodriguez wrote:
> On Sat, Mar 28, 2015 at 12:56:30AM +0100, Luis R. Rodriguez wrote:
> > On Fri, Mar 27, 2015 at 02:40:17PM -0600, Toshi Kani wrote:
> > > On Fri, 2015-03-20 at 16:17 -0700, Luis R. Rodriguez wrote:
> > > :
> > > > @@ -734,6 +742,7 @@ void __init mtrr_bp_init(void)
> > > > }
> > > >
> > > > if (mtrr_if) {
> > > > + mtrr_enabled = true;
> > > > set_num_var_ranges();
> > > > init_table();
> > > > if (use_intel()) {
> > > get_mtrr_state();
> > >
> > > After setting mtrr_enabled to true, get_mtrr_state() reads
> > > MSR_MTRRdefType and sets 'mtrr_state.enabled', which also indicates if
> > > MTRRs are enabled or not on the system. So, potentially, we could have
> > > a case that mtrr_enabled is set to true, but mtrr_state.enabled is set
> > > to disabled when MTRRs are disabled by BIOS.
> >
> > Thanks for the review, in this case then we should update mtrr_enabled to false.
> >
> > > ps.
> > > I recently cleaned up this part of the MTRR code in the patch below,
> > > which is currently available in the -mm & -next trees.
> > > https://lkml.org/lkml/2015/3/24/1063
> >
> > Great I will rebase and work with that and try to address this
> > consideration you have raised.
>
> OK I'll mesh in this change as well in my next respin:
>
> diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
> index a83f27a..ecf7cb9 100644
> --- a/arch/x86/kernel/cpu/mtrr/generic.c
> +++ b/arch/x86/kernel/cpu/mtrr/generic.c
> @@ -438,7 +438,7 @@ static void __init print_mtrr_state(void)
> }
>
> /* Grab all of the MTRR state for this CPU into *state */
> -void __init get_mtrr_state(void)
> +bool __init get_mtrr_state(void)
> {
> struct mtrr_var_range *vrs;
> unsigned long flags;
> @@ -482,6 +482,8 @@ void __init get_mtrr_state(void)
>
> post_set();
> local_irq_restore(flags);
> +
> + return !!mtrr_state.enabled;
This should be:
return mtrr_state.enabled & MTRR_STATE_MTRR_ENABLED;
because the MTRR_STATE_MTRR_FIXED_ENABLED flag is ignored when the
MTRR_STATE_MTRR_ENABLED flag is clear.
Thanks,
-Toshi
^ permalink raw reply
* Re: [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
From: Luis R. Rodriguez @ 2015-04-03 1:08 UTC (permalink / raw)
To: Toshi Kani
Cc: Luis R. Rodriguez, luto, mingo, tglx, hpa, jgross, JBeulich, bp,
suresh.b.siddha, venkatesh.pallipadi, airlied, linux-kernel,
linux-fbdev, x86, xen-devel, Ingo Molnar, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Dave Hansen, Stefan Bader, konrad.wilk, ville.syrjala,
david.vrabel, bhelgaas, Roger Pau Monné, xen-devel
In-Reply-To: <1428018736.31093.102.camel@misato.fc.hp.com>
On Thu, Apr 02, 2015 at 05:52:16PM -0600, Toshi Kani wrote:
> On Thu, 2015-04-02 at 23:49 +0200, Luis R. Rodriguez wrote:
> > On Sat, Mar 28, 2015 at 12:56:30AM +0100, Luis R. Rodriguez wrote:
> > > On Fri, Mar 27, 2015 at 02:40:17PM -0600, Toshi Kani wrote:
> > > > On Fri, 2015-03-20 at 16:17 -0700, Luis R. Rodriguez wrote:
> > > > :
> > > > > @@ -734,6 +742,7 @@ void __init mtrr_bp_init(void)
> > > > > }
> > > > >
> > > > > if (mtrr_if) {
> > > > > + mtrr_enabled = true;
> > > > > set_num_var_ranges();
> > > > > init_table();
> > > > > if (use_intel()) {
> > > > get_mtrr_state();
> > > >
> > > > After setting mtrr_enabled to true, get_mtrr_state() reads
> > > > MSR_MTRRdefType and sets 'mtrr_state.enabled', which also indicates if
> > > > MTRRs are enabled or not on the system. So, potentially, we could have
> > > > a case that mtrr_enabled is set to true, but mtrr_state.enabled is set
> > > > to disabled when MTRRs are disabled by BIOS.
> > >
> > > Thanks for the review, in this case then we should update mtrr_enabled to false.
> > >
> > > > ps.
> > > > I recently cleaned up this part of the MTRR code in the patch below,
> > > > which is currently available in the -mm & -next trees.
> > > > https://lkml.org/lkml/2015/3/24/1063
> > >
> > > Great I will rebase and work with that and try to address this
> > > consideration you have raised.
> >
> > OK I'll mesh in this change as well in my next respin:
> >
> > diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
> > index a83f27a..ecf7cb9 100644
> > --- a/arch/x86/kernel/cpu/mtrr/generic.c
> > +++ b/arch/x86/kernel/cpu/mtrr/generic.c
> > @@ -438,7 +438,7 @@ static void __init print_mtrr_state(void)
> > }
> >
> > /* Grab all of the MTRR state for this CPU into *state */
> > -void __init get_mtrr_state(void)
> > +bool __init get_mtrr_state(void)
> > {
> > struct mtrr_var_range *vrs;
> > unsigned long flags;
> > @@ -482,6 +482,8 @@ void __init get_mtrr_state(void)
> >
> > post_set();
> > local_irq_restore(flags);
> > +
> > + return !!mtrr_state.enabled;
>
> This should be:
> return mtrr_state.enabled & MTRR_STATE_MTRR_ENABLED;
>
> because the MTRR_STATE_MTRR_FIXED_ENABLED flag is ignored when the
> MTRR_STATE_MTRR_ENABLED flag is clear.
Thanks, I've used
return !!(mtrr_state.enabled & MTRR_STATE_MTRR_ENABLED);
Amended.
Luis
^ permalink raw reply
* Re: [PATCH v2] video: mxsfb: Make sure axi clock is enabled when accessing registers
From: Liu Ying @ 2015-04-03 3:39 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Liu Ying, linux-fbdev, Peter Chen,
Jean-Christophe Plagniol-Villard, Fabio Estevam,
Greg Kroah-Hartman, linux-kernel, stable
In-Reply-To: <550C03D3.7090504@ti.com>
2015-03-20 19:26 GMT+08:00 Tomi Valkeinen <tomi.valkeinen@ti.com>:
> On 11/03/15 05:03, Liu Ying wrote:
>
>>> Why do you check for host->enabled here, but not elsewhere?
>>
>> We need this check here to make sure the axi clock reference count is no greater
>> than 1. Looking at the context of mxsfb_set_par(), mxsfb_restore_mode() and
>
> Why is that? The clock framework handles ref counting for you. All the
> driver needs to take care of is to call as many times disable as it
> calls enable.
Okay. I'll remove the check and send another version.
Regards,
Liu Ying
>
> Tomi
>
>
^ permalink raw reply
* [PATCH v4] video: mxsfb: Make sure axi clock is enabled when accessing registers
From: Liu Ying @ 2015-04-03 4:51 UTC (permalink / raw)
To: linux-fbdev
Cc: Peter Chen, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Fabio Estevam, Greg Kroah-Hartman, linux-kernel, stable
The LCDIF engines embedded in i.MX6sl and i.MX6sx SoCs need the axi clock
as the engine's system clock. The clock should be enabled when accessing
LCDIF registers, otherwise the kernel would hang up. We should also keep
the clock enabled when the engine is being active to scan out frames from
memory. This patch makes sure the axi clock is enabled when accessing
registers so that the kernel hang up issue can be fixed.
Reported-by: Peter Chen <peter.chen@freescale.com>
Tested-by: Peter Chen <peter.chen@freescale.com>
Cc: <stable@vger.kernel.org> # 3.19+
Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
v3->v4:
* To address Tomi's comment, enable/disable the axi clock in
mxsfb_pan_display() directly instead of checking the host->enabled flag.
v2->v3:
* To address Tomi's comment, improve the commit message only.
v1->v2:
* Add 'Tested-by: Peter Chen <peter.chen@freescale.com>' tag.
* Add 'Cc: <stable@vger.kernel.org> # 3.19+' tag.
drivers/video/fbdev/mxsfb.c | 68 +++++++++++++++++++++++++++++++++++----------
1 file changed, 54 insertions(+), 14 deletions(-)
diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
index f8ac4a4..0f64165 100644
--- a/drivers/video/fbdev/mxsfb.c
+++ b/drivers/video/fbdev/mxsfb.c
@@ -316,6 +316,18 @@ static int mxsfb_check_var(struct fb_var_screeninfo *var,
return 0;
}
+static inline void mxsfb_enable_axi_clk(struct mxsfb_info *host)
+{
+ if (host->clk_axi)
+ clk_prepare_enable(host->clk_axi);
+}
+
+static inline void mxsfb_disable_axi_clk(struct mxsfb_info *host)
+{
+ if (host->clk_axi)
+ clk_disable_unprepare(host->clk_axi);
+}
+
static void mxsfb_enable_controller(struct fb_info *fb_info)
{
struct mxsfb_info *host = to_imxfb_host(fb_info);
@@ -333,14 +345,13 @@ static void mxsfb_enable_controller(struct fb_info *fb_info)
}
}
- if (host->clk_axi)
- clk_prepare_enable(host->clk_axi);
-
if (host->clk_disp_axi)
clk_prepare_enable(host->clk_disp_axi);
clk_prepare_enable(host->clk);
clk_set_rate(host->clk, PICOS2KHZ(fb_info->var.pixclock) * 1000U);
+ mxsfb_enable_axi_clk(host);
+
/* if it was disabled, re-enable the mode again */
writel(CTRL_DOTCLK_MODE, host->base + LCDC_CTRL + REG_SET);
@@ -380,11 +391,11 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
reg = readl(host->base + LCDC_VDCTRL4);
writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
+ mxsfb_disable_axi_clk(host);
+
clk_disable_unprepare(host->clk);
if (host->clk_disp_axi)
clk_disable_unprepare(host->clk_disp_axi);
- if (host->clk_axi)
- clk_disable_unprepare(host->clk_axi);
host->enabled = 0;
@@ -421,6 +432,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
mxsfb_disable_controller(fb_info);
}
+ mxsfb_enable_axi_clk(host);
+
/* clear the FIFOs */
writel(CTRL1_FIFO_CLEAR, host->base + LCDC_CTRL1 + REG_SET);
@@ -438,6 +451,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
ctrl |= CTRL_SET_WORD_LENGTH(3);
switch (host->ld_intf_width) {
case STMLCDIF_8BIT:
+ mxsfb_disable_axi_clk(host);
dev_err(&host->pdev->dev,
"Unsupported LCD bus width mapping\n");
return -EINVAL;
@@ -451,6 +465,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
writel(CTRL1_SET_BYTE_PACKAGING(0x7), host->base + LCDC_CTRL1);
break;
default:
+ mxsfb_disable_axi_clk(host);
dev_err(&host->pdev->dev, "Unhandled color depth of %u\n",
fb_info->var.bits_per_pixel);
return -EINVAL;
@@ -504,6 +519,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
fb_info->fix.line_length * fb_info->var.yoffset,
host->base + host->devdata->next_buf);
+ mxsfb_disable_axi_clk(host);
+
if (reenable)
mxsfb_enable_controller(fb_info);
@@ -582,10 +599,14 @@ static int mxsfb_pan_display(struct fb_var_screeninfo *var,
offset = fb_info->fix.line_length * var->yoffset;
+ mxsfb_enable_axi_clk(host);
+
/* update on next VSYNC */
writel(fb_info->fix.smem_start + offset,
host->base + host->devdata->next_buf);
+ mxsfb_disable_axi_clk(host);
+
return 0;
}
@@ -608,13 +629,17 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
unsigned line_count;
unsigned period;
unsigned long pa, fbsize;
- int bits_per_pixel, ofs;
+ int bits_per_pixel, ofs, ret = 0;
u32 transfer_count, vdctrl0, vdctrl2, vdctrl3, vdctrl4, ctrl;
+ mxsfb_enable_axi_clk(host);
+
/* Only restore the mode when the controller is running */
ctrl = readl(host->base + LCDC_CTRL);
- if (!(ctrl & CTRL_RUN))
- return -EINVAL;
+ if (!(ctrl & CTRL_RUN)) {
+ ret = -EINVAL;
+ goto err;
+ }
vdctrl0 = readl(host->base + LCDC_VDCTRL0);
vdctrl2 = readl(host->base + LCDC_VDCTRL2);
@@ -635,7 +660,8 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
break;
case 1:
default:
- return -EINVAL;
+ ret = -EINVAL;
+ goto err;
}
fb_info->var.bits_per_pixel = bits_per_pixel;
@@ -673,10 +699,14 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
pa = readl(host->base + host->devdata->cur_buf);
fbsize = fb_info->fix.line_length * vmode->yres;
- if (pa < fb_info->fix.smem_start)
- return -EINVAL;
- if (pa + fbsize > fb_info->fix.smem_start + fb_info->fix.smem_len)
- return -EINVAL;
+ if (pa < fb_info->fix.smem_start) {
+ ret = -EINVAL;
+ goto err;
+ }
+ if (pa + fbsize > fb_info->fix.smem_start + fb_info->fix.smem_len) {
+ ret = -EINVAL;
+ goto err;
+ }
ofs = pa - fb_info->fix.smem_start;
if (ofs) {
memmove(fb_info->screen_base, fb_info->screen_base + ofs, fbsize);
@@ -689,7 +719,11 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
clk_prepare_enable(host->clk);
host->enabled = 1;
- return 0;
+err:
+ if (ret)
+ mxsfb_disable_axi_clk(host);
+
+ return ret;
}
static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
@@ -915,7 +949,9 @@ static int mxsfb_probe(struct platform_device *pdev)
}
if (!host->enabled) {
+ mxsfb_enable_axi_clk(host);
writel(0, host->base + LCDC_CTRL);
+ mxsfb_disable_axi_clk(host);
mxsfb_set_par(fb_info);
mxsfb_enable_controller(fb_info);
}
@@ -954,11 +990,15 @@ static void mxsfb_shutdown(struct platform_device *pdev)
struct fb_info *fb_info = platform_get_drvdata(pdev);
struct mxsfb_info *host = to_imxfb_host(fb_info);
+ mxsfb_enable_axi_clk(host);
+
/*
* Force stop the LCD controller as keeping it running during reboot
* might interfere with the BootROM's boot mode pads sampling.
*/
writel(CTRL_RUN, host->base + LCDC_CTRL + REG_CLR);
+
+ mxsfb_disable_axi_clk(host);
}
static struct platform_driver mxsfb_driver = {
--
2.1.0
^ permalink raw reply related
* Questions about simplefb and hand-over
From: Johannes Pointner @ 2015-04-03 8:57 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Hans de Goede, devicetree
Hello all,
I'd like to use the simplefb in combination with tilcdc drm driver
with a TI AM335x processor. I got the u-boot to simplefb part working,
but I'm not sure how to do the hand-over part.
Is it yet possible to do that? Does the tilcdc drm driver need
additional functionality to support that? Or are there other things I
have to take into account?
Any suggestions would be highly appreciated!
Thanks in Advance,
Hannes
^ permalink raw reply
* Re: Questions about simplefb and hand-over
From: Hans de Goede @ 2015-04-03 9:02 UTC (permalink / raw)
To: Johannes Pointner, linux-fbdev-u79uwXL29TY76Z2rM5mHXA, devicetree,
David Herrmann
In-Reply-To: <CAJKbBDCCU5zBAFLf_KH-nYLzJYbAU9S9z6Jfe8F3w5SY+-ZCuQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi,
On 03-04-15 10:57, Johannes Pointner wrote:
> Hello all,
>
> I'd like to use the simplefb in combination with tilcdc drm driver
> with a TI AM335x processor. I got the u-boot to simplefb part working,
> but I'm not sure how to do the hand-over part.
>
> Is it yet possible to do that? Does the tilcdc drm driver need
> additional functionality to support that? Or are there other things I
> have to take into account?
>
> Any suggestions would be highly appreciated!
There is a remove_conflicting_framebuffers call which is already used
on x86 to remove e.g. efifb when a kms driver loads, the plan for
simplefb / arm is to do the same. That is where we stand now, we've
a plan but you're the first person to actually try this.
Note that you may want to use simpledrm by David Herrmann (added to the
Cc) instead of simplefb as that will give you a kms driver right from
the start, simpledrm is not yet upstream but should go upstream soon.
Regards,
Hans
^ permalink raw reply
* [PATCH] fbdev: sh_mobile_lcdc: Fix destruction of uninitialized mutex
From: Geert Uytterhoeven @ 2015-04-03 12:50 UTC (permalink / raw)
To: linux-fbdev
If sh_mobile_lcdc_probe() fails after the allocation of driver-private
data, but before the initialization of all channels, a warning will be
printed due to the destruction of an uninitialized mutex:
WARNING: CPU: 0 PID: 1 at kernel/locking/mutex-debug.c:116 mutex_destroy+0x5c/0x7c()
DEBUG_LOCKS_WARN_ON(mutex_is_locked(lock))
...
Backtrace:
...
[<c00425b4>] (mutex_destroy) from [<c01d5858>] (sh_mobile_lcdc_remove+0x1bc/0x230)
r4:df6a4800 r3:00000000
[<c01d569c>] (sh_mobile_lcdc_remove) from [<c01d6620>] (sh_mobile_lcdc_probe+0xd54/0xe28)
Move the initialization of the mutexes from sh_mobile_lcdc_channel_init()
to immediately after the allocation of driver-private data to fix this.
Note that the interrupt number is moved to a new variable "irq", so we
can reuse the existing variable "i" for iterating over the channels.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/video/fbdev/sh_mobile_lcdcfb.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index d3013cd9f9769663..97141da154c663b8 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -2605,7 +2605,6 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch)
unsigned int max_size;
unsigned int i;
- mutex_init(&ch->open_lock);
ch->notify = sh_mobile_lcdc_display_notify;
/* Validate the format. */
@@ -2704,7 +2703,7 @@ static int sh_mobile_lcdc_probe(struct platform_device *pdev)
struct resource *res;
int num_channels;
int error;
- int i;
+ int irq, i;
if (!pdata) {
dev_err(&pdev->dev, "no platform data defined\n");
@@ -2712,8 +2711,8 @@ static int sh_mobile_lcdc_probe(struct platform_device *pdev)
}
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- i = platform_get_irq(pdev, 0);
- if (!res || i < 0) {
+ irq = platform_get_irq(pdev, 0);
+ if (!res || irq < 0) {
dev_err(&pdev->dev, "cannot get platform resources\n");
return -ENOENT;
}
@@ -2726,16 +2725,18 @@ static int sh_mobile_lcdc_probe(struct platform_device *pdev)
priv->dev = &pdev->dev;
priv->meram_dev = pdata->meram_dev;
+ for (i = 0; i < ARRAY_SIZE(priv->ch); i++)
+ mutex_init(&priv->ch[i].open_lock);
platform_set_drvdata(pdev, priv);
- error = request_irq(i, sh_mobile_lcdc_irq, 0,
+ error = request_irq(irq, sh_mobile_lcdc_irq, 0,
dev_name(&pdev->dev), priv);
if (error) {
dev_err(&pdev->dev, "unable to request irq\n");
goto err1;
}
- priv->irq = i;
+ priv->irq = irq;
atomic_set(&priv->hw_usecnt, -1);
for (i = 0, num_channels = 0; i < ARRAY_SIZE(pdata->ch); i++) {
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] fbdev: sh_mobile_lcdc: Fix destruction of uninitialized mutex
From: Laurent Pinchart @ 2015-04-04 12:38 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1428065428-25473-1-git-send-email-geert+renesas@glider.be>
Hi Geert,
Thank you for the patch.
On Friday 03 April 2015 14:50:28 Geert Uytterhoeven wrote:
> If sh_mobile_lcdc_probe() fails after the allocation of driver-private
> data, but before the initialization of all channels, a warning will be
> printed due to the destruction of an uninitialized mutex:
>
> WARNING: CPU: 0 PID: 1 at kernel/locking/mutex-debug.c:116
> mutex_destroy+0x5c/0x7c() DEBUG_LOCKS_WARN_ON(mutex_is_locked(lock))
> ...
> Backtrace:
> ...
> [<c00425b4>] (mutex_destroy) from [<c01d5858>]
> (sh_mobile_lcdc_remove+0x1bc/0x230) r4:df6a4800 r3:00000000
> [<c01d569c>] (sh_mobile_lcdc_remove) from [<c01d6620>]
> (sh_mobile_lcdc_probe+0xd54/0xe28)
>
> Move the initialization of the mutexes from sh_mobile_lcdc_channel_init()
> to immediately after the allocation of driver-private data to fix this.
>
> Note that the interrupt number is moved to a new variable "irq", so we
> can reuse the existing variable "i" for iterating over the channels.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tomi, could you please pick this up ?
> ---
> drivers/video/fbdev/sh_mobile_lcdcfb.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> b/drivers/video/fbdev/sh_mobile_lcdcfb.c index
> d3013cd9f9769663..97141da154c663b8 100644
> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> @@ -2605,7 +2605,6 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan
> *ch) unsigned int max_size;
> unsigned int i;
>
> - mutex_init(&ch->open_lock);
> ch->notify = sh_mobile_lcdc_display_notify;
>
> /* Validate the format. */
> @@ -2704,7 +2703,7 @@ static int sh_mobile_lcdc_probe(struct platform_device
> *pdev) struct resource *res;
> int num_channels;
> int error;
> - int i;
> + int irq, i;
>
> if (!pdata) {
> dev_err(&pdev->dev, "no platform data defined\n");
> @@ -2712,8 +2711,8 @@ static int sh_mobile_lcdc_probe(struct platform_device
> *pdev) }
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - i = platform_get_irq(pdev, 0);
> - if (!res || i < 0) {
> + irq = platform_get_irq(pdev, 0);
> + if (!res || irq < 0) {
> dev_err(&pdev->dev, "cannot get platform resources\n");
> return -ENOENT;
> }
> @@ -2726,16 +2725,18 @@ static int sh_mobile_lcdc_probe(struct
> platform_device *pdev)
>
> priv->dev = &pdev->dev;
> priv->meram_dev = pdata->meram_dev;
> + for (i = 0; i < ARRAY_SIZE(priv->ch); i++)
> + mutex_init(&priv->ch[i].open_lock);
> platform_set_drvdata(pdev, priv);
>
> - error = request_irq(i, sh_mobile_lcdc_irq, 0,
> + error = request_irq(irq, sh_mobile_lcdc_irq, 0,
> dev_name(&pdev->dev), priv);
> if (error) {
> dev_err(&pdev->dev, "unable to request irq\n");
> goto err1;
> }
>
> - priv->irq = i;
> + priv->irq = irq;
> atomic_set(&priv->hw_usecnt, -1);
>
> for (i = 0, num_channels = 0; i < ARRAY_SIZE(pdata->ch); i++) {
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH] staging: sm7xxfb: Fix sparse warning
From: Nickolaus Woodruff @ 2015-04-04 19:22 UTC (permalink / raw)
To: sudipmmukherjee, teddywang, gregkh; +Cc: linux-fbdev, devel, linux-kernel
This patch fixes the following sparse warning in sm7xx.h:
drivers/staging/sm7xxfb/sm7xx.h:122:17: warning: symbol 'vgamode'
was not declared. Should it be static?
Signed-off-by: Nickolaus Woodruff <nickolauswoodruff@gmail.com>
---
drivers/staging/sm7xxfb/sm7xx.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/sm7xxfb/sm7xx.h b/drivers/staging/sm7xxfb/sm7xx.h
index 7cc1896..c5d6253 100644
--- a/drivers/staging/sm7xxfb/sm7xx.h
+++ b/drivers/staging/sm7xxfb/sm7xx.h
@@ -119,7 +119,7 @@ struct ModeInit {
/**********************************************************************
SM712 Mode table.
**********************************************************************/
-struct ModeInit vgamode[] = {
+static struct ModeInit vgamode[] = {
{
/* mode#0: 640 x 480 16Bpp 60Hz */
640, 480, 16, 60,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] staging: sm7xxfb: Fix sparse warning
From: Giedrius Statkevičius @ 2015-04-04 19:31 UTC (permalink / raw)
To: Nickolaus Woodruff
Cc: sudipmmukherjee, teddywang, gregkh, devel, linux-fbdev,
linux-kernel
In-Reply-To: <20150404165120.GA3111@Caladan>
On Sat, 4 Apr 2015, Nickolaus Woodruff wrote:
> This patch fixes the following sparse warning in sm7xx.h:
>
> drivers/staging/sm7xxfb/sm7xx.h:122:17: warning: symbol 'vgamode'
> was not declared. Should it be static?
>
> Signed-off-by: Nickolaus Woodruff <nickolauswoodruff@gmail.com>
> ---
> drivers/staging/sm7xxfb/sm7xx.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/sm7xxfb/sm7xx.h b/drivers/staging/sm7xxfb/sm7xx.h
> index 7cc1896..c5d6253 100644
> --- a/drivers/staging/sm7xxfb/sm7xx.h
> +++ b/drivers/staging/sm7xxfb/sm7xx.h
> @@ -119,7 +119,7 @@ struct ModeInit {
> /**********************************************************************
> SM712 Mode table.
> **********************************************************************/
> -struct ModeInit vgamode[] = {
> +static struct ModeInit vgamode[] = {
Someone already fixed this:
https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/sm7xxfb/sm7xx.h?h=staging-testing#n122
Please work against staging-testing branch of the staging tree.
Su pagarba / Regards,
Giedrius
^ permalink raw reply
* [PATCH] Staging: sm750fb: Remove extra parentheses around right bit shift operation
From: Amitoj Kaur Chawla @ 2015-04-05 1:53 UTC (permalink / raw)
To: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
linux-kernel
Removes extra parentheses around bitwise right shift operation.
The issue was detected using the following coccinelle script.
@@
expression e, e1;
constant c;
@@
e -(e1
+e1
<<
-c);
+c;
@@
identifier i;
constant c;
type t;
expression e;
@@
t i -(e
+e
<<
-c);
+c;
@@
expression e, e1;
identifier f;
constant c;
@@
e1 = f(...,
-(e
+e
<<
-c)
+c
,...);
Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 4074375..626b841 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -737,7 +737,7 @@ static int sm750fb_set_drv(struct lynxfb_par * par)
output->paths = sm750_crt;
crtc->channel = sm750_secondary;
/* not consider of padding stuffs for oScreen,need fix*/
- crtc->oScreen = (share->vidmem_size >> 1);
+ crtc->oScreen = share->vidmem_size >> 1;
crtc->vScreen = share->pvMem + crtc->oScreen;
}
break;
@@ -751,7 +751,7 @@ static int sm750fb_set_drv(struct lynxfb_par * par)
output->paths = sm750_crt;
crtc->channel = sm750_primary;
/* not consider of padding stuffs for oScreen,need fix*/
- crtc->oScreen = (share->vidmem_size >> 1);
+ crtc->oScreen = share->vidmem_size >> 1;
crtc->vScreen = share->pvMem + crtc->oScreen;
}
break;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] Staging: sm750fb: Remove extra parentheses around right bit shift operation
From: Sudip Mukherjee @ 2015-04-06 9:30 UTC (permalink / raw)
To: Amitoj Kaur Chawla; +Cc: teddy.wang, gregkh, linux-fbdev, devel, linux-kernel
In-Reply-To: <20150405014147.GA9738@amitoj-Inspiron-3542>
On Sun, Apr 05, 2015 at 07:11:47AM +0530, Amitoj Kaur Chawla wrote:
> Removes extra parentheses around bitwise right shift operation.
this is not applying. please work against staging-testing.
regards
sudip
>
^ permalink raw reply
* Re: [PATCH] Staging: sm750fb: Remove extra parentheses around right bit shift operation
From: Dan Carpenter @ 2015-04-07 8:00 UTC (permalink / raw)
To: Amitoj Kaur Chawla
Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
linux-kernel
In-Reply-To: <20150405014147.GA9738@amitoj-Inspiron-3542>
Some people like to put parenthesis around stuff. It's harmless here.
regards,
dan carpenter
^ permalink raw reply
* simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Pavel Machek @ 2015-04-07 12:12 UTC (permalink / raw)
To: Marek Vasut, kernel list, dinh.linux
Cc: plagnioj, tomi.valkeinen, grant.likely, robh+dt, jg1.han,
robdclark, geert, linux-fbdev, devicetree
Hi!
I have an socfpga board, which uses has simple framebuffer implemented
in the FPGA. On 3.15, framebuffer is fast:
root@wagabuibui:~# time cat /dev/fb0 > /dev/null
real 0m 0.00s
user 0m 0.00s
sys 0m 0.00s
on 3.18, this takes 220msec. Similar slowdown exists for
writes. Simple framebuffer did not change at all between 3.15 and
3.18; resource flags of the framebuffer are still same (0x200).
If I enable caching on 3.18, it speeds up a bit, to 70msec or
so... Which means problem is not only in caching.
Any ideas?
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Geert Uytterhoeven @ 2015-04-07 12:19 UTC (permalink / raw)
To: Pavel Machek
Cc: Marek Vasut, kernel list, Dinh Nguyen,
Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen, Grant Likely,
Rob Herring, Jingoo Han, Rob Clark, Linux Fbdev development list,
devicetree@vger.kernel.org
In-Reply-To: <20150407121247.GA29497@amd>
Hi Pavel,
On Tue, Apr 7, 2015 at 2:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
> I have an socfpga board, which uses has simple framebuffer implemented
> in the FPGA. On 3.15, framebuffer is fast:
>
> root@wagabuibui:~# time cat /dev/fb0 > /dev/null
> real 0m 0.00s
> user 0m 0.00s
> sys 0m 0.00s
>
> on 3.18, this takes 220msec. Similar slowdown exists for
> writes. Simple framebuffer did not change at all between 3.15 and
> 3.18; resource flags of the framebuffer are still same (0x200).
>
> If I enable caching on 3.18, it speeds up a bit, to 70msec or
> so... Which means problem is not only in caching.
>
> Any ideas?
My first guess was commit 67dc0d4758e5 ("vt_buffer: drop console buffer
copying optimisations"), but this was introduced only in v4.0-rc1.
Just in case you encounter another performance regression after upgrading
to a more modern kernel ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Fix destruction of uninitialized mutex
From: Tomi Valkeinen @ 2015-04-07 13:25 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1428065428-25473-1-git-send-email-geert+renesas@glider.be>
[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]
On 03/04/15 15:50, Geert Uytterhoeven wrote:
> If sh_mobile_lcdc_probe() fails after the allocation of driver-private
> data, but before the initialization of all channels, a warning will be
> printed due to the destruction of an uninitialized mutex:
>
> WARNING: CPU: 0 PID: 1 at kernel/locking/mutex-debug.c:116 mutex_destroy+0x5c/0x7c()
> DEBUG_LOCKS_WARN_ON(mutex_is_locked(lock))
> ...
> Backtrace:
> ...
> [<c00425b4>] (mutex_destroy) from [<c01d5858>] (sh_mobile_lcdc_remove+0x1bc/0x230)
> r4:df6a4800 r3:00000000
> [<c01d569c>] (sh_mobile_lcdc_remove) from [<c01d6620>] (sh_mobile_lcdc_probe+0xd54/0xe28)
>
> Move the initialization of the mutexes from sh_mobile_lcdc_channel_init()
> to immediately after the allocation of driver-private data to fix this.
>
> Note that the interrupt number is moved to a new variable "irq", so we
> can reuse the existing variable "i" for iterating over the channels.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> drivers/video/fbdev/sh_mobile_lcdcfb.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
Thanks, queued for 4.1.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFT PATCHv2] drm/exynos: Enable DP clock to fix display on Exynos5250 and other
From: Inki Dae @ 2015-04-07 13:32 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Jingoo Han, Joonyoung Shim, Seung-Woo Kim, Kyungmin Park,
David Airlie, Kukjin Kim, dri-devel, linux-arm-kernel,
linux-samsung-soc, linux-kernel, linux-fbdev, Marek Szyprowski,
Andrzej Hajda, Javier Martinez Canillas, stable
In-Reply-To: <1427472488-21454-1-git-send-email-k.kozlowski@samsung.com>
On 2015년 03월 28일 01:08, Krzysztof Kozlowski wrote:
> After adding display power domain for Exynos5250 in commit
> 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250") the
> display on Chromebook Snow and others stopped working after boot.
>
> The reason for this suggested Andrzej Hajda: the DP clock was disabled.
> This clock is required by Display Port and is enabled by bootloader.
> However when FIMD driver probing was deferred, the display power domain
> was turned off. This effectively reset the value of DP clock enable
> register.
Applied.
Thanks,
Inki Dae
>
> When exynos-dp is later probed, the clock is not enabled and display is
> not properly configured:
>
> exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
> exynos-dp 145b0000.dp-controller: unable to config video
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reported-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Fixes: 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250")
> Cc: <stable@vger.kernel.org>
>
> ---
>
> This should fix issue reported by Javier [1][2].
>
> Tested on Chromebook Snow (Exynos 5250). More testing would be great,
> especially on other Exynos 5xxx products.
>
> [1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/43889
> [2] http://thread.gmane.org/gmane.linux.ports.arm.kernel/400290
>
> Changes since v1:
> 1. Added missing exynos_drm_fimd.h.
> ---
> drivers/gpu/drm/exynos/exynos_dp_core.c | 10 ++++++++++
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 19 +++++++++++++++++++
> drivers/gpu/drm/exynos/exynos_drm_fimd.h | 15 +++++++++++++++
> include/video/samsung_fimd.h | 6 ++++++
> 4 files changed, 50 insertions(+)
> create mode 100644 drivers/gpu/drm/exynos/exynos_drm_fimd.h
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index bf17a60b40ed..1dbfba58f909 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> @@ -32,10 +32,16 @@
> #include <drm/bridge/ptn3460.h>
>
> #include "exynos_dp_core.h"
> +#include "exynos_drm_fimd.h"
>
> #define ctx_from_connector(c) container_of(c, struct exynos_dp_device, \
> connector)
>
> +static inline struct exynos_drm_crtc *dp_to_crtc(struct exynos_dp_device *dp)
> +{
> + return to_exynos_crtc(dp->encoder->crtc);
> +}
> +
> static inline struct exynos_dp_device *
> display_to_dp(struct exynos_drm_display *d)
> {
> @@ -1070,6 +1076,8 @@ static void exynos_dp_poweron(struct exynos_dp_device *dp)
> }
> }
>
> + fimd_dp_clock_enable(dp_to_crtc(dp), true);
> +
> clk_prepare_enable(dp->clock);
> exynos_dp_phy_init(dp);
> exynos_dp_init_dp(dp);
> @@ -1094,6 +1102,8 @@ static void exynos_dp_poweroff(struct exynos_dp_device *dp)
> exynos_dp_phy_exit(dp);
> clk_disable_unprepare(dp->clock);
>
> + fimd_dp_clock_enable(dp_to_crtc(dp), false);
> +
> if (dp->panel) {
> if (drm_panel_unprepare(dp->panel))
> DRM_ERROR("failed to turnoff the panel\n");
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index c300e22da8ac..bdf0818dc8f5 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -32,6 +32,7 @@
> #include "exynos_drm_fbdev.h"
> #include "exynos_drm_crtc.h"
> #include "exynos_drm_iommu.h"
> +#include "exynos_drm_fimd.h"
>
> /*
> * FIMD stands for Fully Interactive Mobile Display and
> @@ -1231,6 +1232,24 @@ static int fimd_remove(struct platform_device *pdev)
> return 0;
> }
>
> +void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
> +{
> + struct fimd_context *ctx = crtc->ctx;
> + u32 val;
> +
> + /*
> + * Only Exynos 5250, 5260, 5410 and 542x requires enabling DP/MIE
> + * clock. On these SoCs the bootloader may enable it but any
> + * power domain off/on will reset it to disable state.
> + */
> + if (ctx->driver_data != &exynos5_fimd_driver_data)
> + return;
> +
> + val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
> + writel(DP_MIE_CLK_DP_ENABLE, ctx->regs + DP_MIE_CLKCON);
> +}
> +EXPORT_SYMBOL_GPL(fimd_dp_clock_enable);
> +
> struct platform_driver fimd_driver = {
> .probe = fimd_probe,
> .remove = fimd_remove,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.h b/drivers/gpu/drm/exynos/exynos_drm_fimd.h
> new file mode 100644
> index 000000000000..b4fcaa568456
> --- /dev/null
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.h
> @@ -0,0 +1,15 @@
> +/*
> + * Copyright (c) 2015 Samsung Electronics Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#ifndef _EXYNOS_DRM_FIMD_H_
> +#define _EXYNOS_DRM_FIMD_H_
> +
> +extern void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable);
> +
> +#endif /* _EXYNOS_DRM_FIMD_H_ */
> diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h
> index a20e4a3a8b15..847a0a2b399c 100644
> --- a/include/video/samsung_fimd.h
> +++ b/include/video/samsung_fimd.h
> @@ -436,6 +436,12 @@
> #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0)
> #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0)
>
> +/* Display port clock control */
> +#define DP_MIE_CLKCON 0x27c
> +#define DP_MIE_CLK_DISABLE 0x0
> +#define DP_MIE_CLK_DP_ENABLE 0x2
> +#define DP_MIE_CLK_MIE_ENABLE 0x3
> +
> /* Notes on per-window bpp settings
> *
> * Value Win0 Win1 Win2 Win3 Win 4
>
^ permalink raw reply
* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Marek Vasut @ 2015-04-07 14:24 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Pavel Machek, kernel list, Dinh Nguyen,
Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen, Grant Likely,
Rob Herring, Jingoo Han, Rob Clark, Linux Fbdev development list,
devicetree@vger.kernel.org
In-Reply-To: <CAMuHMdXWRx7bKWhQuWH05CbPTXPy5imCUcW1xUf2E8f7BtF9NQ@mail.gmail.com>
On Tuesday, April 07, 2015 at 02:19:33 PM, Geert Uytterhoeven wrote:
> Hi Pavel,
>
> On Tue, Apr 7, 2015 at 2:12 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > I have an socfpga board, which uses has simple framebuffer implemented
> > in the FPGA. On 3.15, framebuffer is fast:
> >
> > root@wagabuibui:~# time cat /dev/fb0 > /dev/null
> > real 0m 0.00s
> > user 0m 0.00s
> > sys 0m 0.00s
> >
> > on 3.18, this takes 220msec. Similar slowdown exists for
> > writes. Simple framebuffer did not change at all between 3.15 and
> > 3.18; resource flags of the framebuffer are still same (0x200).
> >
> > If I enable caching on 3.18, it speeds up a bit, to 70msec or
> > so... Which means problem is not only in caching.
> >
> > Any ideas?
>
> My first guess was commit 67dc0d4758e5 ("vt_buffer: drop console buffer
> copying optimisations"), but this was introduced only in v4.0-rc1.
>
> Just in case you encounter another performance regression after upgrading
> to a more modern kernel ;-)
Why don't you use the Altera VIP FB on SoCFPGA ?
Best regards,
Marek Vasut
^ permalink raw reply
* Re: Questions about simplefb and hand-over
From: Johannes Pointner @ 2015-04-08 7:24 UTC (permalink / raw)
To: Hans de Goede
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, devicetree, David Herrmann
In-Reply-To: <551E5715.90600-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hello,
thank you for your reply.
I will take a look at the x86 code and at the function you mentioned.
Do you know where the simpledrm is hosted, because I would like to
take look at it too.
Thanks,
Hannes
2015-04-03 11:02 GMT+02:00 Hans de Goede <hdegoede@redhat.com>:
> Hi,
>
>
> On 03-04-15 10:57, Johannes Pointner wrote:
>>
>> Hello all,
>>
>> I'd like to use the simplefb in combination with tilcdc drm driver
>> with a TI AM335x processor. I got the u-boot to simplefb part working,
>> but I'm not sure how to do the hand-over part.
>>
>> Is it yet possible to do that? Does the tilcdc drm driver need
>> additional functionality to support that? Or are there other things I
>> have to take into account?
>>
>> Any suggestions would be highly appreciated!
>
>
> There is a remove_conflicting_framebuffers call which is already used
> on x86 to remove e.g. efifb when a kms driver loads, the plan for
> simplefb / arm is to do the same. That is where we stand now, we've
> a plan but you're the first person to actually try this.
>
> Note that you may want to use simpledrm by David Herrmann (added to the
> Cc) instead of simplefb as that will give you a kms driver right from
> the start, simpledrm is not yet upstream but should go upstream soon.
>
> Regards,
>
> Hans
>
>
^ permalink raw reply
* Re: Questions about simplefb and hand-over
From: Hans de Goede @ 2015-04-08 7:30 UTC (permalink / raw)
To: Johannes Pointner
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, devicetree, David Herrmann
In-Reply-To: <CAJKbBDD_DQS2DkwMa04ijdOCE4+g-bvw=TWcZDwYJgXfB2xH1Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi,
On 08-04-15 09:24, Johannes Pointner wrote:
> Hello,
>
> thank you for your reply.
> I will take a look at the x86 code and at the function you mentioned.
>
> Do you know where the simpledrm is hosted, because I would like to
> take look at it too.
David Herrmann (in the Cc) is the best person to answer that, I
think he needs to rebase on it on the latest upstream kernel.
David ?
Regards,
Hans
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox