Re: [PATCH] drm/gma500: use NULL instead of using plain integer as pointer

2021-03-19 Thread Patrik Jakobsson
On Wed, Mar 17, 2021 at 9:27 AM Yang Li  wrote:
>
> This fixes the following sparse warnings:
> drivers/gpu/drm/gma500/psb_drv.c:303:56: warning: Using plain integer as
> NULL pointer
>
> Reported-by: Abaci Robot 
> Signed-off-by: Yang Li 

Applied to drm-misc-next

Thanks
Patrik

> ---
>  drivers/gpu/drm/gma500/psb_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c 
> b/drivers/gpu/drm/gma500/psb_drv.c
> index 0bcab06..c2aab62 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -303,7 +303,7 @@ static int psb_driver_load(struct drm_device *dev, 
> unsigned long flags)
>
> ret = -ENOMEM;
>
> -   dev_priv->mmu = psb_mmu_driver_init(dev, 1, 0, 0);
> +   dev_priv->mmu = psb_mmu_driver_init(dev, 1, 0, NULL);
> if (!dev_priv->mmu)
> goto out_err;
>
> --
> 1.8.3.1
>


Re: linux-next: manual merge of the drivers-x86 tree with the drm-misc tree

2021-02-05 Thread Patrik Jakobsson
On Fri, Feb 5, 2021 at 12:07 PM Andy Shevchenko
 wrote:
>
> On Thu, Feb 4, 2021 at 11:04 AM Andy Shevchenko
>  wrote:
> >> Today's linux-next merge of the drivers-x86 tree got a conflict in:
> >
> > Thanks. I already asked Patrik yesterday day if DRM missed to pull an 
> > immutable tag I provided. I think they can pull and resolve conflicts 
> > themselves. Alternatively it would be easy to resolve by Linus by removing 
> > Kconfig lines along with mentioned files,
>
> Patrik, I have sent a PR again, so you may consider pulling it, thanks!

Daniel, is this something you can pull into drm or ask one of the
drm-misc maintainers to do?


Re: [PATCH 09/29] drm/gma500: Avoid comma separated statements

2021-01-30 Thread Patrik Jakobsson
On Sat, Jan 30, 2021 at 7:47 PM Joe Perches  wrote:
>
> On Mon, 2020-08-24 at 21:56 -0700, Joe Perches wrote:
> > Use semicolons and braces.
>
> Ping?

This entire file is going away so perhaps just drop the patch to avoid
a conflict.

-Patrik

