Re: [PATCH] drm/gma500: use NULL instead of using plain integer as pointer
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
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
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
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
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
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
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()
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
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
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
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
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
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
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_
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()
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()
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()
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}()
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().
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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?
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
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
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
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
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
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
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
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
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/