>
> > Signed-off-by: Joe Perches 
> > ---
> >  drivers/gpu/drm/gma500/mdfld_intel_display.c | 44 +---
> >  1 file changed, 28 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/gma500/mdfld_intel_display.c 
> > b/drivers/gpu/drm/gma500/mdfld_intel_display.c
> > index aae2d358364c..bfa330df9443 100644
> > --- a/drivers/gpu/drm/gma500/mdfld_intel_display.c
> > +++ b/drivers/gpu/drm/gma500/mdfld_intel_display.c
> > @@ -824,33 +824,45 @@ static int mdfld_crtc_mode_set(struct drm_crtc *crtc,
> >   if ((ksel == KSEL_CRYSTAL_19) || (ksel == KSEL_BYPASS_19)) {
> >   refclk = 19200;
> >
> >
> > - if (is_mipi || is_mipi2)
> > - clk_n = 1, clk_p2 = 8;
> > - else if (is_hdmi)
> > - clk_n = 1, clk_p2 = 10;
> > + if (is_mipi || is_mipi2) {
> > + clk_n = 1;
> > + clk_p2 = 8;
> > + } else if (is_hdmi) {
> > + clk_n = 1;
> > + clk_p2 = 10;
> > + }
> >   } else if (ksel == KSEL_BYPASS_25) {
> >   refclk = 25000;
> >
> >
> > - if (is_mipi || is_mipi2)
> > - clk_n = 1, clk_p2 = 8;
> > - else if (is_hdmi)
> > - clk_n = 1, clk_p2 = 10;
> > + if (is_mipi || is_mipi2) {
> > + clk_n = 1;
> > + clk_p2 = 8;
> > + } else if (is_hdmi) {
> > + clk_n = 1;
> > + clk_p2 = 10;
> > + }
> >   } else if ((ksel == KSEL_BYPASS_83_100) &&
> >   dev_priv->core_freq == 166) {
> >   refclk = 83000;
> >
> >
> > - if (is_mipi || is_mipi2)
> > - clk_n = 4, clk_p2 = 8;
> > - else if (is_hdmi)
> > - clk_n = 4, clk_p2 = 10;
> > + if (is_mipi || is_mipi2) {
> > + clk_n = 4;
> > + clk_p2 = 8;
> > + } else if (is_hdmi) {
> > + clk_n = 4;
> > + clk_p2 = 10;
> > + }
> >   } else if ((ksel == KSEL_BYPASS_83_100) &&
> >   (dev_priv->core_freq == 100 ||
> >   dev_priv->core_freq == 200)) {
> >   refclk = 10;
> > - if (is_mipi || is_mipi2)
> > - clk_n = 4, clk_p2 = 8;
> > - else if (is_hdmi)
> > - clk_n = 4, clk_p2 = 10;
> > + if (is_mipi || is_mipi2) {
> > + clk_n = 4;
> > + clk_p2 = 8;
> > + } else if (is_hdmi) {
> > + clk_n = 4;
> > + clk_p2 = 10;
> > + }
> >   }
> >
> >
> >   if (is_mipi)
>
>


Re: [PATCH] usb: bdc: Remove the BDC PCI driver

2021-01-18 Thread Patrik Jakobsson
On Mon, Jan 18, 2021 at 1:22 PM Felipe Balbi  wrote:
>
>
> Hi,
>
> Greg Kroah-Hartman  writes:
> >> Al Cooper  writes:
> >> > The BDC PCI driver was only used for design verification with
> >> > an PCI/FPGA board. The board no longer exists and is not in use
> >> > anywhere. All instances of this core now exist as a memory mapped
> >> > device on the platform bus.
> >> >
> >> > NOTE: This only removes the PCI driver and does not remove the
> >> > platform driver.
> >> >
> >> > Signed-off-by: Al Cooper 
> >>
> >> It sounds like it could be used for pre-silicon verification of newer
> >> Core Releases, much like Synopsys still uses the HAPS (with mainline
> >> linux, mind you) for silicon validation.
> >>
> >> Why would we delete this small shim if it *could* still be useful?
> >
> > It ends up conflicting with the PCI id of a device that is actually in
> > the wild (a camera on Apple laptops).  So it's good to drop this driver
> > so the wrong driver doesn't get constantly bound to the wrong device.
>
> I see. Oh well...

It would also help if this got disabled in stable so existing kernels
stop loading bdc.

Can this patch go directly into stable or should I send a patch that
adds "depends on BROKEN"?

-Patrik


Re: [PATCH 4/4] drm/gma500: avoid Woverride-init warning

2020-10-28 Thread Patrik Jakobsson
On Tue, Oct 27, 2020 at 5:50 PM Arnd Bergmann  wrote:
>
> On Tue, Oct 27, 2020 at 10:54 AM Patrik Jakobsson
>  wrote:
> > On Tue, Oct 27, 2020 at 10:33 AM Daniel Vetter  wrote:
> > > On Mon, Oct 26, 2020 at 08:41:04PM +0100, Arnd Bergmann wrote:
> > > > From: Arnd Bergmann 
> > > >
> > > > gcc -Wextra notices that one of the fields in psbfb_roll_ops has two
> > > > initializers:
> > > >
> > > > drivers/gpu/drm/gma500/framebuffer.c:185:20: warning: initialized field 
> > > > overwritten [-Woverride-init]
> > > >
> > > > Open-code this instead, leaving out the extraneous initializers for
> > > > .fb_pan_display.
> > > >
> > > > Fixes: 3da6c2f3b730 ("drm/gma500: use DRM_FB_HELPER_DEFAULT_OPS for 
> > > > fb_ops")
> > > > Signed-off-by: Arnd Bergmann 
> > >
> > > Scrollback is dead, so I'm not sure it's even worth to keep all this. I'd
> > > just garbage-collect this, maybe als the entire accelerator code and just
> > > leave psbfb_unaccel_ops behind ...
> > > -Daniel
> >
> > That's been my idea for quite some time. The gtt roll code is also
> > broken in multi display setups.
> >
> > Arnd, I can take care of this unless you feel an urge to do it yourself.
>
> That would be good, thanks

Should be fixed with:
https://patchwork.freedesktop.org/patch/397482/?series=83153&rev=1

>
> I have no specific interest in the drm drivers, this is just part of a
> larger work to enable more of the W=1 options across the kernel
> by default, after all the existing warnings are addressed.
>
>Arnd


Re: [PATCH 4/4] drm/gma500: avoid Woverride-init warning

2020-10-27 Thread Patrik Jakobsson
On Tue, Oct 27, 2020 at 10:33 AM Daniel Vetter  wrote:
>
> On Mon, Oct 26, 2020 at 08:41:04PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann 
> >
> > gcc -Wextra notices that one of the fields in psbfb_roll_ops has two
> > initializers:
> >
> > drivers/gpu/drm/gma500/framebuffer.c:185:20: warning: initialized field 
> > overwritten [-Woverride-init]
> >
> > Open-code this instead, leaving out the extraneous initializers for
> > .fb_pan_display.
> >
> > Fixes: 3da6c2f3b730 ("drm/gma500: use DRM_FB_HELPER_DEFAULT_OPS for fb_ops")
> > Signed-off-by: Arnd Bergmann 
>
> Scrollback is dead, so I'm not sure it's even worth to keep all this. I'd
> just garbage-collect this, maybe als the entire accelerator code and just
> leave psbfb_unaccel_ops behind ...
> -Daniel

That's been my idea for quite some time. The gtt roll code is also
broken in multi display setups.

Arnd, I can take care of this unless you feel an urge to do it yourself.

-Patrik


Re: [PATCH] drm/gma500: fix error check

2020-08-19 Thread Patrik Jakobsson
On Wed, Aug 5, 2020 at 10:59 PM  wrote:
>
> From: Tom Rix 
>
> Reviewing this block of code in cdv_intel_dp_init()
>
> ret = cdv_intel_dp_aux_native_read(gma_encoder, DP_DPCD_REV, ...
>
> cdv_intel_edp_panel_vdd_off(gma_encoder);
> if (ret == 0) {
> /* if this fails, presume the device is a ghost */
> DRM_INFO("failed to retrieve link info, disabling eDP\n");
> drm_encoder_cleanup(encoder);
> cdv_intel_dp_destroy(connector);
> goto err_priv;
> } else {
>
> The (ret == 0) is not strict enough.
> cdv_intel_dp_aux_native_read() returns > 0 on success
> otherwise it is failure.
>
> So change to <=

Thanks for the patch. Looks correct.

Will apply to drm-misc-next

-Patrik

>
> Fixes: d112a8163f83 ("gma500/cdv: Add eDP support")
>
> Signed-off-by: Tom Rix 
> ---
>  drivers/gpu/drm/gma500/cdv_intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c 
> b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> index f41cbb753bb4..720a767118c9 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> @@ -2078,7 +2078,7 @@ cdv_intel_dp_init(struct drm_device *dev, struct 
> psb_intel_mode_device *mode_dev
>intel_dp->dpcd,
>sizeof(intel_dp->dpcd));
> cdv_intel_edp_panel_vdd_off(gma_encoder);
> -   if (ret == 0) {
> +   if (ret <= 0) {
> /* if this fails, presume the device is a ghost */
> DRM_INFO("failed to retrieve link info, disabling 
> eDP\n");
> drm_encoder_cleanup(encoder);
> --
> 2.18.1
>


Re: [PATCH] drm/gma500: Fix direction check in psb_accel_2d_copy()

2020-06-23 Thread Patrik Jakobsson
On Mon, Jun 22, 2020 at 10:45 PM Denis Efremov  wrote:
>
> psb_accel_2d_copy() checks direction PSB_2D_COPYORDER_BR2TL twice.
> Based on psb_accel_2d_copy_direction() results, PSB_2D_COPYORDER_TL2BR
> should be checked instead in the second direction check.
>
> Fixes: 4d8d096e9ae8 ("gma500: introduce the framebuffer support code")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Denis Efremov 
> ---
>  drivers/gpu/drm/gma500/accel_2d.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/accel_2d.c 
> b/drivers/gpu/drm/gma500/accel_2d.c
> index adc0507545bf..8dc86aac54d2 100644
> --- a/drivers/gpu/drm/gma500/accel_2d.c
> +++ b/drivers/gpu/drm/gma500/accel_2d.c
> @@ -179,8 +179,8 @@ static int psb_accel_2d_copy(struct drm_psb_private 
> *dev_priv,
> src_x += size_x - 1;
> dst_x += size_x - 1;
> }
> -   if (direction == PSB_2D_COPYORDER_BR2TL ||
> -   direction == PSB_2D_COPYORDER_BL2TR) {
> +   if (direction == PSB_2D_COPYORDER_BL2TR ||
> +   direction == PSB_2D_COPYORDER_TL2BR) {

Hi Denis,

Sorry, I don't follow. The code seems already correct to me.

src_x and dst_x gets adjusted when going from right to left
src_y and dst_y gets adjusted when going from bottom to top.

PSB_2D_COPYORDER_TL2BR needs no adjustment because it is the normal
blit direction.

Thanks
Patrik

> src_y += size_y - 1;
> dst_y += size_y - 1;
> }
> --
> 2.26.2
>


Re: [PATCH] [RESEND] drm/gma500: initialize gma_clock_t structures

2018-01-17 Thread Patrik Jakobsson
On Tue, Jan 16, 2018 at 3:57 PM, Arnd Bergmann  wrote:
> The two functions pass a partially initialized structure back to the
> caller after a memset() on the destination.
>
> This is not entirely well-defined, most compilers are sensible enough
> to either keep the zero-initialization for the uninitialized members,
> but gcc-4.4 does not, and it warns about this:
>
> drivers/gpu/drm/gma500/oaktrail_crtc.c: In function 'mrst_sdvo_find_best_pll':
> drivers/gpu/drm/gma500/oaktrail_crtc.c:175: warning: 'clock.vco' may be used 
> uninitialized in this function
> drivers/gpu/drm/gma500/oaktrail_crtc.c:175: warning: 'clock.dot' may be used 
> uninitialized in this function
> drivers/gpu/drm/gma500/oaktrail_crtc.c:175: warning: 'clock.p2' may be used 
> uninitialized in this function
> drivers/gpu/drm/gma500/oaktrail_crtc.c:175: warning: 'clock.m2' may be used 
> uninitialized in this function
> drivers/gpu/drm/gma500/oaktrail_crtc.c:175: warning: 'clock.m1' may be used 
> uninitialized in this function
> drivers/gpu/drm/gma500/oaktrail_crtc.c: In function 'mrst_lvds_find_best_pll':
> drivers/gpu/drm/gma500/oaktrail_crtc.c:208: warning: 'clock.p' may be used 
> uninitialized in this function
> drivers/gpu/drm/gma500/oaktrail_crtc.c:208: warning: 'clock.vco' may be used 
> uninitialized in this function
> drivers/gpu/drm/gma500/oaktrail_crtc.c:208: warning: 'clock.p2' may be used 
> uninitialized in this function
> drivers/gpu/drm/gma500/oaktrail_crtc.c:208: warning: 'clock.m2' may be used 
> uninitialized in this function
> drivers/gpu/drm/gma500/oaktrail_crtc.c:208: warning: 'clock.m1' may be used 
> uninitialized in this function
> drivers/gpu/drm/gma500/oaktrail_crtc.c:208: warning: 'clock.n' may be used 
> uninitialized in this function
>
> This adds an initialization at declaration time to avoid the warning
> and make it well-defined on all compiler versions.
>
> Signed-off-by: Arnd Bergmann 

Dave or Daniel, I'm on sick leave and will be for some time to come.
Can one of you pick up this patch? Thanks.

Reviewed-by: Patrik Jakobsson 

> ---
> Originally submitted on Sep 15, 2017, but got no reply. Resending unchanged.
> ---
>  drivers/gpu/drm/gma500/oaktrail_crtc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/oaktrail_crtc.c 
> b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> index 0fff269d3fe6..b49fe79c3f44 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_crtc.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_crtc.c
> @@ -134,7 +134,7 @@ static bool mrst_sdvo_find_best_pll(const struct 
> gma_limit_t *limit,
> struct drm_crtc *crtc, int target,
> int refclk, struct gma_clock_t 
> *best_clock)
>  {
> -   struct gma_clock_t clock;
> +   struct gma_clock_t clock = {};
> u32 target_vco, actual_freq;
> s32 freq_error, min_error = 10;
>
> @@ -191,7 +191,7 @@ static bool mrst_lvds_find_best_pll(const struct 
> gma_limit_t *limit,
> struct drm_crtc *crtc, int target,
> int refclk, struct gma_clock_t 
> *best_clock)
>  {
> -   struct gma_clock_t clock;
> +   struct gma_clock_t clock = {};
> int err = target;
>
> memset(best_clock, 0, sizeof(*best_clock));
> --
> 2.9.0
>


Re: [PATCH 0/1] gpu: move include files out of include/linux/i2c

2017-06-05 Thread Patrik Jakobsson
On Mon, May 22, 2017 at 12:11 AM, Wolfram Sang  wrote:
> It doesn't make sense to use include/linux/i2c for client drivers which may in
> fact rather be hwmon or input or whatever devices. As a result, I want to
> deprecate include/linux/i2c for good. This series moves the include files to a
> better location, largely include/linux/platform_data because that is what most
> of the moved include files contain. Please let me know if you think another
> location is more suitable.
>
> I prefer the series to go upstream via the subsystem tree; if you prefer that 
> I
> take it via I2C, just let me know.

I don't think it makes sense to take this through the drm/gma500 since
it's platform data and does not really touch any gma500 specifics.
What makes sense for I2C/platform is probably more important here.

>
> No runtime testing because of no HW, but buildbot is happy with this series at
> least. A branch can be found here:

Not sure anybody have hardware to test mdfld + tc358765. Perhaps some
smartphone exists but I've never seen one.

FWIW, for the small gma500 change you have my:

Acked-by: Patrik Jakobsson 

Cheers

>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/platform_data
>
> Thanks and kind regards,
>
>Wolfram
>
>
> Wolfram Sang (1):
>   gpu: drm: tc35876x: move header file out of I2C realm
>
>  arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c | 2 +-
>  drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c  | 2 +-
>  include/linux/{i2c => platform_data}/tc35876x.h | 0
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename include/linux/{i2c => platform_data}/tc35876x.h (100%)
>
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] gma500: Fix a sleep-in-atomic bug in psbfb_2d_submit

2017-05-31 Thread Patrik Jakobsson
On Wed, May 31, 2017 at 10:48 AM, Jia-Ju Bai  wrote:
> The driver may sleep under a spin lock, and the function call path is:
> psbfb_2d_submit (acquire the lock by spin_lock_irqsave)
>   psb_2d_wait_available
> psb_spank
>   msleep --> may sleep
>
> To fix it, the "msleep" is replaced with "mdelay" in psb_spank.
>
> Signed-off-by: Jia-Ju Bai 

Thanks for the patch. checkpatch complains about DOS line endings so
you might want to take a look at your editor settings. Usually we tag
the subject with drm/gma500 but don't think there's a hard rule about
that. I'll fix it up and take it through drm-misc-next.

Signed-off-by: Patrik Jakobsson 

> ---
>  drivers/gpu/drm/gma500/accel_2d.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/accel_2d.c 
> b/drivers/gpu/drm/gma500/accel_2d.c
> index c51d925..7c24c8a 100644
> --- a/drivers/gpu/drm/gma500/accel_2d.c
> +++ b/drivers/gpu/drm/gma500/accel_2d.c
> @@ -55,7 +55,7 @@ void psb_spank(struct drm_psb_private *dev_priv)
> _PSB_CS_RESET_TWOD_RESET, PSB_CR_SOFT_RESET);
> PSB_RSGX32(PSB_CR_SOFT_RESET);
>
> -   msleep(1);
> +   mdelay(1);
>
> PSB_WSGX32(0, PSB_CR_SOFT_RESET);
> wmb();
> @@ -64,7 +64,7 @@ void psb_spank(struct drm_psb_private *dev_priv)
> wmb();
> (void) PSB_RSGX32(PSB_CR_BIF_CTRL);
>
> -   msleep(1);
> +   mdelay(1);
> PSB_WSGX32(PSB_RSGX32(PSB_CR_BIF_CTRL) & ~_PSB_CB_CTRL_CLEAR_FAULT,
>PSB_CR_BIF_CTRL);
> (void) PSB_RSGX32(PSB_CR_BIF_CTRL);
> --
> 1.7.9.5
>
>


Re: [PATCH] gpu: drm: gma500: remove two more dead variable

2017-05-22 Thread Patrik Jakobsson
On Mon, May 22, 2017 at 10:30 PM, Arnd Bergmann  wrote:
> The dead code removal left two unused variables:
>
> drivers/gpu/drm/gma500/mdfld_tpo_vid.c: In function 'tpo_vid_get_config_mode':
> drivers/gpu/drm/gma500/mdfld_tpo_vid.c:34:31: error: unused variable 'ti' 
> [-Werror=unused-variable]
>
> This removes them as well.
>
> Fixes: 94d7fb4982d2 ("gpu: drm: gma500: remove dead code")
> Signed-off-by: Arnd Bergmann 

Thanks, I'll take this through drm-misc

Signed-off-by: Patrik Jakobsson 

> ---
>  drivers/gpu/drm/gma500/mdfld_tpo_vid.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/mdfld_tpo_vid.c 
> b/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> index d40628e6810d..a9420bf9a419 100644
> --- a/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> +++ b/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> @@ -30,8 +30,6 @@
>  static struct drm_display_mode *tpo_vid_get_config_mode(struct drm_device 
> *dev)
>  {
> struct drm_display_mode *mode;
> -   struct drm_psb_private *dev_priv = dev->dev_private;
> -   struct oaktrail_timing_info *ti = &dev_priv->gct_data.DTD;
>
> mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> if (!mode)
> --
> 2.9.0
>


Re: [PATCH v2] gpu: drm: gma500: remove dead code

2017-05-22 Thread Patrik Jakobsson
On Fri, May 19, 2017 at 2:28 PM, Patrik Jakobsson
 wrote:
> On Fri, May 19, 2017 at 11:19 AM, Gustavo A. R. Silva
>  wrote:
>> Local variable use_gct is assigned to a constant value and it is never
>> updated again. Remove this variable and the dead code it guards.
>>
>> Addresses-Coverity-ID: 145690
>> Signed-off-by: Gustavo A. R. Silva 
>
> I believe the first version is already in drm-misc. Actually this
> entire file can be removed. It was never hooked up and since nobody
> every complained I feel confident we can remove it.

Sorry my bad, it is actually in use since it's hardcoded. I'll pick up
Arnd's fix (unless he takes it through some other tree).

Thanks
Patrik

> Cheers
> Patrik
>
>> ---
>> Changes in v2:
>>  Remove variables ti and dev_priv, which was causing a compilation warning.
>>
>>  I have improved my testing to avoid similar issues in the future.
>>  This is how I tested it this time:
>>
>>  $ make allmodconfig
>>  $ make drivers/gpu/drm/gma500/mdfld_tpo_vid.o
>>
>>
>>  drivers/gpu/drm/gma500/mdfld_tpo_vid.c | 53 
>> ++
>>  1 file changed, 9 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/gma500/mdfld_tpo_vid.c 
>> b/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
>> index d8d4170..a9420bf 100644
>> --- a/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
>> +++ b/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
>> @@ -30,55 +30,20 @@
>>  static struct drm_display_mode *tpo_vid_get_config_mode(struct drm_device 
>> *dev)
>>  {
>> struct drm_display_mode *mode;
>> -   struct drm_psb_private *dev_priv = dev->dev_private;
>> -   struct oaktrail_timing_info *ti = &dev_priv->gct_data.DTD;
>> -   bool use_gct = false;
>>
>> mode = kzalloc(sizeof(*mode), GFP_KERNEL);
>> if (!mode)
>> return NULL;
>>
>> -   if (use_gct) {
>> -   mode->hdisplay = (ti->hactive_hi << 8) | ti->hactive_lo;
>> -   mode->vdisplay = (ti->vactive_hi << 8) | ti->vactive_lo;
>> -   mode->hsync_start = mode->hdisplay +
>> -   ((ti->hsync_offset_hi << 8) |
>> -   ti->hsync_offset_lo);
>> -   mode->hsync_end = mode->hsync_start +
>> -   ((ti->hsync_pulse_width_hi << 8) |
>> -   ti->hsync_pulse_width_lo);
>> -   mode->htotal = mode->hdisplay + ((ti->hblank_hi << 8) |
>> -   
>> ti->hblank_lo);
>> -   mode->vsync_start =
>> -   mode->vdisplay + ((ti->vsync_offset_hi << 8) |
>> -   ti->vsync_offset_lo);
>> -   mode->vsync_end =
>> -   mode->vsync_start + ((ti->vsync_pulse_width_hi << 8) 
>> |
>> -   ti->vsync_pulse_width_lo);
>> -   mode->vtotal = mode->vdisplay +
>> -   ((ti->vblank_hi << 8) | ti->vblank_lo);
>> -   mode->clock = ti->pixel_clock * 10;
>> -
>> -   dev_dbg(dev->dev, "hdisplay is %d\n", mode->hdisplay);
>> -   dev_dbg(dev->dev, "vdisplay is %d\n", mode->vdisplay);
>> -   dev_dbg(dev->dev, "HSS is %d\n", mode->hsync_start);
>> -   dev_dbg(dev->dev, "HSE is %d\n", mode->hsync_end);
>> -   dev_dbg(dev->dev, "htotal is %d\n", mode->htotal);
>> -   dev_dbg(dev->dev, "VSS is %d\n", mode->vsync_start);
>> -   dev_dbg(dev->dev, "VSE is %d\n", mode->vsync_end);
>> -   dev_dbg(dev->dev, "vtotal is %d\n", mode->vtotal);
>> -   dev_dbg(dev->dev, "clock is %d\n", mode->clock);
>> -   } else {
>> -   mode->hdisplay = 864;
>> -   mode->vdisplay = 480;
>> -   mode->hsync_start = 873;
>> -   mode->hsync_end = 876;
>> -   mode->htotal = 887;
>> -   mode->vsync_start = 487;
>> -   mode->vsync_end = 490;
>> -   mode->vtotal = 499;
>> -   mode->clock = 33264;
>> -   }
>> +   mode->hdisplay = 864;
>> +   mode->vdisplay = 480;
>> +   mode->hsync_start = 873;
>> +   mode->hsync_end = 876;
>> +   mode->htotal = 887;
>> +   mode->vsync_start = 487;
>> +   mode->vsync_end = 490;
>> +   mode->vtotal = 499;
>> +   mode->clock = 33264;
>>
>> drm_mode_set_name(mode);
>> drm_mode_set_crtcinfo(mode, 0);
>> --
>> 2.5.0
>>


Re: [PATCH v2] gpu: drm: gma500: remove dead code

2017-05-19 Thread Patrik Jakobsson
On Fri, May 19, 2017 at 11:19 AM, Gustavo A. R. Silva
 wrote:
> Local variable use_gct is assigned to a constant value and it is never
> updated again. Remove this variable and the dead code it guards.
>
> Addresses-Coverity-ID: 145690
> Signed-off-by: Gustavo A. R. Silva 

I believe the first version is already in drm-misc. Actually this
entire file can be removed. It was never hooked up and since nobody
every complained I feel confident we can remove it.

Cheers
Patrik

> ---
> Changes in v2:
>  Remove variables ti and dev_priv, which was causing a compilation warning.
>
>  I have improved my testing to avoid similar issues in the future.
>  This is how I tested it this time:
>
>  $ make allmodconfig
>  $ make drivers/gpu/drm/gma500/mdfld_tpo_vid.o
>
>
>  drivers/gpu/drm/gma500/mdfld_tpo_vid.c | 53 
> ++
>  1 file changed, 9 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/mdfld_tpo_vid.c 
> b/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> index d8d4170..a9420bf 100644
> --- a/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> +++ b/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> @@ -30,55 +30,20 @@
>  static struct drm_display_mode *tpo_vid_get_config_mode(struct drm_device 
> *dev)
>  {
> struct drm_display_mode *mode;
> -   struct drm_psb_private *dev_priv = dev->dev_private;
> -   struct oaktrail_timing_info *ti = &dev_priv->gct_data.DTD;
> -   bool use_gct = false;
>
> mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> if (!mode)
> return NULL;
>
> -   if (use_gct) {
> -   mode->hdisplay = (ti->hactive_hi << 8) | ti->hactive_lo;
> -   mode->vdisplay = (ti->vactive_hi << 8) | ti->vactive_lo;
> -   mode->hsync_start = mode->hdisplay +
> -   ((ti->hsync_offset_hi << 8) |
> -   ti->hsync_offset_lo);
> -   mode->hsync_end = mode->hsync_start +
> -   ((ti->hsync_pulse_width_hi << 8) |
> -   ti->hsync_pulse_width_lo);
> -   mode->htotal = mode->hdisplay + ((ti->hblank_hi << 8) |
> -   
> ti->hblank_lo);
> -   mode->vsync_start =
> -   mode->vdisplay + ((ti->vsync_offset_hi << 8) |
> -   ti->vsync_offset_lo);
> -   mode->vsync_end =
> -   mode->vsync_start + ((ti->vsync_pulse_width_hi << 8) |
> -   ti->vsync_pulse_width_lo);
> -   mode->vtotal = mode->vdisplay +
> -   ((ti->vblank_hi << 8) | ti->vblank_lo);
> -   mode->clock = ti->pixel_clock * 10;
> -
> -   dev_dbg(dev->dev, "hdisplay is %d\n", mode->hdisplay);
> -   dev_dbg(dev->dev, "vdisplay is %d\n", mode->vdisplay);
> -   dev_dbg(dev->dev, "HSS is %d\n", mode->hsync_start);
> -   dev_dbg(dev->dev, "HSE is %d\n", mode->hsync_end);
> -   dev_dbg(dev->dev, "htotal is %d\n", mode->htotal);
> -   dev_dbg(dev->dev, "VSS is %d\n", mode->vsync_start);
> -   dev_dbg(dev->dev, "VSE is %d\n", mode->vsync_end);
> -   dev_dbg(dev->dev, "vtotal is %d\n", mode->vtotal);
> -   dev_dbg(dev->dev, "clock is %d\n", mode->clock);
> -   } else {
> -   mode->hdisplay = 864;
> -   mode->vdisplay = 480;
> -   mode->hsync_start = 873;
> -   mode->hsync_end = 876;
> -   mode->htotal = 887;
> -   mode->vsync_start = 487;
> -   mode->vsync_end = 490;
> -   mode->vtotal = 499;
> -   mode->clock = 33264;
> -   }
> +   mode->hdisplay = 864;
> +   mode->vdisplay = 480;
> +   mode->hsync_start = 873;
> +   mode->hsync_end = 876;
> +   mode->htotal = 887;
> +   mode->vsync_start = 487;
> +   mode->vsync_end = 490;
> +   mode->vtotal = 499;
> +   mode->clock = 33264;
>
> drm_mode_set_name(mode);
> drm_mode_set_crtcinfo(mode, 0);
> --
> 2.5.0
>


Re: [PATCH 3/3] gpu: drm: drivers: Convert printk(KERN_ to pr_

2017-02-28 Thread Patrik Jakobsson
On Tue, Feb 28, 2017 at 1:55 PM, Joe Perches  wrote:
> Use a more common logging style.
>
> Miscellanea:
>
> o Coalesce formats and realign arguments
> o Neaten a few macros now using pr_
>
> Signed-off-by: Joe Perches 

For the gma500 changes:

Acked-by: Patrik Jakobsson 

> ---
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c   |  9 -
>  drivers/gpu/drm/gma500/oaktrail_lvds.c| 18 +-
>  drivers/gpu/drm/gma500/psb_drv.h  |  5 ++---
>  drivers/gpu/drm/gma500/psb_intel_lvds.c   |  7 +++
>  drivers/gpu/drm/i915/i915_sw_fence.c  |  8 
>  drivers/gpu/drm/mgag200/mgag200_mode.c|  2 +-
>  drivers/gpu/drm/msm/msm_drv.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_acpi.c|  7 ---
>  drivers/gpu/drm/nouveau/nouveau_vga.c |  4 ++--
>  drivers/gpu/drm/nouveau/nv50_display.c| 22 +++---
>  drivers/gpu/drm/nouveau/nvkm/core/mm.c| 10 +-
>  drivers/gpu/drm/omapdrm/dss/dsi.c | 17 -
>  drivers/gpu/drm/omapdrm/dss/dss.c |  3 +--
>  drivers/gpu/drm/omapdrm/dss/dss.h | 15 ++-
>  drivers/gpu/drm/omapdrm/omap_gem.c|  5 ++---
>  drivers/gpu/drm/r128/r128_cce.c   |  7 +++
>  drivers/gpu/drm/ttm/ttm_bo.c  |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c |  6 ++
>  drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c |  3 +--
>  drivers/gpu/drm/vmwgfx/vmwgfx_resource.c  |  4 ++--
>  20 files changed, 72 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c 
> b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> index 5efdb7fbb7ee..e64960db3224 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c
> @@ -284,8 +284,7 @@ static bool cdv_intel_lvds_mode_fixup(struct drm_encoder 
> *encoder,
> head) {
> if (tmp_encoder != encoder
> && tmp_encoder->crtc == encoder->crtc) {
> -   printk(KERN_ERR "Can't enable LVDS and another "
> -  "encoder on the same pipe\n");
> +   pr_err("Can't enable LVDS and another encoder on the 
> same pipe\n");
> return false;
> }
> }
> @@ -756,13 +755,13 @@ void cdv_intel_lvds_init(struct drm_device *dev,
>
>  failed_find:
> mutex_unlock(&dev->mode_config.mutex);
> -   printk(KERN_ERR "Failed find\n");
> +   pr_err("Failed find\n");
> psb_intel_i2c_destroy(gma_encoder->ddc_bus);
>  failed_ddc:
> -   printk(KERN_ERR "Failed DDC\n");
> +   pr_err("Failed DDC\n");
> psb_intel_i2c_destroy(gma_encoder->i2c_bus);
>  failed_blc_i2c:
> -   printk(KERN_ERR "Failed BLC\n");
> +   pr_err("Failed BLC\n");
> drm_encoder_cleanup(encoder);
> drm_connector_cleanup(connector);
> kfree(lvds_priv);
> diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c 
> b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> index f7038f12ac76..e6943fef0611 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c
> @@ -255,15 +255,15 @@ static void oaktrail_lvds_get_configuration_mode(struct 
> drm_device *dev,
> ((ti->vblank_hi << 8) | ti->vblank_lo);
> mode->clock = ti->pixel_clock * 10;
>  #if 0
> -   printk(KERN_INFO "hdisplay is %d\n", mode->hdisplay);
> -   printk(KERN_INFO "vdisplay is %d\n", mode->vdisplay);
> -   printk(KERN_INFO "HSS is %d\n", mode->hsync_start);
> -   printk(KERN_INFO "HSE is %d\n", mode->hsync_end);
> -   printk(KERN_INFO "htotal is %d\n", mode->htotal);
> -   printk(KERN_INFO "VSS is %d\n", mode->vsync_start);
> -   printk(KERN_INFO "VSE is %d\n", mode->vsync_end);
> -   printk(KERN_INFO "vtotal is %d\n", mode->vtotal);
> -   printk(KERN_INFO "clock is %d\n", mode->clock);
> +   pr_info("hdisplay is %d\n", mode->hdisplay);
> +   pr_info("vdisplay is %d\n", mode->vdisplay);
> +   pr_info("HSS is %d\n", mode->hsync_start);
> +   pr_info("HSE is %d\n", mode->hsync_end);
> +   pr_info("htotal is %d\n", mode->

Re: [PATCH 1/1] gpu: drm: gma500: Use vma_pages()

2016-10-11 Thread Patrik Jakobsson
On Tue, Oct 11, 2016 at 10:15 AM, Daniel Vetter  wrote:
> On Mon, Oct 10, 2016 at 07:22:47AM +0530, Shyam Saini wrote:
>> On Mon, 2016-10-10 at 01:46 +0200, Patrik Jakobsson wrote:
>> > On Mon, Oct 10, 2016 at 1:07 AM, Shyam Saini 
>> > wrote:
>> > >
>> > > Replace explicit computation of vma page count by a call to
>> > > vma_pages()
>> > Hi, I already have this patch in the "queue" from:
>> > Muhammad Falak R Wani 
>> >
>> > Will include that one when I get around to sending out a PR.
>
> Since the merge window is pretty much done I've applied this one here to
> drm-misc to make sure it wont get lost.
> -Daniel
>

Thanks, I'll take them out of my queue that never got sent.

-Patrik

>> >
>> > Thanks
>> > Patrik
>> >
>> > >
>> > >
>> > > Signed-off-by: Shyam Saini 
>> > > ---
>> > >  drivers/gpu/drm/gma500/framebuffer.c | 2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/gma500/framebuffer.c
>> > > b/drivers/gpu/drm/gma500/framebuffer.c
>> > > index 3a44e70..0fde850 100644
>> > > --- a/drivers/gpu/drm/gma500/framebuffer.c
>> > > +++ b/drivers/gpu/drm/gma500/framebuffer.c
>> > > @@ -124,7 +124,7 @@ static int psbfb_vm_fault(struct vm_area_struct
>> > > *vma, struct vm_fault *vmf)
>> > > unsigned long phys_addr = (unsigned long)dev_priv-
>> > > >stolen_base +
>> > >   psbfb->gtt->offset;
>> > >
>> > > -   page_num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>> > > +   page_num = vma_pages(vma);
>> > > address = (unsigned long)vmf->virtual_address - (vmf->pgoff
>> > > << PAGE_SHIFT);
>> > >
>> > > vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
>> > > --
>> > > 2.7.4
>> > >
>>
>> No issue.
>>
>> Thanks
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch


Re: [PATCH 1/1] gpu: drm: gma500: Use vma_pages()

2016-10-09 Thread Patrik Jakobsson
On Mon, Oct 10, 2016 at 1:07 AM, Shyam Saini  wrote:
> Replace explicit computation of vma page count by a call to
> vma_pages()

Hi, I already have this patch in the "queue" from:
Muhammad Falak R Wani 

Will include that one when I get around to sending out a PR.

Thanks
Patrik

>
> Signed-off-by: Shyam Saini 
> ---
>  drivers/gpu/drm/gma500/framebuffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
> b/drivers/gpu/drm/gma500/framebuffer.c
> index 3a44e70..0fde850 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -124,7 +124,7 @@ static int psbfb_vm_fault(struct vm_area_struct *vma, 
> struct vm_fault *vmf)
> unsigned long phys_addr = (unsigned long)dev_priv->stolen_base +
>   psbfb->gtt->offset;
>
> -   page_num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> +   page_num = vma_pages(vma);
> address = (unsigned long)vmf->virtual_address - (vmf->pgoff << 
> PAGE_SHIFT);
>
> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> --
> 2.7.4
>


Re: [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data()

2016-09-20 Thread Patrik Jakobsson
On Tue, Sep 20, 2016 at 2:08 PM, Jani Nikula
 wrote:
> On Tue, 20 Sep 2016, Dan Carpenter  wrote:
>> Don't be a dummy...  This is easy to review an it fixes a bug.

In this particular case it might not be clear that an unknown GCT
version causes a complete GCT failure so both messages are useful.

>>
>> I'm fine with you NAKing all these patches based on who they are from.
>> I mostly just delete these without responding because the guy has
>> history of introducing bugs and never listens to feedback.  But asking
>> pointless rhetorical questions is not helpful.
>>
>> A lot of people are CC'd and you're wasting everyone's time by asking
>> questions where you know the answer.
>
> Fair enough, sorry for the noise.
>
> To be honest, I did only look at the patches, not who they were from. We
> have CI for drm/i915, but I don't think it's constructive to keep
> changing drivers like this where the upstream isn't actively developed
> and tested. But I digress. It's up to Patrik anyway.

Nothing in this series is very helpful so NAK. In general I'm not fond
of trivial changes like this since it's hard to say what motivates the
author. In theory it shouldn't matter but so far it's been directly
related to the quality of the patches. I can help test changes for
gma500 if needed but please make it worth my while.

Best regards
Patrik

>
> BR,
> Jani.
>
>
>
> --
> Jani Nikula, Intel Open Source Technology Center


Re: [PATCH 07/10] drm/gma500: use drm_crtc_vblank_{on,off}()

2016-06-07 Thread Patrik Jakobsson
On Tue, Jun 7, 2016 at 4:07 PM, Gustavo Padovan  wrote:
> From: Gustavo Padovan 
>
> Replace the legacy drm_vblank_{on,off}() with the new helper functions.
>
> Signed-off-by: Gustavo Padovan 

Acked-by: Patrik Jakobsson 

> ---
>  drivers/gpu/drm/gma500/gma_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/gma_display.c 
> b/drivers/gpu/drm/gma500/gma_display.c
> index c95406e..e3d9e35 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -281,7 +281,7 @@ void gma_crtc_dpms(struct drm_crtc *crtc, int mode)
> REG_WRITE(VGACNTRL, VGA_DISP_DISABLE);
>
> /* Turn off vblank interrupts */
> -   drm_vblank_off(dev, pipe);
> +   drm_crtc_vblank_off(crtc);
>
> /* Wait for vblank for the disable to take effect */
> gma_wait_for_vblank(dev);
> --
> 2.5.5
>


Re: [PATCH] drm/gma500: use vma_pages().

2016-05-21 Thread Patrik Jakobsson
On Sat, May 21, 2016 at 3:21 PM, Muhammad Falak R Wani
 wrote:
> Replace explicit computation of vma page count by a call to
> vma_pages()
>
> Signed-off-by: Muhammad Falak R Wani 

Thanks, queued for gma500-next

> ---
>  drivers/gpu/drm/gma500/framebuffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
> b/drivers/gpu/drm/gma500/framebuffer.c
> index 7440bf9..5486e7e 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -125,7 +125,7 @@ static int psbfb_vm_fault(struct vm_area_struct *vma, 
> struct vm_fault *vmf)
> unsigned long phys_addr = (unsigned long)dev_priv->stolen_base +
>   psbfb->gtt->offset;
>
> -   page_num = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> +   page_num = vma_pages(vma);
> address = (unsigned long)vmf->virtual_address - (vmf->pgoff << 
> PAGE_SHIFT);
>
> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> --
> 1.9.1
>


Re: [PATCH 1/1] drm/gma500: mdfld: avoid possible null pointer dereference

2016-05-18 Thread Patrik Jakobsson
On Wed, May 18, 2016 at 10:31 PM, Heinrich Schuchardt
 wrote:
> Only dereference sender after checking if sender is NULL.

Hi Heinrich

I think we had a patch that did something similar a while ago. Don't
remember what happened to it. We do check for !sender right before
calling this function (at only one call-site) so it cannot be NULL at
this point. I would be ok with your change if you also remove the
extra checks in mdfld_dsi_read_mcs(). That way we don't double check
and we check at the point where it actually matters.

Thanks
Patrik

>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c 
> b/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
> index 1616af2..c50534c 100644
> --- a/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
> +++ b/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
> @@ -520,7 +520,7 @@ static int __read_panel_data(struct mdfld_dsi_pkg_sender 
> *sender, u8 data_type,
> u8 *data, u16 len, u32 *data_out, u16 len_out, bool 
> hs)
>  {
> unsigned long flags;
> -   struct drm_device *dev = sender->dev;
> +   struct drm_device *dev;
> int i;
> u32 gen_data_reg;
> int retry = MDFLD_DSI_READ_MAX_COUNT;
> @@ -530,6 +530,8 @@ static int __read_panel_data(struct mdfld_dsi_pkg_sender 
> *sender, u8 data_type,
> return -EINVAL;
> }
>
> +   dev = sender->dev;
> +
> /**
>  * do reading.
>  * 0) send out generic read request
> --
> 2.1.4
>


Re: [PATCH v3] drm/gma500: fix double freeing

2016-04-07 Thread Patrik Jakobsson
On Thu, Apr 7, 2016 at 5:52 PM, Sudip Mukherjee
 wrote:
> On Wednesday 09 December 2015 05:50 PM, Patrik Jakobsson wrote:
>>
>> On Wed, Dec 9, 2015 at 12:53 PM, Sudip Mukherjee
>>  wrote:
>>>
>>> On Thu, Oct 08, 2015 at 06:17:48PM +0530, Sudip Mukherjee wrote:
>>>>
>>>> We are allocating backing using psbfb_alloc() and so
>>>> backing->stolen is always true. So we were freeing backing two times.
>>>> Moreover if we follow the execution path then we should be freeing
>>>> backing after we have released the helper. So remove the one which frees
>>>> backing before the helper is released.
>>>> While at it the error labels are also renamed to give a meaningful
>>>> name.
>>>>
>>>> Signed-off-by: Sudip Mukherjee 
>>>> Reviewed-by: Patrik Jakobsson 
>>>> ---
>>>
>>>
>>> This patch was never picked up. It will not apply now.
>>>
>>> Daniel, please let me know if you want me to resend after making
>>> necessary changes.
>>
>>
>> I will pick this up and pass it along to Dave. Sorry for the delay.
>
>
> This was not picked up. But I guess it is still true. Do you want me to
> rebase and send it again..

I already have it rebased, will send it out later tonight. For real this time ;)

Thanks
Patrik

>
> regards
> sudip
>
>
>>
>> -Patrik
>>
>>>
>>> regards
>>> sudip
>>>
>>>>   drivers/gpu/drm/gma500/framebuffer.c | 13 -
>>>>   1 file changed, 4 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/gma500/framebuffer.c
>>>> b/drivers/gpu/drm/gma500/framebuffer.c
>>>> index 2eaf1b3..52e2bf3 100644
>>>> --- a/drivers/gpu/drm/gma500/framebuffer.c
>>>> +++ b/drivers/gpu/drm/gma500/framebuffer.c
>>>> @@ -411,7 +411,7 @@ static int psbfb_create(struct psb_fbdev *fbdev,
>>>>info = drm_fb_helper_alloc_fbi(&fbdev->psb_fb_helper);
>>>>if (IS_ERR(info)) {
>>>>ret = PTR_ERR(info);
>>>> - goto out_err1;
>>>> + goto err_unlock;
>>>>}
>>>>info->par = fbdev;
>>>>
>>>> @@ -419,7 +419,7 @@ static int psbfb_create(struct psb_fbdev *fbdev,
>>>>
>>>>ret = psb_framebuffer_init(dev, psbfb, &mode_cmd, backing);
>>>>if (ret)
>>>> - goto out_unref;
>>>> + goto err_release;
>>>>
>>>>fb = &psbfb->base;
>>>>psbfb->fbdev = info;
>>>> @@ -465,14 +465,9 @@ static int psbfb_create(struct psb_fbdev *fbdev,
>>>>
>>>>mutex_unlock(&dev->struct_mutex);
>>>>return 0;
>>>> -out_unref:
>>>> - if (backing->stolen)
>>>> - psb_gtt_free_range(dev, backing);
>>>> - else
>>>> - drm_gem_object_unreference(&backing->gem);
>>>> -
>>>> +err_release:
>>>>drm_fb_helper_release_fbi(&fbdev->psb_fb_helper);
>>>> -out_err1:
>>>> +err_unlock:
>>>>mutex_unlock(&dev->struct_mutex);
>>>>psb_gtt_free_range(dev, backing);
>>>>return ret;
>>>> --
>>>> 1.9.1
>>>>
>


Re: [PATCH] gma500: fix missing comma in dsi_errors array initializer

2016-04-02 Thread Patrik Jakobsson
On Wed, Mar 23, 2016 at 2:28 AM, One Thousand Gnomes
 wrote:
> On Tue, 22 Mar 2016 16:40:18 -0700
> Joe Perches  wrote:
>
>> On Tue, 2016-03-22 at 22:49 +, Colin King wrote:
>> > From: Colin Ian King 
>> >
>> > There is a missing comma between two strings in the dsi_errors[]
>> > array initializer, causing two strings to be concatenated and the
>> > array being incorrectly initialized.  Add in the missing comma.
>
> I sent that a while ago, and there's also a patch to remove bogus code
> from mdfld_dsi_dpi.c outstanding somewhere

I have a version of the missing comma fix that I'll send out with
-fixes soon. I should have done that a couple of months ago. Which
patch is the mdffld_dsi_dpi cleanup? I can't find anything related to
that.

-Patrik

>
>
> Alan
>


Re: [PATCH] drm/gma500: Make mdfld_dsi_connector_dpms() return a value

2016-02-25 Thread Patrik Jakobsson
Hi Daniel,

A patch to fix this is already merged into drm-misc:

commit db9b60400f9253c25ae639797df2d0ff7a35d9d8
Author: Sudip Mukherjee 
Date:   Tue Feb 2 11:35:55 2016 +0530

drm/gma500: remove helper function

We were getting build warning about:
drivers/gpu/drm/gma500/mdfld_dsi_output.c:407:2: warning: initialization
from incompatible pointer type

The callback to dpms was pointing to a helper function which had a
return type of void, whereas the callback should point to a function
which has a return type of int.
On closer look it turned out that we do not need the helper function
since if we call drm_helper_connector_dpms() directly, the first check
that drm_helper_connector_dpms() does is: if (mode == connector->dpms)

Signed-off-by: Sudip Mukherjee 
Link: 
http://patchwork.freedesktop.org/patch/msgid/1454393155-13142-1-git-send-email-sudipm.mukher...@gmail.com
Acked-by: Patrik Jakobsson 
Signed-off-by: Daniel Vetter 

Thanks
Patrik


On Thu, Feb 18, 2016 at 3:58 PM, Daniel Wagner
 wrote:
> 9a69a9ac20f7 ("drm: Make the connector dpms callback
> return a value, v2.") wants mdfld_dsi_connector_dpms to return
> a value.
>
> This fixes also a new gcc 5.2 warning:
>
> drivers/gpu/drm/gma500/mdfld_dsi_output.c:407:39: warning: initialization 
> from incompatible pointer type [-Werror=incompatible-pointer-types]
>   .dpms = /*drm_helper_connector_dpms*/mdfld_dsi_connector_dpms,
>^
> Signed-off-by: Daniel Wagner 
> ---
>  drivers/gpu/drm/gma500/mdfld_dsi_output.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c 
> b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> index d758f4c..407b9bb 100644
> --- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> +++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> @@ -382,14 +382,14 @@ static int mdfld_dsi_connector_mode_valid(struct 
> drm_connector *connector,
> return MODE_OK;
>  }
>
> -static void mdfld_dsi_connector_dpms(struct drm_connector *connector, int 
> mode)
> +static int mdfld_dsi_connector_dpms(struct drm_connector *connector, int 
> mode)
>  {
> if (mode == connector->dpms)
> -   return;
> +   return 0;
>
> /*first, execute dpms*/
>
> -   drm_helper_connector_dpms(connector, mode);
> +   return drm_helper_connector_dpms(connector, mode);
>  }
>
>  static struct drm_encoder *mdfld_dsi_connector_best_encoder(
> @@ -404,7 +404,7 @@ static struct drm_encoder 
> *mdfld_dsi_connector_best_encoder(
>
>  /*DSI connector funcs*/
>  static const struct drm_connector_funcs mdfld_dsi_connector_funcs = {
> -   .dpms = /*drm_helper_connector_dpms*/mdfld_dsi_connector_dpms,
> +   .dpms = mdfld_dsi_connector_dpms,
> .detect = mdfld_dsi_connector_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .set_property = mdfld_dsi_connector_set_property,
> --
> 2.5.0


Re: [PATCH] drm/gma500: remove helper function

2016-02-06 Thread Patrik Jakobsson
On Tue, Feb 2, 2016 at 7:05 AM, Sudip Mukherjee
 wrote:
> We were getting build warning about:
> drivers/gpu/drm/gma500/mdfld_dsi_output.c:407:2: warning: initialization
> from incompatible pointer type
>
> The callback to dpms was pointing to a helper function which had a
> return type of void, whereas the callback should point to a function
> which has a return type of int.
> On closer look it turned out that we do not need the helper function
> since if we call drm_helper_connector_dpms() directly, the first check
> that drm_helper_connector_dpms() does is: if (mode == connector->dpms)
>
> Signed-off-by: Sudip Mukherjee 

Looks good, thanks.

Signed-off-by: Patrik Jakobsson 

> ---
>  drivers/gpu/drm/gma500/mdfld_dsi_output.c | 12 +---
>  1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c 
> b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> index d758f4c..907cb51 100644
> --- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> +++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> @@ -382,16 +382,6 @@ static int mdfld_dsi_connector_mode_valid(struct 
> drm_connector *connector,
> return MODE_OK;
>  }
>
> -static void mdfld_dsi_connector_dpms(struct drm_connector *connector, int 
> mode)
> -{
> -   if (mode == connector->dpms)
> -   return;
> -
> -   /*first, execute dpms*/
> -
> -   drm_helper_connector_dpms(connector, mode);
> -}
> -
>  static struct drm_encoder *mdfld_dsi_connector_best_encoder(
> struct drm_connector *connector)
>  {
> @@ -404,7 +394,7 @@ static struct drm_encoder 
> *mdfld_dsi_connector_best_encoder(
>
>  /*DSI connector funcs*/
>  static const struct drm_connector_funcs mdfld_dsi_connector_funcs = {
> -   .dpms = /*drm_helper_connector_dpms*/mdfld_dsi_connector_dpms,
> +   .dpms = drm_helper_connector_dpms,
> .detect = mdfld_dsi_connector_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .set_property = mdfld_dsi_connector_set_property,
> --
> 1.9.1
>


Re: [PATCH] gma500: clean up an excessive and confusing helper

2016-02-01 Thread Patrik Jakobsson
On Fri, Jan 29, 2016 at 8:37 PM, Alan  wrote:
> From: Alan Cox 
>
> This is a left over from the great clean ups in the past. It's confusing as
> it returns an int, yet has one caller that never uses it. The caller already
> has all the right private variables local so the entire function can be
> replaced by a simple if call.
>
> Signed-off-by: Alan Cox 

Signed-off-by: Patrik Jakobsson 

> ---
>  drivers/gpu/drm/gma500/framebuffer.c |   20 
>  1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
> b/drivers/gpu/drm/gma500/framebuffer.c
> index cb95765..033d894 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -674,29 +674,17 @@ static const struct drm_mode_config_funcs 
> psb_mode_funcs = {
> .output_poll_changed = psbfb_output_poll_changed,
>  };
>
> -static int psb_create_backlight_property(struct drm_device *dev)
> -{
> -   struct drm_psb_private *dev_priv = dev->dev_private;
> -   struct drm_property *backlight;
> -
> -   if (dev_priv->backlight_property)
> -   return 0;
> -
> -   backlight = drm_property_create_range(dev, 0, "backlight", 0, 100);
> -
> -   dev_priv->backlight_property = backlight;
> -
> -   return 0;
> -}
> -
>  static void psb_setup_outputs(struct drm_device *dev)
>  {
> struct drm_psb_private *dev_priv = dev->dev_private;
> struct drm_connector *connector;
>
> drm_mode_create_scaling_mode_property(dev);
> -   psb_create_backlight_property(dev);
>
> +   /* It is ok for this to fail - we just don't get backlight control */
> +   if (!dev_priv->backlight_property)
> +   dev_priv->backlight_property = drm_property_create_range(dev, 
> 0,
> +   "backlight", 0, 100);
> dev_priv->ops->output_init(dev);
>
> list_for_each_entry(connector, &dev->mode_config.connector_list,
>


Re: [PATCH v3] drm/gma500: fix double freeing

2015-12-09 Thread Patrik Jakobsson
On Wed, Dec 9, 2015 at 12:53 PM, Sudip Mukherjee
 wrote:
> On Thu, Oct 08, 2015 at 06:17:48PM +0530, Sudip Mukherjee wrote:
>> We are allocating backing using psbfb_alloc() and so
>> backing->stolen is always true. So we were freeing backing two times.
>> Moreover if we follow the execution path then we should be freeing
>> backing after we have released the helper. So remove the one which frees
>> backing before the helper is released.
>> While at it the error labels are also renamed to give a meaningful
>> name.
>>
>> Signed-off-by: Sudip Mukherjee 
>> Reviewed-by: Patrik Jakobsson 
>> ---
>
> This patch was never picked up. It will not apply now.
>
> Daniel, please let me know if you want me to resend after making
> necessary changes.

I will pick this up and pass it along to Dave. Sorry for the delay.

-Patrik

>
> regards
> sudip
>
>>  drivers/gpu/drm/gma500/framebuffer.c | 13 -
>>  1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
>> b/drivers/gpu/drm/gma500/framebuffer.c
>> index 2eaf1b3..52e2bf3 100644
>> --- a/drivers/gpu/drm/gma500/framebuffer.c
>> +++ b/drivers/gpu/drm/gma500/framebuffer.c
>> @@ -411,7 +411,7 @@ static int psbfb_create(struct psb_fbdev *fbdev,
>>   info = drm_fb_helper_alloc_fbi(&fbdev->psb_fb_helper);
>>   if (IS_ERR(info)) {
>>   ret = PTR_ERR(info);
>> - goto out_err1;
>> + goto err_unlock;
>>   }
>>   info->par = fbdev;
>>
>> @@ -419,7 +419,7 @@ static int psbfb_create(struct psb_fbdev *fbdev,
>>
>>   ret = psb_framebuffer_init(dev, psbfb, &mode_cmd, backing);
>>   if (ret)
>> - goto out_unref;
>> + goto err_release;
>>
>>   fb = &psbfb->base;
>>   psbfb->fbdev = info;
>> @@ -465,14 +465,9 @@ static int psbfb_create(struct psb_fbdev *fbdev,
>>
>>   mutex_unlock(&dev->struct_mutex);
>>   return 0;
>> -out_unref:
>> - if (backing->stolen)
>> - psb_gtt_free_range(dev, backing);
>> - else
>> - drm_gem_object_unreference(&backing->gem);
>> -
>> +err_release:
>>   drm_fb_helper_release_fbi(&fbdev->psb_fb_helper);
>> -out_err1:
>> +err_unlock:
>>   mutex_unlock(&dev->struct_mutex);
>>   psb_gtt_free_range(dev, backing);
>>   return ret;
>> --
>> 1.9.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] MAINTAINERS: Add myself as maintainer for the gma500 driver

2015-10-12 Thread Patrik Jakobsson
Signed-off-by: Patrik Jakobsson 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0e1abe8..c7decf7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3368,6 +3368,14 @@ S:   Maintained
 F: drivers/gpu/drm/imx/
 F: Documentation/devicetree/bindings/drm/imx/
 
+DRM DRIVERS FOR GMA500 (Poulsbo, Moorestown and derivative chipsets)
+M: Patrik Jakobsson 
+L: dri-de...@lists.freedesktop.org
+T: git git://github.com/patjak/drm-gma500
+S: Maintained
+F: drivers/gpu/drm/gma500
+F: include/drm/gma500*
+
 DRM DRIVERS FOR NVIDIA TEGRA
 M: Thierry Reding 
 M: Terje Bergström 
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] drm/gma500: fix double freeing

2015-10-07 Thread Patrik Jakobsson
On Tue, Oct 6, 2015 at 5:48 PM, Sudip Mukherjee
 wrote:
> We are allocating backing using psbfb_alloc() and so
> backing->stolen is always true. So we were freeing backing two times.
> Moreover if we follow the execution path then we should be freeing
> backing after we have released the helper. So remove the one which frees
> backing before the helper is released.
> While at it the error labels are also renamed to give a meaningful
> name.
>
> Cc: Patrik Jakobsson 
> Signed-off-by: Sudip Mukherjee 
> ---
>
> Hi Patrik,
> If you donot like the labels I will change them according to what you
> have suggested.

Hi

Label names are used in all sorts of funny ways in the kernel. However
CodingStyle is quite clear on the matter: "Also don't name them after
the goto location like "err_kmalloc_failed:" and if you think about
it, it makes sense. Let's say we add another goto to that label then
the name wouldn't be correct anymore.

err_release and err_unlock would make me happier :)

With that fixed
Reviewed-by: Patrik Jakobsson 

Thanks

>  drivers/gpu/drm/gma500/framebuffer.c | 13 -
>  1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
> b/drivers/gpu/drm/gma500/framebuffer.c
> index 2eaf1b3..52e2bf3 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -411,7 +411,7 @@ static int psbfb_create(struct psb_fbdev *fbdev,
> info = drm_fb_helper_alloc_fbi(&fbdev->psb_fb_helper);
> if (IS_ERR(info)) {
> ret = PTR_ERR(info);
> -   goto out_err1;
> +   goto err_alloc_fbi;
> }
> info->par = fbdev;
>
> @@ -419,7 +419,7 @@ static int psbfb_create(struct psb_fbdev *fbdev,
>
> ret = psb_framebuffer_init(dev, psbfb, &mode_cmd, backing);
> if (ret)
> -   goto out_unref;
> +   goto err_framebuffer_init;
>
> fb = &psbfb->base;
> psbfb->fbdev = info;
> @@ -465,14 +465,9 @@ static int psbfb_create(struct psb_fbdev *fbdev,
>
> mutex_unlock(&dev->struct_mutex);
> return 0;
> -out_unref:
> -   if (backing->stolen)
> -   psb_gtt_free_range(dev, backing);
> -   else
> -   drm_gem_object_unreference(&backing->gem);
> -
> +err_framebuffer_init:
> drm_fb_helper_release_fbi(&fbdev->psb_fb_helper);
> -out_err1:
> +err_alloc_fbi:
> mutex_unlock(&dev->struct_mutex);
> psb_gtt_free_range(dev, backing);
> return ret;
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] drm/gma500: fix double freeing

2015-10-05 Thread Patrik Jakobsson
On Fri, Oct 2, 2015 at 5:56 PM, Sudip Mukherjee
 wrote:
> On Thu, Oct 01, 2015 at 07:07:33PM +0200, Patrik Jakobsson wrote:
>> On Wed, Sep 30, 2015 at 8:12 AM, Sudip Mukherjee
>>  wrote:
>> > On Tue, Sep 29, 2015 at 03:20:35PM +0200, Patrik Jakobsson wrote:
>> >> On Thu, Sep 24, 2015 at 5:57 PM, Sudip Mukherjee
>> >>  wrote:
>> >> > On Wed, Sep 09, 2015 at 06:20:40PM +0530, Sudip Mukherjee wrote:
>> >> >> If backing->stolen is true then we were freeing backing by calling
>> >> >> psb_gtt_free_range() but we called it again after unlocking the mutex.
>> >> >> Lets make it NULL after freeing in psb_gtt_free_range() and check for
>> >> >> NULL before calling the function for the second time.
>> >> >>
>> >> >> Signed-off-by: Sudip Mukherjee 
>> >> >> ---
>> >> > Hi Patrik,
>> >> > A gentle ping.
>> >> >
>> >> > regards
>> >> > sudip
>> >>
>> >> Hi, sorry for the late reply.
>> >>
>> >> Why are we freeing the range twice in the first case?
>> > I think,
>> > if backing->stolen is true then backing is released using
>> > psb_gtt_free_range() but if backing->stolen is false then the gem object
>> > is freed but the backing is not yet freed. To free that backing
>> > psb_gtt_free_range() has been called second time. My patch tried to fix
>> > the possibility of backing->stolen being true and backing being freed 2
>> > times.
>> >
>> > regards
>> > sudip
>>
>> There are some special handling of the stolen framebuffer that I don't
>> remember entirely but the basic concept is that we free the backing
>> when we drop the last reference on a gem object. That will trigger a
>> psb_gtt_free_range(). So in this case it looks to me that the extra
>> free is not needed at all. That's my quick reasoning, feel free to
>> prove me wrong :)
>
> In this case we are allocating backing using psbfb_alloc() and so
> backing->stolen is always true. So we can remove the backing->stolen
> condition. And if drm_fb_helper_alloc_fbi() fails then we
> are jumping to out_err1. So the fitst free will not be needed.

Sounds good, could you also rename the labels to what they're doing
now. I'm thinking out_release and out_unlock or something you feel is
suitable.

Thanks
Patrik

> diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
> b/drivers/gpu/drm/gma500/framebuffer.c
> index 2eaf1b3..932f07b 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -466,11 +466,6 @@ static int psbfb_create(struct psb_fbdev *fbdev,
> mutex_unlock(&dev->struct_mutex);
> return 0;
>  out_unref:
> -   if (backing->stolen)
> -   psb_gtt_free_range(dev, backing);
> -   else
> -   drm_gem_object_unreference(&backing->gem);
> -
> drm_fb_helper_release_fbi(&fbdev->psb_fb_helper);
>  out_err1:
> mutex_unlock(&dev->struct_mutex);
>
>
> If it is ok, I can submit the v2.
>
> regards
> sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] drm/gma500: fix double freeing

2015-10-01 Thread Patrik Jakobsson
On Wed, Sep 30, 2015 at 8:12 AM, Sudip Mukherjee
 wrote:
> On Tue, Sep 29, 2015 at 03:20:35PM +0200, Patrik Jakobsson wrote:
>> On Thu, Sep 24, 2015 at 5:57 PM, Sudip Mukherjee
>>  wrote:
>> > On Wed, Sep 09, 2015 at 06:20:40PM +0530, Sudip Mukherjee wrote:
>> >> If backing->stolen is true then we were freeing backing by calling
>> >> psb_gtt_free_range() but we called it again after unlocking the mutex.
>> >> Lets make it NULL after freeing in psb_gtt_free_range() and check for
>> >> NULL before calling the function for the second time.
>> >>
>> >> Signed-off-by: Sudip Mukherjee 
>> >> ---
>> > Hi Patrik,
>> > A gentle ping.
>> >
>> > regards
>> > sudip
>>
>> Hi, sorry for the late reply.
>>
>> Why are we freeing the range twice in the first case?
> I think,
> if backing->stolen is true then backing is released using
> psb_gtt_free_range() but if backing->stolen is false then the gem object
> is freed but the backing is not yet freed. To free that backing
> psb_gtt_free_range() has been called second time. My patch tried to fix
> the possibility of backing->stolen being true and backing being freed 2
> times.
>
> regards
> sudip

There are some special handling of the stolen framebuffer that I don't
remember entirely but the basic concept is that we free the backing
when we drop the last reference on a gem object. That will trigger a
psb_gtt_free_range(). So in this case it looks to me that the extra
free is not needed at all. That's my quick reasoning, feel free to
prove me wrong :)

Thanks
Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] drm/gma500: fix double freeing

2015-09-29 Thread Patrik Jakobsson
On Thu, Sep 24, 2015 at 5:57 PM, Sudip Mukherjee
 wrote:
> On Wed, Sep 09, 2015 at 06:20:40PM +0530, Sudip Mukherjee wrote:
>> If backing->stolen is true then we were freeing backing by calling
>> psb_gtt_free_range() but we called it again after unlocking the mutex.
>> Lets make it NULL after freeing in psb_gtt_free_range() and check for
>> NULL before calling the function for the second time.
>>
>> Signed-off-by: Sudip Mukherjee 
>> ---
> Hi Patrik,
> A gentle ping.
>
> regards
> sudip

Hi, sorry for the late reply.

Why are we freeing the range twice in the first case?

-Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] gma500:Remove functions that are now deprecated and move to the newer functions in drm_dp_helper.c

2015-05-12 Thread Patrik Jakobsson
On Mon, May 11, 2015 at 5:54 PM, Nicholas Krause  wrote:
>
>
> On May 11, 2015 4:24:01 AM EDT, Patrik Jakobsson 
>  wrote:
>>On Sun, May 10, 2015 at 9:50 PM, Nicholas Krause 
>>wrote:
>>>
>>>
>>> On May 10, 2015 3:45:36 PM EDT, patrik.r.jakobs...@gmail.com wrote:
>>>>On Sun, May 10, 2015 at 02:40:40PM -0400, nick wrote:
>>>>>
>>>>>
>>>>> On 2015-05-10 02:35 PM, patrik.r.jakobs...@gmail.com wrote:
>>>>> > On Sun, May 10, 2015 at 01:48:14PM -0400, nick wrote:
>>>>> >>
>>>>> >>
>>>>> >> On 2015-05-10 01:04 PM, patrik.r.jakobs...@gmail.com wrote:
>>>>> >>> On Tue, May 05, 2015 at 11:17:07AM -0400, nick wrote:
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> On 2015-05-05 09:06 AM, Patrik Jakobsson wrote:
>>>>> >>>>> On Tue, May 5, 2015 at 12:29 AM, Nicholas Krause
>>>> wrote:
>>>>> >>>>>> This removes the deprecated functions,i2c_dp_aux_add_bus and
>>>>> >>>>>> i2c_dp_aux_prepare_bus and the only call in the function,
>>>>> >>>>>> cdv_intel_dp_i2c_init to i2c_dp_aux_add_bus respectfully.
>>>>> >>>>>> The call and use of these functions is now replaced
>>alongside
>>>>> >>>>>> the removal of setting other values in the
>>>>function,cdv_intel_dp_i2c_init
>>>>> >>>>>> to use the helper function, drm_dp_aux_register that can
>>>>handle all this
>>>>> >>>>>> work internally.
>>>>> >>>>>
>>>>> >>>>> You need to fill in the drm_dp_aux structure and implement a
>>>>proper transfer
>>>>> >>>>> function. This patch would only break things. But the cdv dp
>>>>output is already
>>>>> >>>>> broken on my system so it needs fixing first.
>>>>> >>>>>
>>>>> >>>>> -Patrik
>>>>> >>>>>
>>>>> >>>> Patrik,
>>>>> >>>> Due to me not being an expert in this area of the kernel I
>>based
>>>>my work off the similar function,
>>>>> >>>> intel_dp_aux_init for i915. I was unsure of which helper
>>>>functions for i915 need to be written
>>>>> >>>> differently for gma500 based solutions as I don't known the
>>>>differences between these two in the
>>>>> >>>> graphics specs area.
>>>>> >>>> Thanks,
>>>>> >>>> Nick
>>>>> >>>
>>>>> >>> Hi Nick
>>>>> >>> You are _required_ to know what your patches are doing before
>>>>sending them. If
>>>>> >>> you don't test your code and don't know what it's doing then we
>>>>will not accept
>>>>> >>> it. You're expected to read the available documentation and
>>>>relevant literature
>>>>> >>> before sending patches and asking questions. Otherwise someone
>>>>else is doing the
>>>>> >>> work for you, which I assume is not the purpose of you patch
>>>>submission.
>>>>> >>>
>>>>> >>> Thanks
>>>>> >>> Patrik
>>>>>
>>>>> >> Patrik,
>>>>> >> Sorry about that my question is actually this ,I am unsure of
>>what
>>>>parts of the function,intel_dp_aux_init
>>>>> >> need to be ported for the gma500 gpu driver. Furthermore I will
>>>>list what parts for certain I known need to
>>>>> >> be ported over above the lines of code in the function
>>definition
>>>>I have pasted below. Please let me known
>>>>> >> if there are any areas I am missing. This is more just of a
>>double
>>>>check to make sure I don't miss something
>>>>> >> critical:)(I like to verify something before I code it).
>>>>> >> Sorry for the misunderstanding,
>>>>> >
>>>>> > You can figure this out by reading the drm dp helpers docs and
>>>>looking at
>>>>> > cdv_intel_dp.c. For instance, in gma500 we initialize DP_B and
>>>>DP_C.
>>>>> >
>>>>> > My point is that _you_ need to read the code and understand it. I
>>>>cannot help
>>>>> > you with that.
>>>>> >
>>>>> > But as I said in my first reply. DP on CDV is broken (at least on
>>>>my machine) so
>>>>> > that should be fixed first.
>>>>> >
>>>>> > Cheers
>>>>> > Patrik
>>>>> >
>>>>> Patrik,
>>>>> Sorry about that :). Your right it's a bad habit :(, I will start
>>>>working on it.
>>>>> Further more by "broken" you mean what. This is very vague and if
>>you
>>>>would like
>>>>> some help with it, I am willing to if you can give me a detailed
>>>>response of what
>>>>> is wrong.
>>>>> Nick
>>>>
>>>>Vague? Both DP and LVDS displays are black after boot when DP is
>>>>connected. What
>>>>additional information do you need?
>>>>
>>>  That sounds broken to me. This seems like a good place to start
>>helping a new with your permission.
>>> Nick
>>
>>Sorry but I'm no teacher and cannot help you with that.
>>
>>Patrik
>>
> That's not what I meant in terms of teaching me.  I asked if you want help 
> solving this issue with your machine.
> Nick

Sure. What info do you need?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] gma500:Remove functions that are now deprecated and move to the newer functions in drm_dp_helper.c

2015-05-11 Thread Patrik Jakobsson
On Sun, May 10, 2015 at 9:50 PM, Nicholas Krause  wrote:
>
>
> On May 10, 2015 3:45:36 PM EDT, patrik.r.jakobs...@gmail.com wrote:
>>On Sun, May 10, 2015 at 02:40:40PM -0400, nick wrote:
>>>
>>>
>>> On 2015-05-10 02:35 PM, patrik.r.jakobs...@gmail.com wrote:
>>> > On Sun, May 10, 2015 at 01:48:14PM -0400, nick wrote:
>>> >>
>>> >>
>>> >> On 2015-05-10 01:04 PM, patrik.r.jakobs...@gmail.com wrote:
>>> >>> On Tue, May 05, 2015 at 11:17:07AM -0400, nick wrote:
>>> >>>>
>>> >>>>
>>> >>>> On 2015-05-05 09:06 AM, Patrik Jakobsson wrote:
>>> >>>>> On Tue, May 5, 2015 at 12:29 AM, Nicholas Krause
>> wrote:
>>> >>>>>> This removes the deprecated functions,i2c_dp_aux_add_bus and
>>> >>>>>> i2c_dp_aux_prepare_bus and the only call in the function,
>>> >>>>>> cdv_intel_dp_i2c_init to i2c_dp_aux_add_bus respectfully.
>>> >>>>>> The call and use of these functions is now replaced alongside
>>> >>>>>> the removal of setting other values in the
>>function,cdv_intel_dp_i2c_init
>>> >>>>>> to use the helper function, drm_dp_aux_register that can
>>handle all this
>>> >>>>>> work internally.
>>> >>>>>
>>> >>>>> You need to fill in the drm_dp_aux structure and implement a
>>proper transfer
>>> >>>>> function. This patch would only break things. But the cdv dp
>>output is already
>>> >>>>> broken on my system so it needs fixing first.
>>> >>>>>
>>> >>>>> -Patrik
>>> >>>>>
>>> >>>> Patrik,
>>> >>>> Due to me not being an expert in this area of the kernel I based
>>my work off the similar function,
>>> >>>> intel_dp_aux_init for i915. I was unsure of which helper
>>functions for i915 need to be written
>>> >>>> differently for gma500 based solutions as I don't known the
>>differences between these two in the
>>> >>>> graphics specs area.
>>> >>>> Thanks,
>>> >>>> Nick
>>> >>>
>>> >>> Hi Nick
>>> >>> You are _required_ to know what your patches are doing before
>>sending them. If
>>> >>> you don't test your code and don't know what it's doing then we
>>will not accept
>>> >>> it. You're expected to read the available documentation and
>>relevant literature
>>> >>> before sending patches and asking questions. Otherwise someone
>>else is doing the
>>> >>> work for you, which I assume is not the purpose of you patch
>>submission.
>>> >>>
>>> >>> Thanks
>>> >>> Patrik
>>>
>>> >> Patrik,
>>> >> Sorry about that my question is actually this ,I am unsure of what
>>parts of the function,intel_dp_aux_init
>>> >> need to be ported for the gma500 gpu driver. Furthermore I will
>>list what parts for certain I known need to
>>> >> be ported over above the lines of code in the function definition
>>I have pasted below. Please let me known
>>> >> if there are any areas I am missing. This is more just of a double
>>check to make sure I don't miss something
>>> >> critical:)(I like to verify something before I code it).
>>> >> Sorry for the misunderstanding,
>>> >
>>> > You can figure this out by reading the drm dp helpers docs and
>>looking at
>>> > cdv_intel_dp.c. For instance, in gma500 we initialize DP_B and
>>DP_C.
>>> >
>>> > My point is that _you_ need to read the code and understand it. I
>>cannot help
>>> > you with that.
>>> >
>>> > But as I said in my first reply. DP on CDV is broken (at least on
>>my machine) so
>>> > that should be fixed first.
>>> >
>>> > Cheers
>>> > Patrik
>>> >
>>> Patrik,
>>> Sorry about that :). Your right it's a bad habit :(, I will start
>>working on it.
>>> Further more by "broken" you mean what. This is very vague and if you
>>would like
>>> some help with it, I am willing to if you can give me a detailed
>>response of what
>>

Re: [PATCH] gma500:Remove functions that are now deprecated and move to the newer functions in drm_dp_helper.c

2015-05-05 Thread Patrik Jakobsson
On Tue, May 5, 2015 at 12:29 AM, Nicholas Krause  wrote:
> This removes the deprecated functions,i2c_dp_aux_add_bus and
> i2c_dp_aux_prepare_bus and the only call in the function,
> cdv_intel_dp_i2c_init to i2c_dp_aux_add_bus respectfully.
> The call and use of these functions is now replaced alongside
> the removal of setting other values in the function,cdv_intel_dp_i2c_init
> to use the helper function, drm_dp_aux_register that can handle all this
> work internally.

You need to fill in the drm_dp_aux structure and implement a proper transfer
function. This patch would only break things. But the cdv dp output is already
broken on my system so it needs fixing first.

-Patrik

> Signed-off-by: Nicholas Krause 
> ---
>  drivers/gpu/drm/gma500/cdv_intel_dp.c | 42 
> ++-
>  1 file changed, 2 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c 
> b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> index 0fafb8e..c8c20fc 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> @@ -200,38 +200,6 @@ static const struct i2c_algorithm i2c_dp_aux_algo = {
> .functionality  = i2c_algo_dp_aux_functionality,
>  };
>
> -static void
> -i2c_dp_aux_reset_bus(struct i2c_adapter *adapter)
> -{
> -   (void) i2c_algo_dp_aux_address(adapter, 0, false);
> -   (void) i2c_algo_dp_aux_stop(adapter, false);
> -}
> -
> -static int
> -i2c_dp_aux_prepare_bus(struct i2c_adapter *adapter)
> -{
> -   adapter->algo = &i2c_dp_aux_algo;
> -   adapter->retries = 3;
> -   i2c_dp_aux_reset_bus(adapter);
> -   return 0;
> -}
> -
> -/*
> - * FIXME: This is the old dp aux helper, gma500 is the last driver that 
> needs to
> - * be ported over to the new helper code in drm_dp_helper.c like i915 or 
> radeon.
> - */
> -static int __deprecated
> -i2c_dp_aux_add_bus(struct i2c_adapter *adapter)
> -{
> -   int error;
> -
> -   error = i2c_dp_aux_prepare_bus(adapter);
> -   if (error)
> -   return error;
> -   error = i2c_add_adapter(adapter);
> -   return error;
> -}
> -
>  #define _wait_for(COND, MS, W) ({ \
>  unsigned long timeout__ = jiffies + msecs_to_jiffies(MS);   \
>  int ret__ = 0;  \
> @@ -275,6 +243,7 @@ struct cdv_intel_dp {
> int backlight_on_delay;
> int backlight_off_delay;
> struct drm_display_mode *panel_fixed_mode;  /* for eDP */
> +   struct drm_dp_aux aux;
> bool panel_on;
>  };
>
> @@ -855,18 +824,11 @@ cdv_intel_dp_i2c_init(struct gma_connector *connector,
> intel_dp->algo.running = false;
> intel_dp->algo.address = 0;
> intel_dp->algo.aux_ch = cdv_intel_dp_i2c_aux_ch;
> -
> memset(&intel_dp->adapter, '\0', sizeof (intel_dp->adapter));
> -   intel_dp->adapter.owner = THIS_MODULE;
> -   intel_dp->adapter.class = I2C_CLASS_DDC;
> -   strncpy (intel_dp->adapter.name, name, sizeof(intel_dp->adapter.name) 
> - 1);
> -   intel_dp->adapter.name[sizeof(intel_dp->adapter.name) - 1] = '\0';
> -   intel_dp->adapter.algo_data = &intel_dp->algo;
> -   intel_dp->adapter.dev.parent = connector->base.kdev;
>
> if (is_edp(encoder))
> cdv_intel_edp_panel_vdd_on(encoder);
> -   ret = i2c_dp_aux_add_bus(&intel_dp->adapter);
> +   ret = drm_dp_aux_register(&intel_dp->aux);
> if (is_edp(encoder))
> cdv_intel_edp_panel_vdd_off(encoder);
>
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] gpu: drm: gma500: mmu.c: Remove unused function

2015-01-01 Thread Patrik Jakobsson
On Thu, Jan 1, 2015 at 5:55 PM, Rickard Strandqvist
 wrote:
> Remove the function psb_get_default_pd_addr() that is not used anywhere.
>
> This was partially found by using a static code analysis program called 
> cppcheck.
>
> Signed-off-by: Rickard Strandqvist 

Hej Rickard

I'd like to keep that function since it's relevant to getting the MSVDX running.
I'm not working on it but someone else might be.

> ---
>  drivers/gpu/drm/gma500/mmu.c |9 -
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/mmu.c b/drivers/gpu/drm/gma500/mmu.c
> index 0eaf11c..557cae7 100644
> --- a/drivers/gpu/drm/gma500/mmu.c
> +++ b/drivers/gpu/drm/gma500/mmu.c
> @@ -426,15 +426,6 @@ struct psb_mmu_pd *psb_mmu_get_default_pd(struct 
> psb_mmu_driver *driver)
> return pd;
>  }
>
> -/* Returns the physical address of the PD shared by sgx/msvdx */
> -uint32_t psb_get_default_pd_addr(struct psb_mmu_driver *driver)
> -{
> -   struct psb_mmu_pd *pd;
> -
> -   pd = psb_mmu_get_default_pd(driver);
> -   return page_to_pfn(pd->p) << PAGE_SHIFT;
> -}
> -
>  void psb_mmu_driver_takedown(struct psb_mmu_driver *driver)
>  {
> struct drm_device *dev = driver->dev;
> --
> 1.7.10.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/1 linux-next] drm/gma500: replace 0 by NULL for pointer

2014-12-27 Thread Patrik Jakobsson
On Sat, Dec 27, 2014 at 4:37 PM, Fabian Frederick  wrote:
> Fix sparse warning:
> drivers/gpu/drm/gma500/psb_drv.c:
> 328:56: warning: Using plain integer as NULL pointer
>
> Signed-off-by: Fabian Frederick 

Reviewed-by: Patrik Jakobsson 

> ---
>  drivers/gpu/drm/gma500/psb_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c 
> b/drivers/gpu/drm/gma500/psb_drv.c
> index 92e7e57..9b49c155 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -325,7 +325,7 @@ static int psb_driver_load(struct drm_device *dev, 
> unsigned long flags)
> if (ret)
> goto out_err;
>
> -   dev_priv->mmu = psb_mmu_driver_init(dev, 1, 0, 0);
> +   dev_priv->mmu = psb_mmu_driver_init(dev, 1, 0, NULL);
> if (!dev_priv->mmu)
> goto out_err;
>
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] drivers:gpu:drm Remove unneeded struct of type psb_intel_i2c_chan in the header file psb_drv.h

2014-11-30 Thread Patrik Jakobsson
On Sun, Nov 30, 2014 at 3:24 AM, Nicholas Krause  wrote:
> Removes unneeeded struct *lvds_i2c_bus of type, psb_intel_i2c_chan as this 
> struct
> is no needed due to never being used in the header file, psb_drv.h and 
> therefore
> should be removed from this header file.
>
> Signed-off-by: Nicholas Krause 

The lvds_i2c_bus is in use and can't be removed. Please at least compile test
your patches before submitting them.

Cheers
Patrik

> ---
>  drivers/gpu/drm/gma500/psb_drv.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/psb_drv.h 
> b/drivers/gpu/drm/gma500/psb_drv.h
> index 55ebe2b..fc74518 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.h
> +++ b/drivers/gpu/drm/gma500/psb_drv.h
> @@ -522,7 +522,6 @@ struct drm_psb_private {
> struct drm_display_mode *sdvo_lvds_vbt_mode;
>
> struct bdb_lvds_backlight *lvds_bl; /* LVDS backlight info from VBT */
> -   struct psb_intel_i2c_chan *lvds_i2c_bus; /* FIXME: Remove this? */
>
> /* Feature bits from the VBIOS */
> unsigned int int_tv_support:1;
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Remove Code in psb_drb.h

2014-11-22 Thread Patrik Jakobsson
On Sat, Nov 22, 2014 at 2:45 PM, nick  wrote:
> Greetings again David and other maintainers,
> I am wondering if I can remove the following code below my message in 
> psb_drv.h as you state it's unneeded in a TODO.
> Cheers Nick
> /* TODO: To get rid of */
> #define PSB_TT_PRIV0_LIMIT   (256*1024*1024)
> #define PSB_TT_PRIV0_PLIMIT  (PSB_TT_PRIV0_LIMIT >> PAGE_SHIFT)

Hi Nick

Yes, those can safely be removed

Cheers
Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] mba6x_bl: Backlight driver for mid 2013 MacBook Air

2014-05-05 Thread Patrik Jakobsson
On Mon, May 5, 2014 at 8:28 PM, Matthew Garrett
 wrote:
> On Mon, 2014-05-05 at 16:49 +0200, Patrik Jakobsson wrote:
>> Hi Matthew,
>>
>> This seems to be in good shape. Do you have anything to add?
>> If not, can you pick this patch up through platform-drivers-x86?
>
> I'm a bit reluctant. Do we have any evidence that this is the way OS X
> behaves? What's the actual difference in state before and after suspend?

I cannot see any change in state on either the LP8550 or on the Intel GPU side.
Both me and Jani have twisted our heads around this but cannot see anything
wrong. The ACPI methods only trigger the opregion so that's a dead end as well.
My guess is that OSX is quirking this in the GPU driver which is something that
doesn't belong in i915.

So unfortunately, this is the best approach I can come up with without probing
the signal or reversing the OSX GPU driver.

Thanks
Patrik

> --
> Matthew Garrett 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] mba6x_bl: Backlight driver for mid 2013 MacBook Air

2014-05-05 Thread Patrik Jakobsson
Hi Matthew,

This seems to be in good shape. Do you have anything to add?
If not, can you pick this patch up through platform-drivers-x86?

Thanks
Patrik

On Tue, Apr 29, 2014 at 4:21 PM, Patrik Jakobsson
 wrote:
> This driver takes control over the LP8550 backlight driver chip found
> in the mid 2013 and newer MacBook Air (6,1 and 6,2). The i915 GPU driver
> cannot properly restore the backlight after resume, but with this driver
> we can hijack the LP8550 and get fully functional backlight support.
>
> v2: - Dropped "if ACPI" in Kconfig since we already depend on it
> - Added comment about brightness mapping
> - Removed lp8550_init() from set_brightness()
> - Always write to dev_ctl when setting brightness
> - Change %Ld to standard C %lld
> - Constify the backlight_ops struct
>
> Signed-off-by: Patrik Jakobsson 
> ---
>  MAINTAINERS |   6 +
>  drivers/platform/x86/Kconfig|  13 ++
>  drivers/platform/x86/Makefile   |   1 +
>  drivers/platform/x86/mba6x_bl.c | 353 
> 
>  4 files changed, 373 insertions(+)
>  create mode 100644 drivers/platform/x86/mba6x_bl.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e67ea24..cad3e82 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5576,6 +5576,12 @@ T:   git 
> git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git
>  S:     Maintained
>  F: net/mac80211/rc80211_pid*
>
> +MACBOOK AIR 6,X BACKLIGHT DRIVER
> +M: Patrik Jakobsson 
> +L: platform-driver-...@vger.kernel.org
> +S: Maintained
> +F: drivers/platform/x86/mba6x_bl.c
> +
>  MACVLAN DRIVER
>  M: Patrick McHardy 
>  L: net...@vger.kernel.org
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 27df2c5..10ac918 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -795,6 +795,19 @@ config APPLE_GMUX
>   graphics as well as the backlight. Currently only backlight
>   control is supported by the driver.
>
> +config MBA6X_BL
> +   tristate "MacBook Air 6,x backlight driver"
> +   depends on ACPI
> +   depends on BACKLIGHT_CLASS_DEVICE
> +   select ACPI_VIDEO
> +   help
> +This driver takes control over the LP8550 backlight driver found in
> +some MacBook Air models. Say Y here if you have a MacBook Air from 
> mid
> +2013 or newer.
> +
> +To compile this driver as a module, choose M here: the module will
> +be called mba6x_bl.
> +
>  config INTEL_RST
>  tristate "Intel Rapid Start Technology Driver"
> depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 1a2eafc..9a182fe 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -56,3 +56,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)  += 
> intel-smartconnect.o
>
>  obj-$(CONFIG_PVPANIC)   += pvpanic.o
>  obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o
> +obj-$(CONFIG_MBA6X_BL) += mba6x_bl.o
> diff --git a/drivers/platform/x86/mba6x_bl.c b/drivers/platform/x86/mba6x_bl.c
> new file mode 100644
> index 000..c549667
> --- /dev/null
> +++ b/drivers/platform/x86/mba6x_bl.c
> @@ -0,0 +1,353 @@
> +/*
> + * MacBook Air 6,1 and 6,2 (mid 2013) backlight driver
> + *
> + * Copyright (C) 2014 Patrik Jakobsson (patrik.r.jakobs...@gmail.com)
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published 
> by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define LP8550_SMBUS_ADDR  (0x58 >> 1)
> +#define LP8550_REG_BRIGHTNESS  0
> +#define LP8550_REG_DEV_CTL 1
> +#define LP8550_REG_FAULT   2
> +#define LP8550_REG_IDENT   3
> +#define LP8550_REG_DIRECT_CTL  4
> +#define LP8550_REG_TEMP_MSB5 /* Must be read before TEMP_LSB */
> +#define LP8550_REG_TEMP_LSB6
> +
> +#define INIT_BRIGHTNESS150
> +
> +static struct {
> +   u8 brightness;  /* Brightness control */
> +   u8 dev_ctl; /* Device control */
> +   u8 fault;   /* Fault indication */
> +   u8 ident;   

[PATCH v2] mba6x_bl: Backlight driver for mid 2013 MacBook Air

2014-04-29 Thread Patrik Jakobsson
This driver takes control over the LP8550 backlight driver chip found
in the mid 2013 and newer MacBook Air (6,1 and 6,2). The i915 GPU driver
cannot properly restore the backlight after resume, but with this driver
we can hijack the LP8550 and get fully functional backlight support.

v2: - Dropped "if ACPI" in Kconfig since we already depend on it
- Added comment about brightness mapping
- Removed lp8550_init() from set_brightness()
- Always write to dev_ctl when setting brightness
- Change %Ld to standard C %lld
- Constify the backlight_ops struct

Signed-off-by: Patrik Jakobsson 
---
 MAINTAINERS |   6 +
 drivers/platform/x86/Kconfig|  13 ++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/mba6x_bl.c | 353 
 4 files changed, 373 insertions(+)
 create mode 100644 drivers/platform/x86/mba6x_bl.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e67ea24..cad3e82 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5576,6 +5576,12 @@ T:   git 
git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git
 S: Maintained
 F: net/mac80211/rc80211_pid*
 
+MACBOOK AIR 6,X BACKLIGHT DRIVER
+M:     Patrik Jakobsson 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/mba6x_bl.c
+
 MACVLAN DRIVER
 M: Patrick McHardy 
 L: net...@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 27df2c5..10ac918 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -795,6 +795,19 @@ config APPLE_GMUX
  graphics as well as the backlight. Currently only backlight
  control is supported by the driver.
 
+config MBA6X_BL
+   tristate "MacBook Air 6,x backlight driver"
+   depends on ACPI
+   depends on BACKLIGHT_CLASS_DEVICE
+   select ACPI_VIDEO
+   help
+This driver takes control over the LP8550 backlight driver found in
+some MacBook Air models. Say Y here if you have a MacBook Air from mid
+2013 or newer.
+
+To compile this driver as a module, choose M here: the module will
+be called mba6x_bl.
+
 config INTEL_RST
 tristate "Intel Rapid Start Technology Driver"
depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 1a2eafc..9a182fe 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -56,3 +56,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)  += intel-smartconnect.o
 
 obj-$(CONFIG_PVPANIC)   += pvpanic.o
 obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o
+obj-$(CONFIG_MBA6X_BL) += mba6x_bl.o
diff --git a/drivers/platform/x86/mba6x_bl.c b/drivers/platform/x86/mba6x_bl.c
new file mode 100644
index 000..c549667
--- /dev/null
+++ b/drivers/platform/x86/mba6x_bl.c
@@ -0,0 +1,353 @@
+/*
+ * MacBook Air 6,1 and 6,2 (mid 2013) backlight driver
+ *
+ * Copyright (C) 2014 Patrik Jakobsson (patrik.r.jakobs...@gmail.com)
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define LP8550_SMBUS_ADDR  (0x58 >> 1)
+#define LP8550_REG_BRIGHTNESS  0
+#define LP8550_REG_DEV_CTL 1
+#define LP8550_REG_FAULT   2
+#define LP8550_REG_IDENT   3
+#define LP8550_REG_DIRECT_CTL  4
+#define LP8550_REG_TEMP_MSB5 /* Must be read before TEMP_LSB */
+#define LP8550_REG_TEMP_LSB6
+
+#define INIT_BRIGHTNESS150
+
+static struct {
+   u8 brightness;  /* Brightness control */
+   u8 dev_ctl; /* Device control */
+   u8 fault;   /* Fault indication */
+   u8 ident;   /* Identification */
+   u8 direct_ctl;  /* Direct control */
+   u8 temp_msb;/* Temperature MSB  */
+   u8 temp_lsb;/* Temperature LSB */
+} lp8550_regs;
+
+static struct platform_device *platform_device;
+static struct backlight_device *backlight_device;
+
+static int lp8550_reg_read(u8 reg, u8 *val)
+{
+   acpi_status status;
+   acpi_handle handle;
+   struct acpi_object_list arg_list;
+   struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+   union acpi_object args[2];
+   union acpi_object *result;
+   int ret = 0;
+
+   status = acpi_get_handle(NULL, "\\_SB.PCI0.SBUS.SRDB", &handle);
+   if (ACPI_FAILURE(status)) {
+   pr_debug("mba6x_bl: Failed to get acpi handle\n");
+   ret = -ENODEV;
+   go

Re: [RFC PATCH] mba6x_bl: Backlight driver for mid 2013 MacBook Air

2014-04-29 Thread Patrik Jakobsson
On Tue, Apr 29, 2014 at 1:10 PM, Hans de Goede  wrote:
>
> Hi,
>
> Why is this patch an RFC? If it is ready for upstreaming please drop
> the RFC prefix when you post the next version.

I'll do that

> On 04/27/2014 10:56 PM, Patrik Jakobsson wrote:
> > This driver takes control over the LP8550 backlight driver chip found
> > in the mid 2013 and newer MacBook Air (6,1 and 6,2). The i915 GPU driver
> > cannot properly restore the backlight after resume, but with this driver
> > we can hijack the LP8550 and get fully functional backlight support.
> >
> > Signed-off-by: Patrik Jakobsson 
> > ---
> >  MAINTAINERS |   6 +
> >  drivers/platform/x86/Kconfig|  13 ++
> >  drivers/platform/x86/Makefile   |   1 +
> >  drivers/platform/x86/mba6x_bl.c | 351 
> > 
> >  4 files changed, 371 insertions(+)
> >  create mode 100644 drivers/platform/x86/mba6x_bl.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index e67ea24..cad3e82 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5576,6 +5576,12 @@ T: git 
> > git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git
> >  S:   Maintained
> >  F:   net/mac80211/rc80211_pid*
> >
> > +MACBOOK AIR 6,X BACKLIGHT DRIVER
> > +M:   Patrik Jakobsson 
> > +L:   platform-driver-...@vger.kernel.org
> > +S:   Maintained
> > +F:   drivers/platform/x86/mba6x_bl.c
> > +
> >  MACVLAN DRIVER
> >  M:   Patrick McHardy 
> >  L:   net...@vger.kernel.org
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 27df2c5..1308924 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -795,6 +795,19 @@ config APPLE_GMUX
> > graphics as well as the backlight. Currently only backlight
> > control is supported by the driver.
> >
> > +config MBA6X_BL
> > + tristate "MacBook Air 6,x backlight driver"
> > + depends on ACPI
> > + depends on BACKLIGHT_CLASS_DEVICE
> > +select ACPI_VIDEO if ACPI
>
> You can drop the if ACPI here, as you already DEPEND on it above

Ok, will do

> > + help
> > +  This driver takes control over the LP8550 backlight driver found in
> > +  some MacBook Air models. Say Y here if you have a MacBook Air from 
> > mid
> > +  2013 or newer.
> > +
> > +  To compile this driver as a module, choose M here: the module will
> > +  be called mba6x_bl.
> > +
> >  config INTEL_RST
> >  tristate "Intel Rapid Start Technology Driver"
> >   depends on ACPI
> > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> > index 1a2eafc..9a182fe 100644
> > --- a/drivers/platform/x86/Makefile
> > +++ b/drivers/platform/x86/Makefile
> > @@ -56,3 +56,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)+= 
> > intel-smartconnect.o
> >
> >  obj-$(CONFIG_PVPANIC)   += pvpanic.o
> >  obj-$(CONFIG_ALIENWARE_WMI)  += alienware-wmi.o
> > +obj-$(CONFIG_MBA6X_BL)   += mba6x_bl.o
> > diff --git a/drivers/platform/x86/mba6x_bl.c 
> > b/drivers/platform/x86/mba6x_bl.c
> > new file mode 100644
> > index 000..022bc8d
> > --- /dev/null
> > +++ b/drivers/platform/x86/mba6x_bl.c
> > @@ -0,0 +1,351 @@
> > +/*
> > + * MacBook Air 6,1 and 6,2 (mid 2013) backlight driver
> > + *
> > + * Copyright (C) 2014 Patrik Jakobsson (patrik.r.jakobs...@gmail.com)
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as 
> > published by
> > + * the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software Foundation.
> > + *
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define LP8550_SMBUS_ADDR(0x58 >> 1)
> > +#define LP8550_REG_BRIGHTNESS0
> > +#define LP8550_REG_DEV_CTL   1
>

[RFC PATCH] mba6x_bl: Backlight driver for mid 2013 MacBook Air

2014-04-27 Thread Patrik Jakobsson
This driver takes control over the LP8550 backlight driver chip found
in the mid 2013 and newer MacBook Air (6,1 and 6,2). The i915 GPU driver
cannot properly restore the backlight after resume, but with this driver
we can hijack the LP8550 and get fully functional backlight support.

Signed-off-by: Patrik Jakobsson 
---
 MAINTAINERS |   6 +
 drivers/platform/x86/Kconfig|  13 ++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/mba6x_bl.c | 351 
 4 files changed, 371 insertions(+)
 create mode 100644 drivers/platform/x86/mba6x_bl.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e67ea24..cad3e82 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5576,6 +5576,12 @@ T:   git 
git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git
 S: Maintained
 F: net/mac80211/rc80211_pid*
 
+MACBOOK AIR 6,X BACKLIGHT DRIVER
+M: Patrik Jakobsson 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/mba6x_bl.c
+
 MACVLAN DRIVER
 M: Patrick McHardy 
 L: net...@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 27df2c5..1308924 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -795,6 +795,19 @@ config APPLE_GMUX
  graphics as well as the backlight. Currently only backlight
  control is supported by the driver.
 
+config MBA6X_BL
+   tristate "MacBook Air 6,x backlight driver"
+   depends on ACPI
+   depends on BACKLIGHT_CLASS_DEVICE
+select ACPI_VIDEO if ACPI
+   help
+This driver takes control over the LP8550 backlight driver found in
+some MacBook Air models. Say Y here if you have a MacBook Air from mid
+2013 or newer.
+
+To compile this driver as a module, choose M here: the module will
+be called mba6x_bl.
+
 config INTEL_RST
 tristate "Intel Rapid Start Technology Driver"
depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 1a2eafc..9a182fe 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -56,3 +56,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)  += intel-smartconnect.o
 
 obj-$(CONFIG_PVPANIC)   += pvpanic.o
 obj-$(CONFIG_ALIENWARE_WMI)+= alienware-wmi.o
+obj-$(CONFIG_MBA6X_BL) += mba6x_bl.o
diff --git a/drivers/platform/x86/mba6x_bl.c b/drivers/platform/x86/mba6x_bl.c
new file mode 100644
index 000..022bc8d
--- /dev/null
+++ b/drivers/platform/x86/mba6x_bl.c
@@ -0,0 +1,351 @@
+/*
+ * MacBook Air 6,1 and 6,2 (mid 2013) backlight driver
+ *
+ * Copyright (C) 2014 Patrik Jakobsson (patrik.r.jakobs...@gmail.com)
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define LP8550_SMBUS_ADDR  (0x58 >> 1)
+#define LP8550_REG_BRIGHTNESS  0
+#define LP8550_REG_DEV_CTL 1
+#define LP8550_REG_FAULT   2
+#define LP8550_REG_IDENT   3
+#define LP8550_REG_DIRECT_CTL  4
+#define LP8550_REG_TEMP_MSB5 /* Must be read before TEMP_LSB */
+#define LP8550_REG_TEMP_LSB6
+
+#define INIT_BRIGHTNESS150
+
+static struct {
+   u8 brightness;  /* Brightness control */
+   u8 dev_ctl; /* Device control */
+   u8 fault;   /* Fault indication */
+   u8 ident;   /* Identification */
+   u8 direct_ctl;  /* Direct control */
+   u8 temp_msb;/* Temperature MSB  */
+   u8 temp_lsb;/* Temperature LSB */
+} lp8550_regs;
+
+static struct platform_device *platform_device;
+static struct backlight_device *backlight_device;
+
+static int lp8550_reg_read(u8 reg, u8 *val)
+{
+   acpi_status status;
+   acpi_handle handle;
+   struct acpi_object_list arg_list;
+   struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+   union acpi_object args[2];
+   union acpi_object *result;
+   int ret = 0;
+
+   status = acpi_get_handle(NULL, "\\_SB.PCI0.SBUS.SRDB", &handle);
+   if (ACPI_FAILURE(status)) {
+   pr_debug("mba6x_bl: Failed to get acpi handle\n");
+   ret = -ENODEV;
+   goto out;
+   }
+
+   args[0].type = ACPI_TYPE_INTEGER;
+   args[0].integer.value = (LP8550_SMBUS_ADDR << 1) | 1;
+

Backlight driver for MacBook Air 6,1 and 6,2

2014-01-20 Thread Patrik Jakobsson
Hi Andrew and CCs

I've put together (rather quickly) a driver for directly handling the backlight
driver chip (LM8550) on the 2013 MacBook Air. It is needed to work around a bug
(likely in firmware) that occurs after suspend/resume. See:

https://bugs.freedesktop.org/show_bug.cgi?id=67454

This seems to fall outside of what the i915 driver should handle and thus need a
separate driver. It's available at: https://github.com/patjak/mba6x_bl

The MacBook Air provides ACPI backlight methods but they also break after
suspend. I'm planning to mainline this and have a few questions.

1) I'm accessing the LP8550 on the SMBUS through ACPI methods. Should I access
the SMBUS directly instead or is this ok? I probably need to look at locking
around SMBUS accesses.

2) Is DMI the proper way of probing? Currently I'm just checking if the chip is
there and that it returns the proper contents in an identifier byte.

3) I assume the backlight type should be BACKLIGHT_PLATFORM (currently
BACKLIGHT_FIRMWARE) but do I also need to blacklist the ACPI backlight on these
devices? How do I get the proper precedence over other backlight devices?

Is there still time to get this into 3.14-rc1?

Thanks
Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 08/85] drivers: gpu: Mark function as static in cdv_intel_dp.c

2014-01-06 Thread Patrik Jakobsson
On Mon, Jan 6, 2014 at 4:02 PM, Rashika Kheria  wrote:
> Mark function cdv_intel_fixed_panel_mode() as static in
> drm/gma500/cdv_intel_dp.c because it is not used outside this file.
>
> This eliminates the following warning in drm/gma500/cdv_intel_dp.c:
> drivers/gpu/drm/gma500/cdv_intel_dp.c:680:6: warning: no previous prototype 
> for ‘cdv_intel_fixed_panel_mode’ [-Wmissing-prototypes]
>
> Signed-off-by: Rashika Kheria 
> Reviewed-by: Josh Triplett 
> ---
>  drivers/gpu/drm/gma500/cdv_intel_dp.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c 
> b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> index f88a181..36975bd 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> @@ -677,7 +677,7 @@ cdv_intel_dp_i2c_init(struct gma_connector *connector,
> return ret;
>  }
>
> -void cdv_intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
> +static void cdv_intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
> struct drm_display_mode *adjusted_mode)
>  {
> adjusted_mode->hdisplay = fixed_mode->hdisplay;
> --
> 1.7.9.5
>

Pushed to gma500-next

Thanks
Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Black screen with GMA500 driver on Atom E680 (invalid VBT signature)

2013-11-14 Thread Patrik Jakobsson
On Thu, Nov 14, 2013 at 2:52 PM, One Thousand Gnomes
 wrote:
>> My question now is, is it a bug in the driver or in the VGA Bios?
>>
>> I´ve checked the VGA bios and did a hex dump but there is really no "$VBT" 
>> string.
>> On an Atom Gen1 CPU with intel GMA500 Graphic there is the string in the VGA 
>> BIOS.
>> But if i create the VGA BIOS for intel Atom E680 which has a GMA600 Graphic
>> there is no string.
>>
>> Does anybody of you running the GMA500 driver with and Atom E680 without 
>> having this
>> problem? Or does anybody know how to get this string into the VGA BIOS?
>
> E680 I believe will try and use the other style of parsing. I never got
> my hands on one to confirm that but in general E6xx seems to use
> Moorestown style tables instead and you'll only get support for some
> outputs.

As Alan says, Moorestown should use GCT as in mid_bios.c and the signature
should be $GCT instead of $VBT. Though I don't know if you get any LVDS info
from from that or just MIPI stuff.

In my last set of patches I included 0x4108 in the IS_MRST macro which might
make gma500 find the LVDS modes via I2C. In the same patchset there is also
support for SDVO on Moorestown.

Patches are available in Dave Airlies drm-next and heading for 3.13-rc1

Thanks
Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [BUG] gma500: sleeping function called from invalid context at kernel/mutex.c:413

2013-11-13 Thread Patrik Jakobsson
On Wed, Nov 13, 2013 at 4:49 PM, Holger Schurig  wrote:
> Kernel: 3.10.19
>
> From time to time, when I booted, I had a completely dark screen (with
> kernel command line quiet) and a non-blinking cursor. I wondered if
> that was perhaps gma500. So I turned on various debug checks. Then
> I've got the BUG from the subject.
>
> The device is a " VGA compatible controller [0300]: Intel Corporation
> System Controller Hub (SCH Poulsbo) Graphics Controller [8086:8108]
> (rev 07)".
>
> Here's relevant "dmesg" output:
>
> ...

Hi

We're calling backlight_update_status() in IRQ context which isn't allowed.

I'll add it to my todo list.

Thanks
Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] gma500: define do_gma_backlight_set only when used

2013-09-26 Thread Patrik Jakobsson
On Thu, Sep 26, 2013 at 11:46 PM, Vincent Stehlé
 wrote:
> Make sure static function do_gma_backlight_set() is only defined when
> CONFIG_BACKLIGHT_CLASS_DEVICE is defined, as it is never called otherwise.
>
> This fixes the following warning:
>
>   drivers/gpu/drm/gma500/backlight.c:29:13: warning: ‘do_gma_backlight_set’ 
> defined but not used [-Wunused-function]
>
> While at it, remove some end of line spaces.
>
> Signed-off-by: Vincent Stehlé 
> Cc: David Airlie 
> ---
>
> Hi,
>
> This can be seen with mainline or linux-next with e.g. allmodconfig on x86.
>
> Best regards,
>
> V.
>
>  drivers/gpu/drm/gma500/backlight.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/backlight.c 
> b/drivers/gpu/drm/gma500/backlight.c
> index 143eba3..399731e 100644
> --- a/drivers/gpu/drm/gma500/backlight.c
> +++ b/drivers/gpu/drm/gma500/backlight.c
> @@ -26,13 +26,13 @@
>  #include "intel_bios.h"
>  #include "power.h"
>
> +#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
>  static void do_gma_backlight_set(struct drm_device *dev)
>  {
> -#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
> struct drm_psb_private *dev_priv = dev->dev_private;
> backlight_update_status(dev_priv->backlight_device);
> -#endif
>  }
> +#endif
>
>  void gma_backlight_enable(struct drm_device *dev)
>  {
> @@ -43,7 +43,7 @@ void gma_backlight_enable(struct drm_device *dev)
> dev_priv->backlight_device->props.brightness = 
> dev_priv->backlight_level;
> do_gma_backlight_set(dev);
> }
> -#endif
> +#endif
>  }
>
>  void gma_backlight_disable(struct drm_device *dev)
> @@ -55,7 +55,7 @@ void gma_backlight_disable(struct drm_device *dev)
> dev_priv->backlight_device->props.brightness = 0;
> do_gma_backlight_set(dev);
> }
> -#endif
> +#endif
>  }
>
>  void gma_backlight_set(struct drm_device *dev, int v)
> @@ -67,7 +67,7 @@ void gma_backlight_set(struct drm_device *dev, int v)
> dev_priv->backlight_device->props.brightness = v;
> do_gma_backlight_set(dev);
> }
> -#endif
> +#endif
>  }
>
>  int gma_backlight_init(struct drm_device *dev)
> --
> 1.8.4.rc3

Thanks, looks good, I'll apply it to the gma500-fixes branch.

Acked-by: Patrik Jakobsson 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: gma500: remove double free in psbfb_create

2013-09-20 Thread Patrik Jakobsson
On Fri, Sep 20, 2013 at 3:56 PM, Dave Jones  wrote:
> This code appears to be calling psb_gtt_free_range twice with the same args.
> (The second call didn't appear in the diff output, it's right after the 
> mutex_unlock)
>
> Spotted with Coverity, not tested due to lack of hardware.
>
> Signed-off-by: Dave Jones 
>
> diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
> b/drivers/gpu/drm/gma500/framebuffer.c
> index 01dd7d2..d35ffc4 100644
> --- a/drivers/gpu/drm/gma500/framebuffer.c
> +++ b/drivers/gpu/drm/gma500/framebuffer.c
> @@ -479,9 +479,7 @@ static int psbfb_create(struct psb_fbdev *fbdev,
> mutex_unlock(&dev->struct_mutex);
> return 0;
>  out_unref:
> -   if (backing->stolen)
> -   psb_gtt_free_range(dev, backing);
> -   else
> +   if (!backing->stolen)
> drm_gem_object_unreference(&backing->gem);
>  out_err1:
> mutex_unlock(&dev->struct_mutex);

Hi Dave, thanks for the patch.

This part of the code is a ref count disaster and probably needs to be
reworked anyways. It would be nice to always let drm_gem_object_unreference()
do the final cleanup call to psb_gtt_free_range() but stolen memory is treated
as an exception in some cases. Also we don't seem to take down the framebuffer
correctly on failure after drm_framebuffer_init() which seems scary.
psb_framebuffer_init() calls drm_framebuffer_init() which clearly states that
we need to be done with the setup and stop failing at this point.

Also, we always get a stolen memory backed object (we're even lowering the
depth to make sure we get one) so the drm_gem_object_unreference() is a NOP.

Anyways, your patch looks good but I would like to look at this some more since
it seems broken anyways.

Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Abysmal HDD/USB write speed after sleep on a UEFI system

2013-05-08 Thread Patrik Jakobsson
On Wed, May 8, 2013 at 10:37 AM, Artem S. Tashkinov  wrote:
>>I think this is the official statement from Intel on the SATA issue:
>>http://newsroom.intel.com/community/intel_newsroom/blog/2011/01/31/intel-identifies-chipset-design-error-implementing-solution
>
> My motherboard has a new fixed B3 revision so this issue doesn't affect me.
> Besides this SATA ports degradation issue is constantly present - it has no
> relationship to suspend.

Yes, Rev. B3 should be fine.

>>And here's a link to a discussion about the PCIe-to-PCI bridge stuff:
>>https://lkml.org/lkml/2012/1/30/216
>>
>>> Artem's system has a PCIe-to-PCI bridge (not a PCI-to-PCIe bridge) at
>>> 05:00.0, but it leads to [bus 06] and there's nothing on bus 06, so I
>>> don't think that's the problem.
>>
>>I meant what you said ;) and yes, it seems unrelated. Both my P8H67 and a
>>P8P67 I've built behave nicely if nothing is connected.
>
> Have you tried suspending more than three times? In the absence of UEFI
> boot this bug emerges only on a third or even fourth resume attempt. UEFI
> boot triggers it immediately on a first resume though.

I haven't enabled UEFI boot but did ~10 suspend/resume cycles with no issues.
I'm on 3.9-rc5 if that makes a difference. I'll do some more testing with
various kernel versions to see if I can trigger it.

-Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Abysmal HDD/USB write speed after sleep on a UEFI system

2013-05-07 Thread Patrik Jakobsson
On Wed, May 8, 2013 at 12:02 AM, Bjorn Helgaas  wrote:
> On Tue, May 7, 2013 at 2:48 PM, Patrik Jakobsson
>  wrote:
>> On Tue, May 7, 2013 at 10:20 PM, Bjorn Helgaas  wrote:
>>>> I'm not sure if reading /proc/mtrr actually reads the registers out of
>>>> the CPU each time, or whether we just return the cached values we read
>>>> out during initial boot-up. If the latter, then this output isn't
>>>> really useful as there's no guarantee the values are still intact.
>>>
>>> Good point.  From what I can tell, on Artem's system with "CPU0:
>>> Intel(R) Core(TM) i5-2500 CPU @ 3.30GHz," we would be using
>>> generic_mtrr_ops, and generic_get_mtrr() appears to read from the
>>> MSRs, so I think it should be useful.
>>
>> FWIW, that motherboard suffers from a PCI to PCIE bridge problem. It might
>> have been fixed by bios upgrades by now but not sure.
>>
>> It might also suffer (depending on the revision) from the Sandy bridge SATA
>> issue. So if affected, SATA controller is a ticking bomb.
>>
>> I have a P8H67-V motherboard but I haven't seen any suspend related issues.
>>
>> If this is totally unrelated I'm sorry for wasting your time. Just thought it
>> might be good to know.
>
> Thanks for chiming in.  I'm not familiar with either of the issues you
> mentioned.  Do you have any references where I could read up on them?

I think this is the official statement from Intel on the SATA issue:
http://newsroom.intel.com/community/intel_newsroom/blog/2011/01/31/intel-identifies-chipset-design-error-implementing-solution

And here's a link to a discussion about the PCIe-to-PCI bridge stuff:
https://lkml.org/lkml/2012/1/30/216

> Artem's system has a PCIe-to-PCI bridge (not a PCI-to-PCIe bridge) at
> 05:00.0, but it leads to [bus 06] and there's nothing on bus 06, so I
> don't think that's the problem.

I meant what you said ;) and yes, it seems unrelated. Both my P8H67 and a
P8P67 I've built behave nicely if nothing is connected.

> And the issue affects both USB and a hard drive, so I suspect it's
> more than just SATA.  Artem, did you identify the PCI devices leading
> to your USB and hard drive?  I can't remember if I've actually seen
> that.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Abysmal HDD/USB write speed after sleep on a UEFI system

2013-05-07 Thread Patrik Jakobsson
On Tue, May 7, 2013 at 10:20 PM, Bjorn Helgaas  wrote:
>> I'm not sure if reading /proc/mtrr actually reads the registers out of
>> the CPU each time, or whether we just return the cached values we read
>> out during initial boot-up. If the latter, then this output isn't
>> really useful as there's no guarantee the values are still intact.
>
> Good point.  From what I can tell, on Artem's system with "CPU0:
> Intel(R) Core(TM) i5-2500 CPU @ 3.30GHz," we would be using
> generic_mtrr_ops, and generic_get_mtrr() appears to read from the
> MSRs, so I think it should be useful.

FWIW, that motherboard suffers from a PCI to PCIE bridge problem. It might
have been fixed by bios upgrades by now but not sure.

It might also suffer (depending on the revision) from the Sandy bridge SATA
issue. So if affected, SATA controller is a ticking bomb.

I have a P8H67-V motherboard but I haven't seen any suspend related issues.

If this is totally unrelated I'm sorry for wasting your time. Just thought it
might be good to know.

Thanks
Patrik Jakobsson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: How to override GMA500 resolution set by BIOS?

2013-04-17 Thread Patrik Jakobsson
On Wed, Apr 17, 2013 at 7:39 PM, Marcin Szewczyk
 wrote:
> Hello,
>
> I've got a problem with GMA500 on my EVOC EC2-1711. The graphics
> adapter's resolution is set by selecting an option in BIOS but I would
> like to override it. Can kernel or Xorg do this?
>
> There are two modes "almost" right for an 800x600 16bit LVDS screen I
> have.  The first one is 800x600 but in 24bit LVDS mode, so colors are
> mangled. The second one is 800x480 16bit -- colors are right, but the
> screen cuts the image at line 480.
>
> I've tried video=, overwriting modes via patched 915resolution, vga= and
> Xorg modesetting driver. Nothing works.
>
> Does anyone know if it's possible to override the mode?

Hi Marcin, I haven't played much with 16 bpp on the gma500 kernel driver but at
the moment it seems broken. I added a screen section to xorg.conf with
DefaultDepth 16 running the fbdev driver. That, at least tries to do the right
thing but gma500 fails in setting it up. On the other hand, you managed to set
800x480 at 16bbp. Have you tried the vesafb driver? Might do a better job.

-Patrik Jakobsson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] gma500:fix build failure for 3.9-rc5

2013-04-10 Thread Patrik Jakobsson
On Wed, Apr 10, 2013 at 2:37 PM, Xiong Zhou  wrote:
> From: Xiong Zhou 
>
> Last version of this patch is not clear enough and X86 duplicated.
>
> This patch fixes build failure of v3.9-rc5 and rc6.
> When config ACPI_VIDEO as m, DRM_GMA500 as y, here comes the failure.
> GMA5/600 needs acpi_video just like nouveau.
> And some tab type fix by the way.
>
> Failure message:
> drivers/built-in.o: In function `psb_driver_load':
> kernel-3.9-rc5/drivers/gpu/drm/gma500/psb_drv.c:340: \
> undefined reference to `acpi_video_register'
> make: *** [vmlinux] Error 1
>
> Signed-off-by: Xiong Zhou 
> ---
> drivers/gpu/drm/gma500/Kconfig |   13 +
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
> index 1188f0f..1f6e2df 100644
> --- a/drivers/gpu/drm/gma500/Kconfig
> +++ b/drivers/gpu/drm/gma500/Kconfig
> @@ -2,10 +2,15 @@ config DRM_GMA500
> tristate "Intel GMA5/600 KMS Framebuffer"
> depends on DRM && PCI && X86
> select FB_CFB_COPYAREA
> -select FB_CFB_FILLRECT
> -select FB_CFB_IMAGEBLIT
> -select DRM_KMS_HELPER
> -select DRM_TTM
> +   select FB_CFB_FILLRECT
> +   select FB_CFB_IMAGEBLIT
> +   select DRM_KMS_HELPER
> +   select DRM_TTM
> +   # GMA500 depends on ACPI_VIDEO when ACPI is enabled, just like i915
> +   select ACPI_VIDEO if ACPI
> +   select BACKLIGHT_CLASS_DEVICE if ACPI
> +   select VIDEO_OUTPUT_CONTROL if ACPI
> +   select INPUT if ACPI
> help
>   Say yes for an experimental 2D KMS framebuffer driver for the
>   Intel GMA500 ('Poulsbo') and other Intel IMG based graphics

Your patch has been applied to:
https://github.com/patjak/drm-gma500.git gma500-next

I didn't have c97fc5f regarding the experimental dep in my tree so I had to fix
that. Will sort out the missing patch later.

Thanks
Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] drivers/gpu/drm/gma500:fix build failure for 3.9-rc5

2013-04-09 Thread Patrik Jakobsson
On Tue, Apr 9, 2013 at 8:35 AM, Xiong Zhou  wrote:
> From: Xiong Zhou 
>
> This patch fixes build failure of v3.9-rc5.
> When config ACPI_VIDEO as m, DRM_GMA500 as y, here comes the failure.
> gma5/600 needs acpi_video just like nouveau.
>
> Failure message:
> drivers/built-in.o: In function `psb_driver_load':
> kernel-3.9-rc5/drivers/gpu/drm/gma500/psb_drv.c:340: \
> undefined reference to `acpi_video_register'
> make: *** [vmlinux] Error 1
>
> Signed-off-by: Xiong Zhou 
> ---
> drivers/gpu/drm/gma500/Kconfig |1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
> index 1188f0f..d64fc45 100644
> --- a/drivers/gpu/drm/gma500/Kconfig
> +++ b/drivers/gpu/drm/gma500/Kconfig
> @@ -6,6 +6,7 @@ config DRM_GMA500
>  select FB_CFB_IMAGEBLIT
>  select DRM_KMS_HELPER
>  select DRM_TTM
> +select ACPI_VIDEO if ACPI && X86 && BACKLIGHT_CLASS_DEVICE && 
> VIDEO_OUTPUT_CONTROL && INPUT
> help
>   Say yes for an experimental 2D KMS framebuffer driver for the
>   Intel GMA500 ('Poulsbo') and other Intel IMG based graphics
> --

Hi

Thanks for catching this, but if I can be picky, I'd prefer if you wrote it
like i915 does. E.g.

select ACPI_VIDEO if ACPI
select BACKLIGHT_CLASS_DEVICE if ACPI
...

And you can skip X86 since we already 'depend' on it.

Thanks
Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] drivers: gpu: drm: gma500: Replaced calls kzalloc & memcpy with kmemdup

2013-04-02 Thread Patrik Jakobsson
On Mon, Mar 11, 2013 at 8:46 PM, Alexandru Gheorghiu
 wrote:
> Replaced calls kzalloc followed by memcpy with call to kmemdup.
> Patch found using coccinelle.
>
> Signed-off-by: Alexandru Gheorghiu 
> ---
>  drivers/gpu/drm/gma500/intel_bios.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/intel_bios.c 
> b/drivers/gpu/drm/gma500/intel_bios.c
> index 403fffb..d349734 100644
> --- a/drivers/gpu/drm/gma500/intel_bios.c
> +++ b/drivers/gpu/drm/gma500/intel_bios.c
> @@ -218,12 +218,11 @@ static void parse_backlight_data(struct drm_psb_private 
> *dev_priv,
> bl_start = find_section(bdb, BDB_LVDS_BACKLIGHT);
> vbt_lvds_bl = (struct bdb_lvds_backlight *)(bl_start + 1) + p_type;
>
> -   lvds_bl = kzalloc(sizeof(*vbt_lvds_bl), GFP_KERNEL);
> +   lvds_bl = kmemdup(vbt_lvds_bl, sizeof(*vbt_lvds_bl), GFP_KERNEL);
> if (!lvds_bl) {
> dev_err(dev_priv->dev->dev, "out of memory for backlight 
> data\n");
> return;
> }
> -   memcpy(lvds_bl, vbt_lvds_bl, sizeof(*vbt_lvds_bl));
> dev_priv->lvds_bl = lvds_bl;
>  }
>
> --
> 1.7.9.5
>
> --

Your patch has been applied to:
https://github.com/patjak/drm-gma500.git gma500-next

Thanks
Patrik Jakobsson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH]gma500: remove unused drm_psb_no_fb

2013-04-02 Thread Patrik Jakobsson
On Wed, Mar 13, 2013 at 9:19 AM, Wang YanQing  wrote:
> commit f9f23a77f07506a32d9dc1d925bf85c0e7507b66(gma500: remove no_fb bits)
> remove all the drm_psb_no_fb relations code in gma500 except this line code,
> so remove it also.
>
> Signed-off-by: Wang YanQing 
> ---
>  drivers/gpu/drm/gma500/psb_drv.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gma500/psb_drv.h 
> b/drivers/gpu/drm/gma500/psb_drv.h
> index a7fd6c4..6053b8a 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.h
> +++ b/drivers/gpu/drm/gma500/psb_drv.h
> @@ -876,7 +876,6 @@ extern const struct psb_ops cdv_chip_ops;
>  #define PSB_D_MSVDX   (1 << 9)
>  #define PSB_D_TOPAZ   (1 << 10)
>
> -extern int drm_psb_no_fb;
>  extern int drm_idle_check_interval;
>
>  /*
> --
> 1.7.12.4.dirty
> --

Your patch has been applied to:
https://github.com/patjak/drm-gma500.git gma500-next

Thanks
Patrik Jakobsson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] gma500: Make VGA and HDMI connector hotpluggable

2013-04-02 Thread Patrik Jakobsson
On Sun, Mar 31, 2013 at 1:38 PM, Kero  wrote:
> From: Kero van Gelder 
>
> Both VGA and HDMI connectors are available on my Asus EeePC X101CH.
> This patch will cause output to be shown on either when plugged in.
> For both, it shows the leftmost 800x600, of the 1024x600 on LVDS.
>
> Signed-off-by: Kero van Gelder 
> ---
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_crt.c 
> b/drivers/gpu/drm/gma500/cdv_intel_crt.c
> index 8c17534..7b8386f 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_crt.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_crt.c
> @@ -276,6 +276,7 @@ void cdv_intel_crt_init(struct drm_device *dev,
> goto failed_connector;
>
> connector = &psb_intel_connector->base;
> +   connector->polled = DRM_CONNECTOR_POLL_HPD;
> drm_connector_init(dev, connector,
> &cdv_intel_crt_connector_funcs, DRM_MODE_CONNECTOR_VGA);
>
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c 
> b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
> index e223b50..464153d 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
> @@ -319,6 +319,7 @@ void cdv_hdmi_init(struct drm_device *dev,
> goto err_priv;
>
> connector = &psb_intel_connector->base;
> +   connector->polled = DRM_CONNECTOR_POLL_HPD;
> encoder = &psb_intel_encoder->base;
> drm_connector_init(dev, connector,
>&cdv_hdmi_connector_funcs,
> --

Your patch has been applied to:
https://github.com/patjak/drm-gma500.git gma500-next

We might also consider polling if this causes problems for people, but for now
this is fine. No biggie, but your tabs where converted to spaces so I recommend
running checkpatch.pl before submitting.

Thanks
Patrik Jakobsson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] drivers: gpu: drm: gma500: Replaced calls kzalloc & memcpy with kmemdup

2013-03-11 Thread Patrik Jakobsson
On Mon, Mar 11, 2013 at 8:46 PM, Alexandru Gheorghiu
 wrote:
> Replaced calls kzalloc followed by memcpy with call to kmemdup.
> Patch found using coccinelle.
>
> Signed-off-by: Alexandru Gheorghiu 
> ---
>  drivers/gpu/drm/gma500/intel_bios.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/intel_bios.c 
> b/drivers/gpu/drm/gma500/intel_bios.c
> index 403fffb..d349734 100644
> --- a/drivers/gpu/drm/gma500/intel_bios.c
> +++ b/drivers/gpu/drm/gma500/intel_bios.c
> @@ -218,12 +218,11 @@ static void parse_backlight_data(struct drm_psb_private 
> *dev_priv,
> bl_start = find_section(bdb, BDB_LVDS_BACKLIGHT);
> vbt_lvds_bl = (struct bdb_lvds_backlight *)(bl_start + 1) + p_type;
>
> -   lvds_bl = kzalloc(sizeof(*vbt_lvds_bl), GFP_KERNEL);
> +   lvds_bl = kmemdup(vbt_lvds_bl, sizeof(*vbt_lvds_bl), GFP_KERNEL);
> if (!lvds_bl) {
> dev_err(dev_priv->dev->dev, "out of memory for backlight 
> data\n");
> return;
> }
> -   memcpy(lvds_bl, vbt_lvds_bl, sizeof(*vbt_lvds_bl));
> dev_priv->lvds_bl = lvds_bl;
>  }
>
> --
> 1.7.9.5

Reviewed-by: Patrik Jakobsson 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 54/86] drm/i915: Set i9xx sdvo clock limits according to specifications

2013-02-27 Thread Patrik Jakobsson
On Wed, Feb 27, 2013 at 11:19 AM, Chris Wilson  wrote:
> On Wed, Feb 27, 2013 at 11:11:38AM +0100, Patrik Jakobsson wrote:
>> On Wed, Feb 27, 2013 at 1:08 AM, Greg Kroah-Hartman
>>  wrote:
>> > 3.4-stable review patch.  If anyone has any objections, please let me know.
>> >
>> > --
>> >
>> > From: Patrik Jakobsson 
>> >
>> > commit 4f7dfb6788dd022446847fbbfbe45e13bedb5be2 upstream.
>
>> Is this really stable material? It fixes no known issues and the bug
>> report seems to be a different problem.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=59066 is a genuine bug
> fixed.
> -Chris

Great, then I have no problems with this going in.
-Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 54/86] drm/i915: Set i9xx sdvo clock limits according to specifications

2013-02-27 Thread Patrik Jakobsson
On Wed, Feb 27, 2013 at 1:08 AM, Greg Kroah-Hartman
 wrote:
> 3.4-stable review patch.  If anyone has any objections, please let me know.
>
> --
>
> From: Patrik Jakobsson 
>
> commit 4f7dfb6788dd022446847fbbfbe45e13bedb5be2 upstream.
>
> The Intel PRM says the M1 and M2 divisors must be in the range of 10-20 and 
> 5-9.
> Since we do all calculations based on them being register values (which are
> subtracted by 2) we need to specify them accordingly.
>
> Signed-off-by: Patrik Jakobsson 
> Reviewed-by: Chris Wilson 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=56359
> Signed-off-by: Daniel Vetter 
> Signed-off-by: Greg Kroah-Hartman 
>
> ---
>  drivers/gpu/drm/i915/intel_display.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -142,8 +142,8 @@ static const intel_limit_t intel_limits_
> .vco = { .min = 140, .max = 280 },
> .n = { .min = 1, .max = 6 },
> .m = { .min = 70, .max = 120 },
> -   .m1 = { .min = 10, .max = 22 },
> -   .m2 = { .min = 5, .max = 9 },
> +   .m1 = { .min = 8, .max = 18 },
> +   .m2 = { .min = 3, .max = 7 },
> .p = { .min = 5, .max = 80 },
> .p1 = { .min = 1, .max = 8 },
> .p2 = { .dot_limit = 20,

Is this really stable material? It fixes no known issues and the bug
report seems to be a different problem.

-Patrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/