Re: [Intel-gfx] [PATCH 1/2] drm/i915: Updating plane parameters for primary plane in setplane
On 8/20/2014 2:20 AM, Matt Roper wrote:
On Tue, Aug 19, 2014 at 11:56:42AM +0530, sonika.jin...@intel.com wrote:
From: Sonika Jindal
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/intel_display.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index e9b578e..1b0e403 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11528,6 +11528,21 @@ intel_primary_plane_setplane(struct drm_plane *plane,
struct drm_crtc *crtc,
.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
};
+ const struct {
+ int crtc_x, crtc_y;
+ unsigned int crtc_w, crtc_h;
+ uint32_t src_x, src_y, src_w, src_h;
+ } orig = {
+ .crtc_x = crtc_x,
+ .crtc_y = crtc_y,
+ .crtc_w = crtc_w,
+ .crtc_h = crtc_h,
+ .src_x = src_x,
+ .src_y = src_y,
+ .src_w = src_w,
+ .src_h = src_h,
+ };
+ struct intel_plane *intel_plane = to_intel_plane(plane);
bool visible;
int ret;
@@ -11604,6 +11619,15 @@ intel_primary_plane_setplane(struct drm_plane *plane,
struct drm_crtc *crtc,
return 0;
}
+ intel_plane->crtc_x = orig.crtc_x;
+ intel_plane->crtc_y = orig.crtc_y;
+ intel_plane->crtc_w = orig.crtc_w;
+ intel_plane->crtc_h = orig.crtc_h;
+ intel_plane->src_x = orig.src_x;
+ intel_plane->src_y = orig.src_y;
+ intel_plane->src_w = orig.src_w;
+ intel_plane->src_h = orig.src_h;
+ intel_plane->obj = obj;
ret = intel_pipe_set_base(crtc, src.x1, src.y1, fb);
if (ret)
I haven't been following all of this thread carefully, but wouldn't you
want to update the intel_plane fields after the point where the function
can no longer fail? E.g., if I try to setplane() to a new location
while I have a pageflip pending, my request will fail and the plane
position won't update, yet you're still going to be updating the
internal state tracking variables here. Then if you do an
intel_plane_restore() later you're going to see the plane jump
unexpectedly.
On the other hand, what about the case where the setplane succeeds, but
the plane isn't visible (either because it's fully clipped or because
your crtc isn't active right now). Those are other paths through the
intel_primary_plane_setplane() function that don't seem to be updating
the intel_plane fields, even though it seems like you'd want to so that
your plane doesn't snap back to an old position on a later
intel_plane_restore().
I understand your points, but I am not sure why the updation of these
intel_plane member variables was left in the first place. Do you a
reason why this was not present till now? Am I missing something?
Sorry if I'm overlooking something obvious here; I haven't been keeping
up with this whole email thread.
Matt
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add 180 degree primary plane rotation support
On 8/20/2014 4:21 AM, Matt Roper wrote:
On Tue, Aug 19, 2014 at 11:56:43AM +0530, sonika.jin...@intel.com wrote:
From: Sonika Jindal
Primary planes support 180 degree rotation. Expose the feature
through rotation drm property.
v2: Calculating linear/tiled offsets based on pipe source width and
height. Added 180 degree rotation support in ironlake_update_plane.
v3: Checking if CRTC is active before issueing update_plane. Added
wait for vblank to make sure we dont overtake page flips. Disabling
FBC since it does not work with rotated planes.
v4: Updated rotation checks for pending flips, fbc disable. Creating
rotation property only for Gen4 onwards. Property resetting as part
of lastclose.
v5: Resetting property in i915_driver_lastclose properly for planes
and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
width in i9xx_update_plane and ironlake_update_plane. Removed tab
based indentation and unnecessary braces in intel_crtc_set_property
and intel_update_fbc. FBC and flip related checks should be done only
for valid crtcs.
v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
and positioning the disable code in intel_update_fbc.
v7: In case rotation property on inactive crtc is updated, we return
successfully printing debug log as crtc is inactive and only property change
is preserved.
v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
crtc->primary->fb and return value of update_primary_plane is ignored.
v9: added rotation property to primary plane instead of crtc. Removing reset
of rotation property from lastclose. rotation_property is moved to
drm_mode_config, so drm layer will take care of resetting. Adding updation of
fbc when rotation is set to 0. Allowing rotation only if value is
different than old one.
v10: Calling intel_primary_plane_setplane instead of update_primary_plane in
set_property(Daniel).
Testcase: igt/kms_rotation_crc
Signed-off-by: Uma Shankar
Signed-off-by: Sagar Kamble
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/i915_reg.h |1 +
drivers/gpu/drm/i915/intel_display.c | 116 --
drivers/gpu/drm/i915/intel_pm.c |6 ++
3 files changed, 119 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7a6cc69..f8aaf0b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4209,6 +4209,7 @@ enum punit_power_well {
#define DISPPLANE_NO_LINE_DOUBLE0
#define DISPPLANE_STEREO_POLARITY_FIRST 0
#define DISPPLANE_STEREO_POLARITY_SECOND(1<<18)
+#define DISPPLANE_ROTATE_180 (1<<15)
#define DISPPLANE_TRICKLE_FEED_DISABLE (1<<14) /* Ironlake */
#define DISPPLANE_TILED (1<<10)
#define _DSPAADDR 0x70184
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1b0e403..cc2999b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2332,6 +2332,9 @@ static void i9xx_update_primary_plane(struct drm_crtc
*crtc,
unsigned long linear_offset;
u32 dspcntr;
u32 reg = DSPCNTR(plane);
+ int pixel_size;
+
+ pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
if (!intel_crtc->primary_enabled) {
I915_WRITE(reg, 0);
@@ -2359,6 +2362,7 @@ static void i9xx_update_primary_plane(struct drm_crtc
*crtc,
(intel_crtc->config.pipe_src_w - 1));
I915_WRITE(DSPPOS(plane), 0);
}
+ dspcntr &= ~DISPPLANE_ROTATE_180;
switch (fb->pixel_format) {
case DRM_FORMAT_C8:
@@ -2398,8 +2402,6 @@ static void i9xx_update_primary_plane(struct drm_crtc
*crtc,
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
- I915_WRITE(reg, dspcntr);
-
linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
if (INTEL_INFO(dev)->gen >= 4) {
@@ -2412,6 +2414,21 @@ static void i9xx_update_primary_plane(struct drm_crtc
*crtc,
intel_crtc->dspaddr_offset = linear_offset;
}
+ if (to_intel_plane(crtc->primary)->rotation == BIT(DRM_ROTATE_180)) {
+ dspcntr |= DISPPLANE_ROTATE_180;
+
+ x += (intel_crtc->config.pipe_src_w - 1);
+ y += (intel_crtc->config.pipe_src_h - 1);
+
+ /* Finding the last pixel of the last line of the display
+ data and adding to linear_offset*/
+ linear_offset += (intel_crtc->config.pipe_src_h - 1) *
+ fb->pitches[0] +
+ (intel_crtc->config.pipe_src_w - 1) * pixel_size;
+ }
+
+ I915_WRITE(reg, dspcntr);
+
DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
Re: [Intel-gfx] How to create PCH to support those existing driver
On 2014/8/20 5:51, Michael S. Tsirkin wrote: On Tue, Aug 19, 2014 at 09:24:03PM +, Kay, Allen M wrote: Allen, Could you reply this? Let me summarized what we have discussed and learned so far: 1) Future Windows/Linux IGD drivers will be modified to restrain from accessing MCH/PCH devices. We are prototyping this in Windows driver right now and will pass the same methodology to Linux driver once we have a workable solution. The goal is removing all MCH/PCH accesses in the IGD driver. 2) We want the same solution to work in both native and virtualization environments. Given most driver developers test their changes only in native environment, doing anything specific for virtualization in the driver will cause frequent breakage for virtualization use cases. 3) Back porting this new code to support previous generations of HW will be problematic if not impossible. Each Windows IGD driver release binary supports two generations of HW. For example, 15.36 driver supports Broadwell/Haswell, 15.33 driver supports Haswell/IvyBridge, 15.31 driver supports Ivybridge/Sandybridge, etc. Once the driver is product validated, there is little opportunity to go back and make high impact changes that might affect stability in native environment. 4) I agree there is little reason to do anything that requires driver changes at this point, unless it is the final desired solution. The question is whether/how to support IGD passthrough for previous generations of HW? 1) If we want to support SandyBridge/IvyBridge/Haswell/Broadwell, we will need most of the original QEMU patches. We might be able to do without igd_pci_write(). I have tested QEMU changes without igd_pci_write() on both Haswell/Broadwell and Windows booted without any problems. This will limit only read operations which should reduce a quite a bit of risk to the host platform. Excellent. I was thinking about changing host's driver to do the writes in a safe manner, but if don't need that, all the better. As a next step, we need to limit read operations to specific set of registers that we can validate. We can't just pass through reads blindly to host, pci reads have side-effects and host chipset isn't protected by the iommu. Since these are legacy devices and drivers, it should be possible to enumerate all registers that they need. 2) If we want the upstream QEMU only work with future driver version, then most of the IGD passthrough patch is probably not needed - with exception of opregion mapping handlers. The downside is products that depend on this feature will need to apply private patches separately to re-enable IGD passthrough capability. Any advice on how should Tiejun proceed from here? Allen I'm fine with either trying to make existing windows and linux drivers work, or waiting for future drivers. Michael and Allen, As I concern, now we may not take a consideration of supporting those existing drivers, just leave to qemu-traditional in Xen code repertory. I think what we should do here just focus on supporting all platforms including those legacy platforms. To me, quick hacks that need minor changes in driver but don't avoid poking at MCH/PCH don't seem to have value, So to me, that subsystem id way is more clear rather than others because I'm not sure its really possible not to poke MCH/PCH theoretically both Windows and Linux driver. Allen, I think Michael is saying this, http://article.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/42258 What about your opinion? I know I proposed some of these myself but this was before I realised a cleaner solution is possible. I think all we are fine if this really come true :) Tiejun ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: switch off backlight for backlight class 0 brightness
On 08/12/2014 07:11 AM, Jani Nikula wrote:
Make backlight class sysfs brightness 0 value switch off the backlight
for connectors that have the backlight_power callback defined. For eDP,
this has the similar caveats regarding power savings as bl_power as only
the power sequencer backlight control is switched off.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_panel.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index c365f2a57c75..574690afadb3 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -986,7 +986,8 @@ static int intel_backlight_device_update_status(struct
backlight_device *bd)
*/
if (panel->backlight.enabled) {
if (panel->backlight_power) {
- bool enable = bd->props.power == FB_BLANK_UNBLANK;
+ bool enable = bd->props.power == FB_BLANK_UNBLANK &&
+ bd->props.brightness != 0;
panel->backlight_power(connector, enable);
}
} else {
Didn't get a chance to test this on nightly.
Reviewed_by: Clinton Taylor
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/4] drm/i915: add some framework for backlight bl_power support
On 08/13/2014 02:10 AM, Jani Nikula wrote:
Make backlight class sysfs bl_power a sub-state of backlight enabled, if
a backlight power connector callback is defined. It's up to the
connector callback to handle the sub-state, typically in a way that
respects panel power sequencing.
v2: Post the version that does not oops. *facepalm*.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_panel.c | 26 ++
2 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1b3d1d7e466e..43b7b6609f0e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -173,6 +173,8 @@ struct intel_panel {
bool active_low_pwm;
struct backlight_device *device;
} backlight;
+
+ void (*backlight_power)(struct intel_connector *, bool enable);
};
struct intel_connector {
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 59b028f0b1e8..af5435634929 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -751,6 +751,8 @@ void intel_panel_disable_backlight(struct intel_connector
*connector)
spin_lock_irqsave(&dev_priv->backlight_lock, flags);
+ if (panel->backlight.device)
+ panel->backlight.device->props.power = FB_BLANK_POWERDOWN;
panel->backlight.enabled = false;
dev_priv->display.disable_backlight(connector);
@@ -957,6 +959,8 @@ void intel_panel_enable_backlight(struct intel_connector
*connector)
dev_priv->display.enable_backlight(connector);
panel->backlight.enabled = true;
+ if (panel->backlight.device)
+ panel->backlight.device->props.power = FB_BLANK_UNBLANK;
spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
}
@@ -965,6 +969,7 @@ void intel_panel_enable_backlight(struct intel_connector
*connector)
static int intel_backlight_device_update_status(struct backlight_device *bd)
{
struct intel_connector *connector = bl_get_data(bd);
+ struct intel_panel *panel = &connector->panel;
struct drm_device *dev = connector->base.dev;
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
@@ -972,6 +977,22 @@ static int intel_backlight_device_update_status(struct
backlight_device *bd)
bd->props.brightness, bd->props.max_brightness);
intel_panel_set_backlight(connector, bd->props.brightness,
bd->props.max_brightness);
+
+ /*
+* Allow flipping bl_power as a sub-state of enabled. Sadly the
+* backlight class device does not make it easy to to differentiate
+* between callbacks for brightness and bl_power, so our backlight_power
+* callback needs to take this into account.
+*/
+ if (panel->backlight.enabled) {
+ if (panel->backlight_power) {
+ bool enable = bd->props.power == FB_BLANK_UNBLANK;
+ panel->backlight_power(connector, enable);
+ }
+ } else {
+ bd->props.power = FB_BLANK_POWERDOWN;
+ }
+
drm_modeset_unlock(&dev->mode_config.connection_mutex);
return 0;
}
@@ -1023,6 +1044,11 @@ static int intel_backlight_device_register(struct
intel_connector *connector)
panel->backlight.level,
props.max_brightness);
+ if (panel->backlight.enabled)
+ props.power = FB_BLANK_UNBLANK;
+ else
+ props.power = FB_BLANK_POWERDOWN;
+
/*
* Note: using the same name independent of the connector prevents
* registration of multiple backlight devices in the driver.
Reviewed_by: Clinton Taylor
Tested_by: Clinton Taylor
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915/dp: make backlight bl_power control power sequencer backlight
On 08/12/2014 07:11 AM, Jani Nikula wrote:
This lets the userspace switch off the backlight using the backlight
class sysfs bl_power file. The switch is done using the power sequencer;
the backlight PWM, and everything else, remains enabled. The display
backlight won't draw power, but for maximum power savings the encoder
needs to be switched off.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dp.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d8baf60ff3fd..f6e3e9a906b0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1453,6 +1453,27 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
intel_panel_disable_backlight(intel_dp->attached_connector);
}
+/*
+ * Hook for controlling the panel power control backlight through the bl_power
+ * sysfs attribute. Take care to handle multiple calls.
+ */
+static void intel_edp_backlight_power(struct intel_connector *connector,
+ bool enable)
+{
+ struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
+ bool is_enabled = ironlake_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
+
+ if (is_enabled == enable)
+ return;
+
+ DRM_DEBUG_KMS("\n");
+
+ if (enable)
+ _intel_edp_backlight_on(intel_dp);
+ else
+ _intel_edp_backlight_off(intel_dp);
+}
+
static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -4579,6 +4600,7 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
}
intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+ intel_connector->panel.backlight_power = intel_edp_backlight_power;
intel_panel_setup_backlight(connector);
return true;
Reviewed_by: Clinton Taylor
Tested_by: Clinton Taylor
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915/dp: split up panel power control from backlight pwm control
On 08/12/2014 07:11 AM, Jani Nikula wrote:
Make it possible to change panel power control backlight state without
touching the PWM. No functional changes.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_dp.c | 39 ++-
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e5ada4fbe945..d8baf60ff3fd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1384,7 +1384,8 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
intel_display_power_put(dev_priv, power_domain);
}
-void intel_edp_backlight_on(struct intel_dp *intel_dp)
+/* Enable backlight in the panel power control. */
+static void _intel_edp_backlight_on(struct intel_dp *intel_dp)
{
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -1392,13 +1393,6 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp)
u32 pp;
u32 pp_ctrl_reg;
- if (!is_edp(intel_dp))
- return;
-
- DRM_DEBUG_KMS("\n");
-
- intel_panel_enable_backlight(intel_dp->attached_connector);
-
/*
* If we enable the backlight right away following a panel power
* on, we may see slight flicker as the panel syncs with the eDP
@@ -1415,17 +1409,26 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp)
POSTING_READ(pp_ctrl_reg);
}
-void intel_edp_backlight_off(struct intel_dp *intel_dp)
+/* Enable backlight PWM and backlight PP control. */
+void intel_edp_backlight_on(struct intel_dp *intel_dp)
+{
+ if (!is_edp(intel_dp))
+ return;
+
+ DRM_DEBUG_KMS("\n");
+
+ intel_panel_enable_backlight(intel_dp->attached_connector);
+ _intel_edp_backlight_on(intel_dp);
+}
+
+/* Disable backlight in the panel power control. */
+static void _intel_edp_backlight_off(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct drm_i915_private *dev_priv = dev->dev_private;
u32 pp;
u32 pp_ctrl_reg;
- if (!is_edp(intel_dp))
- return;
-
- DRM_DEBUG_KMS("\n");
pp = ironlake_get_pp_control(intel_dp);
pp &= ~EDP_BLC_ENABLE;
@@ -1436,7 +1439,17 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
intel_dp->last_backlight_off = jiffies;
edp_wait_backlight_off(intel_dp);
+}
+
+/* Disable backlight PP control and backlight PWM. */
+void intel_edp_backlight_off(struct intel_dp *intel_dp)
+{
+ if (!is_edp(intel_dp))
+ return;
+
+ DRM_DEBUG_KMS("\n");
+ _intel_edp_backlight_off(intel_dp);
intel_panel_disable_backlight(intel_dp->attached_connector);
}
Reviewed_by: Clinton Taylor
Tested_by: Clinton Taylor
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add 180 degree primary plane rotation support
On Tue, Aug 19, 2014 at 11:56:43AM +0530, sonika.jin...@intel.com wrote:
> From: Sonika Jindal
>
> Primary planes support 180 degree rotation. Expose the feature
> through rotation drm property.
>
> v2: Calculating linear/tiled offsets based on pipe source width and
> height. Added 180 degree rotation support in ironlake_update_plane.
>
> v3: Checking if CRTC is active before issueing update_plane. Added
> wait for vblank to make sure we dont overtake page flips. Disabling
> FBC since it does not work with rotated planes.
>
> v4: Updated rotation checks for pending flips, fbc disable. Creating
> rotation property only for Gen4 onwards. Property resetting as part
> of lastclose.
>
> v5: Resetting property in i915_driver_lastclose properly for planes
> and crtcs. Fixed linear offset calculation that was off by 1 w.r.t
> width in i9xx_update_plane and ironlake_update_plane. Removed tab
> based indentation and unnecessary braces in intel_crtc_set_property
> and intel_update_fbc. FBC and flip related checks should be done only
> for valid crtcs.
>
> v6: Minor nits in FBC disable checks for comments in intel_crtc_set_property
> and positioning the disable code in intel_update_fbc.
>
> v7: In case rotation property on inactive crtc is updated, we return
> successfully printing debug log as crtc is inactive and only property change
> is preserved.
>
> v8: update_plane is changed to update_primary_plane, crtc->fb is changed to
> crtc->primary->fb and return value of update_primary_plane is ignored.
>
> v9: added rotation property to primary plane instead of crtc. Removing reset
> of rotation property from lastclose. rotation_property is moved to
> drm_mode_config, so drm layer will take care of resetting. Adding updation of
> fbc when rotation is set to 0. Allowing rotation only if value is
> different than old one.
>
> v10: Calling intel_primary_plane_setplane instead of update_primary_plane in
> set_property(Daniel).
>
> Testcase: igt/kms_rotation_crc
>
> Signed-off-by: Uma Shankar
> Signed-off-by: Sagar Kamble
> Signed-off-by: Sonika Jindal
> ---
> drivers/gpu/drm/i915/i915_reg.h |1 +
> drivers/gpu/drm/i915/intel_display.c | 116
> --
> drivers/gpu/drm/i915/intel_pm.c |6 ++
> 3 files changed, 119 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7a6cc69..f8aaf0b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4209,6 +4209,7 @@ enum punit_power_well {
> #define DISPPLANE_NO_LINE_DOUBLE 0
> #define DISPPLANE_STEREO_POLARITY_FIRST0
> #define DISPPLANE_STEREO_POLARITY_SECOND (1<<18)
> +#define DISPPLANE_ROTATE_180 (1<<15)
> #define DISPPLANE_TRICKLE_FEED_DISABLE (1<<14) /* Ironlake */
> #define DISPPLANE_TILED(1<<10)
> #define _DSPAADDR0x70184
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 1b0e403..cc2999b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2332,6 +2332,9 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
> unsigned long linear_offset;
> u32 dspcntr;
> u32 reg = DSPCNTR(plane);
> + int pixel_size;
> +
> + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>
> if (!intel_crtc->primary_enabled) {
> I915_WRITE(reg, 0);
> @@ -2359,6 +2362,7 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
> (intel_crtc->config.pipe_src_w - 1));
> I915_WRITE(DSPPOS(plane), 0);
> }
> + dspcntr &= ~DISPPLANE_ROTATE_180;
>
> switch (fb->pixel_format) {
> case DRM_FORMAT_C8:
> @@ -2398,8 +2402,6 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
> if (IS_G4X(dev))
> dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>
> - I915_WRITE(reg, dspcntr);
> -
> linear_offset = y * fb->pitches[0] + x * (fb->bits_per_pixel / 8);
>
> if (INTEL_INFO(dev)->gen >= 4) {
> @@ -2412,6 +2414,21 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
> intel_crtc->dspaddr_offset = linear_offset;
> }
>
> + if (to_intel_plane(crtc->primary)->rotation == BIT(DRM_ROTATE_180)) {
> + dspcntr |= DISPPLANE_ROTATE_180;
> +
> + x += (intel_crtc->config.pipe_src_w - 1);
> + y += (intel_crtc->config.pipe_src_h - 1);
> +
> + /* Finding the last pixel of the last line of the display
> + data and adding to linear_offset*/
> + linear_offset += (intel_crtc->config.pipe_src_h - 1) *
> + fb->pitches[0] +
> + (intel_crtc->config.pipe_src_w - 1) * pixel_size;
> + }
> +
> + I915_WRITE(reg, dspcntr);
> +
> DRM_DEBUG_KMS("Writing b
Re: [Intel-gfx] How to create PCH to support those existing driver
> -Original Message- > From: Michael S. Tsirkin [mailto:m...@redhat.com] > Sent: Tuesday, August 19, 2014 2:51 PM > To: Kay, Allen M > Cc: Chen, Tiejun; Paolo Bonzini; Wang, Yong Y; Don Dutile; Jesse Barnes; > Konrad Rzeszutek Wilk; qemu-de...@nongnu.org; xen- > de...@lists.xensource.com; intel-gfx; Stefano Stabellini > (stefano.stabell...@citrix.com) > Subject: Re: How to create PCH to support those existing driver > > On Tue, Aug 19, 2014 at 09:24:03PM +, Kay, Allen M wrote: > > > Allen, > > > > > > Could you reply this? > > > > Let me summarized what we have discussed and learned so far: > > > > 1) Future Windows/Linux IGD drivers will be modified to restrain from > accessing MCH/PCH devices. We are prototyping this in Windows driver right > now and will pass the same methodology to Linux driver once we have a > workable solution. The goal is removing all MCH/PCH accesses in the IGD > driver. > > > > 2) We want the same solution to work in both native and virtualization > environments. Given most driver developers test their changes only in > native environment, doing anything specific for virtualization in the driver > will > cause frequent breakage for virtualization use cases. > > > > 3) Back porting this new code to support previous generations of HW will > be problematic if not impossible. Each Windows IGD driver release binary > supports two generations of HW. For example, 15.36 driver supports > Broadwell/Haswell, 15.33 driver supports Haswell/IvyBridge, 15.31 driver > supports Ivybridge/Sandybridge, etc. Once the driver is product validated, > there is little opportunity to go back and make high impact changes that > might affect stability in native environment. > > > > 4) I agree there is little reason to do anything that requires driver > > changes > at this point, unless it is the final desired solution. > > > > The question is whether/how to support IGD passthrough for previous > generations of HW? > > > > 1) If we want to support SandyBridge/IvyBridge/Haswell/Broadwell, we will > need most of the original QEMU patches. We might be able to do without > igd_pci_write(). I have tested QEMU changes without igd_pci_write() on > both Haswell/Broadwell and Windows booted without any problems. This > will limit only read operations which should reduce a quite a bit of risk to > the > host platform. > > Excellent. I was thinking about changing host's driver to do the writes in a > safe manner, but if don't need that, all the better. > As a next step, we need to limit read operations to specific set of registers > that we can validate. > We can't just pass through reads blindly to host, pci reads have side-effects > and host chipset isn't protected by the iommu. > Since these are legacy devices and drivers, it should be possible to > enumerate all registers that they need. > If we limit platform support to HSW/BDW the number of register reads is quite small. I believe some of the register reads are for the old Ironlake platforms. I will work with Tiejun to get the smaller set for HSW/BDW systems. > > > 2) If we want the upstream QEMU only work with future driver version, > then most of the IGD passthrough patch is probably not needed - with > exception of opregion mapping handlers. The downside is products that > depend on this feature will need to apply private patches separately to re- > enable IGD passthrough capability. > > > > Any advice on how should Tiejun proceed from here? > > > > Allen > > I'm fine with either trying to make existing windows and linux drivers work, > or waiting for future drivers. > > To me, quick hacks that need minor changes in driver but don't avoid poking > at MCH/PCH don't seem to have value, I know I proposed some of these > myself but this was before I realised a cleaner solution is possible. > > > -- > MST Allen ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] How to create PCH to support those existing driver
On Tue, Aug 19, 2014 at 09:24:03PM +, Kay, Allen M wrote: > > Allen, > > > > Could you reply this? > > Let me summarized what we have discussed and learned so far: > > 1) Future Windows/Linux IGD drivers will be modified to restrain from > accessing MCH/PCH devices. We are prototyping this in Windows driver right > now and will pass the same methodology to Linux driver once we have a > workable solution. The goal is removing all MCH/PCH accesses in the IGD > driver. > > 2) We want the same solution to work in both native and virtualization > environments. Given most driver developers test their changes only in native > environment, doing anything specific for virtualization in the driver will > cause frequent breakage for virtualization use cases. > > 3) Back porting this new code to support previous generations of HW will be > problematic if not impossible. Each Windows IGD driver release binary > supports two generations of HW. For example, 15.36 driver supports > Broadwell/Haswell, 15.33 driver supports Haswell/IvyBridge, 15.31 driver > supports Ivybridge/Sandybridge, etc. Once the driver is product validated, > there is little opportunity to go back and make high impact changes that > might affect stability in native environment. > > 4) I agree there is little reason to do anything that requires driver changes > at this point, unless it is the final desired solution. > > The question is whether/how to support IGD passthrough for previous > generations of HW? > > 1) If we want to support SandyBridge/IvyBridge/Haswell/Broadwell, we will > need most of the original QEMU patches. We might be able to do without > igd_pci_write(). I have tested QEMU changes without igd_pci_write() on both > Haswell/Broadwell and Windows booted without any problems. This will limit > only read operations which should reduce a quite a bit of risk to the host > platform. Excellent. I was thinking about changing host's driver to do the writes in a safe manner, but if don't need that, all the better. As a next step, we need to limit read operations to specific set of registers that we can validate. We can't just pass through reads blindly to host, pci reads have side-effects and host chipset isn't protected by the iommu. Since these are legacy devices and drivers, it should be possible to enumerate all registers that they need. > 2) If we want the upstream QEMU only work with future driver version, then > most of the IGD passthrough patch is probably not needed - with exception of > opregion mapping handlers. The downside is products that depend on this > feature will need to apply private patches separately to re-enable IGD > passthrough capability. > > Any advice on how should Tiejun proceed from here? > > Allen I'm fine with either trying to make existing windows and linux drivers work, or waiting for future drivers. To me, quick hacks that need minor changes in driver but don't avoid poking at MCH/PCH don't seem to have value, I know I proposed some of these myself but this was before I realised a cleaner solution is possible. -- MST ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Responsiveness Changes to i915 Driver
Greetings - after reviewing Chris¹s feedback below and some thought, I
most likely do not need to add another trace message and the existing
³i915_flip_complete² trace message can be used.
Thus the only change requested is to have the T1_T3 value printed out
during driver init/re-init. Here is the requested change:
diff -rupN src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_dp.c
src/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_dp.c
---
src.org/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_dp.c 2014-08-14
14:24:45.655312785 -0700
+++ src/third_party/kernel/3.10/drivers/gpu/drm/i915/intel_dp.c 2014-08-14
11:57:30.203608374 -0700
@@ -3528,6 +3528,7 @@ intel_dp_init_panel_power_sequencer(stru
intel_dp->panel_power_cycle_delay = get_delay(t11_t12);
#undef get_delay
+ printk(KERN_INFO "i915: eDP T3 Value: %d\n",
intel_dp->panel_power_up_delay);
DRM_DEBUG_KMS("panel power up delay %d, power down delay %d, power cycle
delay %d\n",
intel_dp->panel_power_up_delay,
intel_dp->panel_power_down_delay,
intel_dp->panel_power_cycle_delay);
Regards,
-martin
On 8/14/14, 11:26 PM, "Chris Wilson" wrote:
>On Thu, Aug 14, 2014 at 11:52:47PM +, Wilde, Martin wrote:
>> Greetings,
>>
>> I am submitting the below changes to i915 Gfx driver to support resume
>>time Responsiveness measurements. These changes parallel the work
>>already done in the IVB Windows Gfx driver. These changes in addition
>>to other OTS scripts (suspend_resume) allow tracking of what is referred
>>to as the ³B2I² or ³Button To Image² time of the platform. The shorter
>>this time, the more responsiveness the platform is viewed by the end
>>user. Panel selection is an important factor in providing a more
>>responsive system. Note there is no dependency on other scripts. The
>>changes are standalone.
>>
>>
>> * Display the current T1_T3 value. This is used to verify that the
>>timing set in the VBT is correct. We have seen many instances where the
>>value is not set correctly for the panel and the resume time is longer
>>than necessary (e.g. 500ms T3 versus 200ms T3).
>> * Print the time when the first page flip occurs. This is when the
>>user first sees the desktop displayed from resume. While this
>>measurement could be done by other methods, this is the actual time that
>>the desktop manager/framebuffer makes the driver request and the Gfx
>>driver performs the action. Thus any layering software added can be
>>correlated to increases in this time.
>>
>> To support the latter (first page flip), I added a new ftrace called
>>³trace_i915_resume². I looked at the existing page flip trace message
>>and that one is designed for every page flip. I did not want to
>>convolute it with the one time flip trace on resume. I used a trace
>>message instead of a printk to reduce any performance impacts of using a
>>printk. Additionally printk is not reliable of when the message
>>actually appears in the kernel log.
>
>I am sorry, you are complaining that there is a tracepoint that gives
>you exactly what you want already, only that userspace needs to do some
>filtering? Which by the way, does not give you what you say you want
>anyway - the scanout is already active long before the first flip is
>handled, and in many, many cass that flip is just a figment of your
>imagination. Maybe what you mean is to B2UR rather than
>button-to-static-image.
>-Chris
>
>
>--
>Chris Wilson, Intel Open Source Technology Centre
i915.patch
Description: i915.patch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/edid: Reduce horizontal timings for pixel replicated modes
From: Clint Taylor
Pixel replicated modes should be 720 horizontal pixel and pixel
replicated by the HW across the HDMI cable at 2X pixel clock. Current
horizontal resolution of 1440 does not allow pixel duplication to
occur and scaling artifacts occur on the TV. HDMI certification
7-26 currently fails for all pixel replicated modes. This change fizes
the HDMI certification issues with 480i/576i.
V2: Removed interlace flag from VICs 44 and 45. Will be submitted in
another patch. Various other formatting fixes.
V3: 576i@200 htotal fixed. Check min and max pixel clocks.
Signed-off-by: Clint Taylor
---
drivers/gpu/drm/drm_edid.c| 96 ++---
drivers/gpu/drm/i915/intel_hdmi.c | 15 --
2 files changed, 60 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f905c63..dc25999 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -632,27 +632,27 @@ static const struct drm_display_mode edid_cea_modes[] = {
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC |
DRM_MODE_FLAG_INTERLACE),
.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
- /* 6 - 1440x480i@60Hz */
- { DRM_MODE("1440x480i", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478,
- 1602, 1716, 0, 480, 488, 494, 525, 0,
+ /* 6 - 720(1440)x480i@60Hz */
+ { DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 13500, 720, 739,
+ 801, 858, 0, 480, 488, 494, 525, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK),
.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, },
- /* 7 - 1440x480i@60Hz */
- { DRM_MODE("1440x480i", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478,
- 1602, 1716, 0, 480, 488, 494, 525, 0,
+ /* 7 - 720(1440)x480i@60Hz */
+ { DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 13500, 720, 739,
+ 801, 858, 0, 480, 488, 494, 525, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK),
.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
- /* 8 - 1440x240@60Hz */
- { DRM_MODE("1440x240", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478,
- 1602, 1716, 0, 240, 244, 247, 262, 0,
+ /* 8 - 720(1440)x240@60Hz */
+ { DRM_MODE("720x240", DRM_MODE_TYPE_DRIVER, 13500, 720, 739,
+ 801, 858, 0, 240, 244, 247, 262, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
DRM_MODE_FLAG_DBLCLK),
.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, },
- /* 9 - 1440x240@60Hz */
- { DRM_MODE("1440x240", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478,
- 1602, 1716, 0, 240, 244, 247, 262, 0,
+ /* 9 - 720(1440)x240@60Hz */
+ { DRM_MODE("720x240", DRM_MODE_TYPE_DRIVER, 13500, 720, 739,
+ 801, 858, 0, 240, 244, 247, 262, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
DRM_MODE_FLAG_DBLCLK),
.vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
@@ -714,27 +714,27 @@ static const struct drm_display_mode edid_cea_modes[] = {
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC |
DRM_MODE_FLAG_INTERLACE),
.vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
- /* 21 - 1440x576i@50Hz */
- { DRM_MODE("1440x576i", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1464,
- 1590, 1728, 0, 576, 580, 586, 625, 0,
+ /* 21 - 720(1440)x576i@50Hz */
+ { DRM_MODE("720x576i", DRM_MODE_TYPE_DRIVER, 13500, 720, 732,
+ 795, 864, 0, 576, 580, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK),
.vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, },
- /* 22 - 1440x576i@50Hz */
- { DRM_MODE("1440x576i", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1464,
- 1590, 1728, 0, 576, 580, 586, 625, 0,
+ /* 22 - 720(1440)x576i@50Hz */
+ { DRM_MODE("720x576i", DRM_MODE_TYPE_DRIVER, 13500, 720, 732,
+ 795, 864, 0, 576, 580, 586, 625, 0,
DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC |
DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK),
.vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
- /* 23 - 1440x288@50Hz */
- { DRM_MODE("1440x288", DRM_MODE_TYPE_DRIVER, 27000, 1440, 1464,
- 1590, 1728, 0, 288, 290, 293, 312, 0,
+ /* 23 - 720(1440)x288@50Hz */
+ { DRM_MODE("720x288", DRM_MODE_TYPE_DRIVER, 13500, 720, 732,
+ 795,
Re: [Intel-gfx] How to create PCH to support those existing driver
> Allen,
>
> Could you reply this?
Let me summarized what we have discussed and learned so far:
1) Future Windows/Linux IGD drivers will be modified to restrain from accessing
MCH/PCH devices. We are prototyping this in Windows driver right now and will
pass the same methodology to Linux driver once we have a workable solution.
The goal is removing all MCH/PCH accesses in the IGD driver.
2) We want the same solution to work in both native and virtualization
environments. Given most driver developers test their changes only in native
environment, doing anything specific for virtualization in the driver will
cause frequent breakage for virtualization use cases.
3) Back porting this new code to support previous generations of HW will be
problematic if not impossible. Each Windows IGD driver release binary supports
two generations of HW. For example, 15.36 driver supports Broadwell/Haswell,
15.33 driver supports Haswell/IvyBridge, 15.31 driver supports
Ivybridge/Sandybridge, etc. Once the driver is product validated, there is
little opportunity to go back and make high impact changes that might affect
stability in native environment.
4) I agree there is little reason to do anything that requires driver changes
at this point, unless it is the final desired solution.
The question is whether/how to support IGD passthrough for previous generations
of HW?
1) If we want to support SandyBridge/IvyBridge/Haswell/Broadwell, we will need
most of the original QEMU patches. We might be able to do without
igd_pci_write(). I have tested QEMU changes without igd_pci_write() on both
Haswell/Broadwell and Windows booted without any problems. This will limit
only read operations which should reduce a quite a bit of risk to the host
platform.
2) If we want the upstream QEMU only work with future driver version, then most
of the IGD passthrough patch is probably not needed - with exception of
opregion mapping handlers. The downside is products that depend on this
feature will need to apply private patches separately to re-enable IGD
passthrough capability.
Any advice on how should Tiejun proceed from here?
Allen
> -Original Message-
> From: Chen, Tiejun
> Sent: Monday, August 18, 2014 6:39 PM
> To: Michael S. Tsirkin
> Cc: Paolo Bonzini; Kay, Allen M; Wang, Yong Y; Don Dutile; Jesse Barnes;
> Konrad Rzeszutek Wilk; qemu-de...@nongnu.org; xen-
> de...@lists.xensource.com; intel-gfx
> Subject: Re: How to create PCH to support those existing driver
>
>
>
> On 2014/8/18 17:58, Michael S. Tsirkin wrote:
> > On Mon, Aug 18, 2014 at 05:01:25PM +0800, Chen, Tiejun wrote:
> >> On 2014/8/18 16:21, Michael S. Tsirkin wrote:
> >>> On Mon, Aug 18, 2014 at 11:06:29AM +0800, Chen, Tiejun wrote:
> On 2014/8/17 18:32, Michael S. Tsirkin wrote:
> > On Fri, Aug 15, 2014 at 09:58:40AM +0800, Chen, Tiejun wrote:
> >> Michael and Paolo,
> >
> > Please re-post discussion on list. These off list ones are just
> > wasting time since they invariably have to be repeated on list again.
>
> Okay, now just reissue this discussion to all related guys. And do
> you think we need to discuss in public, qemu and xen mail list?
> >>>
> >>> Absolutely.
> >>
> >> Now -CC qemu, xen and intel-gfx.
> >>
> >> If I'm missing someone important please tell me as well.
> >>
> >>>
> >
> >> After I created that new machine specific to IGD passthrough,
> >> xenigd, now I will step next to register the PCH.
> >>
> >> IIRC, our complete solution should be as follows:
> >>
> >> #1 create a new machine based on piix, xenigd
> >>
> >> This is done with Michael help.
> >>
> >> #2 register ISA bridge
> >>
> >> 1> Its still fixed at 1f.0.
> >> 2> ISA bridge's vendor_id/device_id should be emulated but then
> >>
> >>subsystem_vendor_id = PCI_VENDOR_ID_XEN;
> >>subsystem_device_id = ISA Bridge's real device id
> >>
> >> This mean we need to change driver to walk with this way.
> >> For example, in
> >> case of Linux native driver,
> >>
> >> intel_detect_pch()
> >> {
> >>...
> >>if (pch->subsystem_vendor == PCI_VENDOR_ID_XEN)
> >>id = pch->subsystem_device &
> INTEL_PCH_DEVICE_ID_MASK;
> >>
> >> Then driver can get that real device id by 'subsystem_device', right?
> >>
> >> This is fine now but how to support those existing drivers which
> >> are just
>
> Here correct one point, we don't need to care about supporting the
> legacy driver since the legacy driver still should work
> qemu-traditional. So we just make sure the existing driver with
> this subsystem_id way can support those existing and legacy platform.
>
> Now this is clear to me.
>
> >> dependent on checking real vendor_id/device_id directly,
> >>
> >>if (pch->vendor == PCI_VENDOR_ID_INTEL) {
> >>>
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Updating plane parameters for primary plane in setplane
On Tue, Aug 19, 2014 at 11:56:42AM +0530, sonika.jin...@intel.com wrote:
> From: Sonika Jindal
>
> Signed-off-by: Sonika Jindal
> ---
> drivers/gpu/drm/i915/intel_display.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e9b578e..1b0e403 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11528,6 +11528,21 @@ intel_primary_plane_setplane(struct drm_plane
> *plane, struct drm_crtc *crtc,
> .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
> .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
> };
> + const struct {
> + int crtc_x, crtc_y;
> + unsigned int crtc_w, crtc_h;
> + uint32_t src_x, src_y, src_w, src_h;
> + } orig = {
> + .crtc_x = crtc_x,
> + .crtc_y = crtc_y,
> + .crtc_w = crtc_w,
> + .crtc_h = crtc_h,
> + .src_x = src_x,
> + .src_y = src_y,
> + .src_w = src_w,
> + .src_h = src_h,
> + };
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> bool visible;
> int ret;
>
> @@ -11604,6 +11619,15 @@ intel_primary_plane_setplane(struct drm_plane
> *plane, struct drm_crtc *crtc,
>
> return 0;
> }
> + intel_plane->crtc_x = orig.crtc_x;
> + intel_plane->crtc_y = orig.crtc_y;
> + intel_plane->crtc_w = orig.crtc_w;
> + intel_plane->crtc_h = orig.crtc_h;
> + intel_plane->src_x = orig.src_x;
> + intel_plane->src_y = orig.src_y;
> + intel_plane->src_w = orig.src_w;
> + intel_plane->src_h = orig.src_h;
> + intel_plane->obj = obj;
>
> ret = intel_pipe_set_base(crtc, src.x1, src.y1, fb);
> if (ret)
I haven't been following all of this thread carefully, but wouldn't you
want to update the intel_plane fields after the point where the function
can no longer fail? E.g., if I try to setplane() to a new location
while I have a pageflip pending, my request will fail and the plane
position won't update, yet you're still going to be updating the
internal state tracking variables here. Then if you do an
intel_plane_restore() later you're going to see the plane jump
unexpectedly.
On the other hand, what about the case where the setplane succeeds, but
the plane isn't visible (either because it's fully clipped or because
your crtc isn't active right now). Those are other paths through the
intel_primary_plane_setplane() function that don't seem to be updating
the intel_plane fields, even though it seems like you'd want to so that
your plane doesn't snap back to an old position on a later
intel_plane_restore().
Sorry if I'm overlooking something obvious here; I haven't been keeping
up with this whole email thread.
Matt
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Graphics driver Init code
On Tue, Aug 19, 2014 at 11:29 AM, Dushyant Behl wrote: > Ping. > > > On Thu, Aug 14, 2014 at 2:55 AM, Dushyant Behl > wrote: >> >> Hi Everyone, >> >> I am an Operating system developer and I'm working on a project for >> which I wanted to understand the working of Intel Graphics Driver. >> Being somewhat new in this field, Can I ask anyone to point me where >> should I start digging in the code? I mean where the graphic driver >> initializes contact with the GPU. >> >> I'm extremely sorry if I broke any mailing list etiquette. Please forgive >> me. >> >> Thanks in Advance, >> Dushyant Behl > > > I would be really thankful if someone can help me. All of the code exists in drivers/gpu/drm/i915 i915_drv.c contains i915_init, which is the first thing that gets called when the module is loaded. >From there, there's another 67k lines of code that should keep you busy. :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: FBC flush nuke for BDW
Readding intel-gfx. Please don't drop mailing lists cc's without telling me.
Thanks, Daniel
On Tue, Aug 19, 2014 at 8:57 PM, Daniel Vetter wrote:
> Yeah, that does a lot too much flushing - you need to track relevant
> dirty bits like psr does, and then only flush when there has been a
> preceeding invalidate with the primary plane frontbuffer bit for the
> pipe that's using fbc. On top of that there's room for more
> improvements (filtering out pageflips and optimizing that more, atm we
> just disable fbc over a pageflip which is a bit meh), and we should
> also be able to ditch all the existing fbc nuking we do from the cmd
> streamer.
> -Daniel
>
> On Tue, Aug 19, 2014 at 12:09 AM, Rodrigo Vivi wrote:
>> http://cgit.freedesktop.org/~vivijim/drm-intel/commit/?h=fbc-sw-nuke-hsw&id=71875d3331aa3baef4f6f6bd297cc70dd94df1b6
>>
>>
>> On Fri, Aug 8, 2014 at 12:06 AM, Daniel Vetter wrote:
>>>
>>> On Thu, Aug 07, 2014 at 01:04:19PM -0700, Rodrigo Vivi wrote:
>>> > I tested here on HSW a full sw nuke/cache clean and I didn't liked the
>>> > result.
>>> > It seems to compress less than the hw one and to recompress everything a
>>> > lot and stay less time compressed.
>>>
>>> That is really unexpected. For a modern desktop (i.e. anything that
>>> pageflips) there should be zero difference. And for actual frontbuffer
>>> rendering there should only be a difference when doing tiny cpu rendering
>>> to the frontbuffer.
>>>
>>> So something didn't work out as expected. Can you please push the code
>>> somewhere, or just submit patches to intel-gfx?
>>>
>>> Thanks, Daniel
>>> >
>>> > So, imho v3 is the way to go.
>>> >
>>> >
>>> > On Mon, Aug 4, 2014 at 3:51 AM, Rodrigo Vivi
>>> > wrote:
>>> >
>>> > > According to spec FBC on BDW and HSW are identical without any gaps.
>>> > > So let's copy the nuke and let FBC really start compressing stuff.
>>> > >
>>> > > Without this patch we can verify with false color that nothing is
>>> > > being
>>> > > compressed. With the nuke in place and false color it is possible
>>> > > to see false color debugs.
>>> > >
>>> > > Unfortunatelly on some rings like BCS on BDW we have to avoid Bits
>>> > > 22:18 on
>>> > > LRIs due to a high risk of hung. So, when using Blt ring for
>>> > > frontbuffer
>>> > > rend
>>> > > cache would never been cleaned and FBC would stop compressing buffer.
>>> > > One alternative is to cache clean on software frontbuffer tracking.
>>> > >
>>> > > v2: Fix rebase conflict.
>>> > > v3: Do not clean cache on BCS ring. Instead use sw frontbuffer
>>> > > tracking.
>>> > >
>>> > > Signed-off-by: Rodrigo Vivi
>>> > > ---
>>> > > drivers/gpu/drm/i915/i915_drv.h | 1 +
>>> > > drivers/gpu/drm/i915/intel_display.c| 3 +++
>>> > > drivers/gpu/drm/i915/intel_pm.c | 10 ++
>>> > > drivers/gpu/drm/i915/intel_ringbuffer.c | 10 +-
>>> > > 4 files changed, 23 insertions(+), 1 deletion(-)
>>> > >
>>> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>> > > b/drivers/gpu/drm/i915/i915_drv.h
>>> > > index 2a372f2..25d7365 100644
>>> > > --- a/drivers/gpu/drm/i915/i915_drv.h
>>> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> > > @@ -2713,6 +2713,7 @@ extern void intel_modeset_setup_hw_state(struct
>>> > > drm_device *dev,
>>> > > extern void i915_redisable_vga(struct drm_device *dev);
>>> > > extern void i915_redisable_vga_power_on(struct drm_device *dev);
>>> > > extern bool intel_fbc_enabled(struct drm_device *dev);
>>> > > +extern void gen8_fbc_sw_flush(struct drm_device *dev, u32 value);
>>> > > extern void intel_disable_fbc(struct drm_device *dev);
>>> > > extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
>>> > > extern void intel_init_pch_refclk(struct drm_device *dev);
>>> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
>>> > > b/drivers/gpu/drm/i915/intel_display.c
>>> > > index 883af0b..c8421cd 100644
>>> > > --- a/drivers/gpu/drm/i915/intel_display.c
>>> > > +++ b/drivers/gpu/drm/i915/intel_display.c
>>> > > @@ -9044,6 +9044,9 @@ void intel_frontbuffer_flush(struct drm_device
>>> > > *dev,
>>> > > intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
>>> > >
>>> > > intel_edp_psr_flush(dev, frontbuffer_bits);
>>> > > +
>>> > > + if (IS_GEN8(dev))
>>> > > + gen8_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
>>> > > }
>>> > >
>>> > > /**
>>> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
>>> > > b/drivers/gpu/drm/i915/intel_pm.c
>>> > > index 684dc5f..de07d3e 100644
>>> > > --- a/drivers/gpu/drm/i915/intel_pm.c
>>> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> > > @@ -345,6 +345,16 @@ bool intel_fbc_enabled(struct drm_device *dev)
>>> > > return dev_priv->display.fbc_enabled(dev);
>>> > > }
>>> > >
>>> > > +void gen8_fbc_sw_flush(struct drm_device *dev, u32 value)
>>> > > +{
>>> > > + struct drm_i915_private *dev_priv = dev->dev_private;
>>> > > +
>>> > > + if (!IS_GEN8(dev))
>>> > > + return;
>>> > > +
>>> > > + I915_WRITE(MSG_FBC_
Re: [Intel-gfx] Graphics driver Init code
Ping. On Thu, Aug 14, 2014 at 2:55 AM, Dushyant Behl wrote: > Hi Everyone, > > I am an Operating system developer and I'm working on a project for > which I wanted to understand the working of Intel Graphics Driver. > Being somewhat new in this field, Can I ask anyone to point me where > should I start digging in the code? I mean where the graphic driver > initializes contact with the GPU. > > I'm extremely sorry if I broke any mailing list etiquette. Please forgive > me. > > Thanks in Advance, > Dushyant Behl > I would be really thankful if someone can help me. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 15/14] drm/i915: Add comments explaining the vdd on/off functions
From: Ville Syrjälä
Jani wanted some comments to explain why we call certain vdd on/off
functions in certain places.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1067082..1233a10 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1315,6 +1315,7 @@ static u32 ironlake_get_pp_control(struct intel_dp
*intel_dp)
return control;
}
+/* should be paired with edp_panel_vdd_off() */
static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -1365,6 +1366,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
return need_to_disable;
}
+/* should be paired with intel_edp_panel_vdd_off() */
void intel_edp_panel_vdd_on(struct intel_dp *intel_dp)
{
struct drm_i915_private *dev_priv =
@@ -1447,6 +1449,7 @@ static void edp_panel_vdd_schedule_off(struct intel_dp
*intel_dp)
schedule_delayed_work(&intel_dp->panel_vdd_work, delay);
}
+/* should be paired with edp_panel_vdd_on() */
static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync)
{
struct drm_i915_private *dev_priv =
@@ -1467,6 +1470,7 @@ static void edp_panel_vdd_off(struct intel_dp *intel_dp,
bool sync)
edp_panel_vdd_schedule_off(intel_dp);
}
+/* should be paired with intel_edp_panel_vdd_on() */
static void intel_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync)
{
struct drm_i915_private *dev_priv =
@@ -4307,6 +4311,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
drm_encoder_cleanup(encoder);
if (is_edp(intel_dp)) {
cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
+ /*
+* vdd might still be enabled do to the delayed vdd off.
+* Make sure vdd is actually turned off here.
+*/
mutex_lock(&dev_priv->pps_mutex);
edp_panel_vdd_off_sync(intel_dp);
mutex_unlock(&dev_priv->pps_mutex);
@@ -4327,6 +4335,10 @@ static void intel_dp_encoder_suspend(struct
intel_encoder *intel_encoder)
if (!is_edp(intel_dp))
return;
+ /*
+* vdd might still be enabled do to the delayed vdd off.
+* Make sure vdd is actually turned off here.
+*/
mutex_lock(&dev_priv->pps_mutex);
edp_panel_vdd_off_sync(intel_dp);
mutex_unlock(&dev_priv->pps_mutex);
@@ -5025,6 +5037,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
drm_dp_aux_unregister(&intel_dp->aux);
if (is_edp(intel_dp)) {
cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
+ /*
+* vdd might still be enabled do to the delayed vdd off.
+* Make sure vdd is actually turned off here.
+*/
mutex_lock(&dev_priv->pps_mutex);
edp_panel_vdd_off_sync(intel_dp);
mutex_unlock(&dev_priv->pps_mutex);
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10.1/14] drm/i915: Reset power sequencer pipe tracking when disp2d is off
From: Ville Syrjälä
The power sequencer loses its state when the disp2d power well is down.
Clear the dev_priv->pps_pipe tracking so that the power sequencer state
gets reinitialized the next time it's needed.
Signed-off-by: Ville Syrjälä
---
Might be this should be squashed with
'[PATCH] drm/i915: Track which port is using which pipe's power sequencer'
to avoid having a regression in between. But I left if as a separate patch
for clarity.
Also maybe it would be cleaner to track this pipe power seqeuencer<->port
relationship the other way around? So something like this:
/* the current port for each power seqeuencer */
enum port pps_ports[2];
drivers/gpu/drm/i915/intel_dp.c | 23 +++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 2 ++
3 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d6fe1a2..3389c89 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -389,6 +389,29 @@ vlv_initial_power_sequencer_setup(struct intel_dp
*intel_dp)
&power_seq);
}
+void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
+{
+ struct drm_device *dev = dev_priv->dev;
+ struct intel_encoder *encoder;
+
+ if (WARN_ON(!IS_VALLEYVIEW(dev)))
+ return;
+
+ mutex_lock(&dev_priv->pps_mutex);
+
+ list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head)
{
+ struct intel_dp *intel_dp;
+
+ if (encoder->type != INTEL_OUTPUT_EDP)
+ continue;
+
+ intel_dp = enc_to_intel_dp(&encoder->base);
+ intel_dp->pps_pipe = INVALID_PIPE;
+ }
+
+ mutex_unlock(&dev_priv->pps_mutex);
+}
+
static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 849a447..32627ae 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -943,6 +943,7 @@ void intel_dp_mst_suspend(struct drm_device *dev);
void intel_dp_mst_resume(struct drm_device *dev);
int intel_dp_max_link_bw(struct intel_dp *intel_dp);
void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
+void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv);
/* intel_dp_mst.c */
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int
conn_id);
void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 39dd066..d5ffda9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6328,6 +6328,8 @@ static void vlv_display_power_well_disable(struct
drm_i915_private *dev_priv,
spin_unlock_irq(&dev_priv->irq_lock);
vlv_set_power_well(dev_priv, power_well, false);
+
+ vlv_power_sequencer_reset(dev_priv);
}
static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 09/14] drm/i915: Fix edp vdd locking
From: Ville Syrjälä
Introduce a new mutex (pps_mutex) to protect the power sequencer
state. For now this state includes want_panel_vdd as well as the
power sequencer registers.
We need a single mutex (as opposed to per port) because later on we
will need to deal with VLV/CHV which have multiple power sequencer
which can be reassigned to different ports.
v2: Add the locking to intel_dp_encoder_suspend too (Imre)
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/intel_display.c | 2 +
drivers/gpu/drm/i915/intel_dp.c | 99
3 files changed, 93 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6fbd316..c5faefb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1501,6 +1501,9 @@ struct drm_i915_private {
/* LVDS info */
bool no_aux_handshake;
+ /* protects panel power sequencer state */
+ struct mutex pps_mutex;
+
struct drm_i915_fence_reg fence_regs[I915_MAX_NUM_FENCES]; /* assume
965 */
int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */
int num_fence_regs; /* 8 on pre-965, 16 otherwise */
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0b327eb..ff86729 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12437,6 +12437,8 @@ static void intel_init_display(struct drm_device *dev)
}
intel_panel_init_backlight_funcs(dev);
+
+ mutex_init(&dev_priv->pps_mutex);
}
/*
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9efa6bf..446df28 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -300,6 +300,8 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
enum port port = intel_dig_port->port;
enum pipe pipe;
+ lockdep_assert_held(&dev_priv->pps_mutex);
+
/* modeset should have pipe */
if (crtc)
return to_intel_crtc(crtc)->pipe;
@@ -352,6 +354,8 @@ static int edp_notify_handler(struct notifier_block *this,
unsigned long code,
if (!IS_VALLEYVIEW(dev) || !is_edp(intel_dp) || code != SYS_RESTART)
return 0;
+ mutex_lock(&dev_priv->pps_mutex);
+
pipe = vlv_power_sequencer_pipe(intel_dp);
pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
@@ -364,6 +368,8 @@ static int edp_notify_handler(struct notifier_block *this,
unsigned long code,
I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
msleep(intel_dp->panel_power_cycle_delay);
+ mutex_unlock(&dev_priv->pps_mutex);
+
return 0;
}
@@ -372,6 +378,8 @@ static bool edp_have_panel_power(struct intel_dp *intel_dp)
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct drm_i915_private *dev_priv = dev->dev_private;
+ lockdep_assert_held(&dev_priv->pps_mutex);
+
return (I915_READ(_pp_stat_reg(intel_dp)) & PP_ON) != 0;
}
@@ -383,6 +391,8 @@ static bool edp_have_panel_vdd(struct intel_dp *intel_dp)
struct intel_encoder *intel_encoder = &intel_dig_port->base;
enum intel_display_power_domain power_domain;
+ lockdep_assert_held(&dev_priv->pps_mutex);
+
power_domain = intel_display_port_power_domain(intel_encoder);
return intel_display_power_enabled(dev_priv, power_domain) &&
(I915_READ(_pp_ctrl_reg(intel_dp)) & EDP_FORCE_VDD) != 0;
@@ -533,6 +543,8 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
bool has_aux_irq = HAS_AUX_IRQ(dev);
bool vdd;
+ mutex_lock(&dev_priv->pps_mutex);
+
/*
* We will be called with VDD already enabled for dpcd/edid/oui reads.
* In such cases we want to leave VDD enabled and it's up to upper
layers
@@ -648,6 +660,8 @@ out:
if (vdd)
edp_panel_vdd_off(intel_dp, false);
+ mutex_unlock(&dev_priv->pps_mutex);
+
return ret;
}
@@ -1102,6 +1116,8 @@ static void wait_panel_status(struct intel_dp *intel_dp,
struct drm_i915_private *dev_priv = dev->dev_private;
u32 pp_stat_reg, pp_ctrl_reg;
+ lockdep_assert_held(&dev_priv->pps_mutex);
+
pp_stat_reg = _pp_stat_reg(intel_dp);
pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
@@ -1165,6 +1181,8 @@ static u32 ironlake_get_pp_control(struct intel_dp
*intel_dp)
struct drm_i915_private *dev_priv = dev->dev_private;
u32 control;
+ lockdep_assert_held(&dev_priv->pps_mutex);
+
control = I915_READ(_pp_ctrl_reg(intel_dp));
control &= ~PANEL_UNLOCK_MASK;
control |= PANEL_UNLOCK_REGS;
@@ -1182,6 +1200,8 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
u32 pp_stat_reg, pp_ctrl_reg;
bool need_to_disable = !intel_dp->want_panel_vdd;
+ lockdep_assert_held(&dev
[Intel-gfx] [PATCH v2] drm/i915: Handle i915_ppgtt_put correctly
Unfortunately, the gem_obj/vma relationship is not symmetrical; a gem_obj
can look up for the same vma more than once (where the ppgtt refcount is
incremented), but will free the vma only once (i915_gem_free_object).
This difference in refcount get/put means that the ppgtt is not removed
after the context and vma are destroyed, because sometimes the refcount
will never go back to zero.
v2: Just move the ppgtt refcount into vma_create.
OTC-Jira: VIZ-3719
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 840365c..95f6df4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2251,8 +2251,10 @@ static struct i915_vma *__i915_gem_vma_create(struct
drm_i915_gem_object *obj,
/* Keep GGTT vmas first to make debug easier */
if (i915_is_ggtt(vm))
list_add(&vma->vma_link, &obj->vma_list);
- else
+ else {
list_add_tail(&vma->vma_link, &obj->vma_list);
+ i915_ppgtt_get(i915_vm_to_ppgtt(vm));
+ }
return vma;
}
@@ -2267,8 +2269,5 @@ i915_gem_obj_lookup_or_create_vma(struct
drm_i915_gem_object *obj,
if (!vma)
vma = __i915_gem_vma_create(obj, vm);
- if (!i915_is_ggtt(vm))
- i915_ppgtt_get(i915_vm_to_ppgtt(vm));
-
return vma;
}
--
2.0.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Populate lrc with aliasing ppgtt if required
Ok, things start to dawn upon me - let's blame it on the lack of
decent coffee here ;-) I mixed up USES_PPGTT with USES_FULL_PPGTT
(again) so didn't realize that execlist works with just aliasing
ppgtt. So I guess there's still room to clean up these macros a bit,
maybe do an s/USES_PPGTT/USES_HW_PPGTT/ or so, to make it clear that
this isn't about multiple address spaces.
I do like Damien's suggestion though since imo depending upon runtime
state is less fragile than rechecking driver option.
-Daniel
On Mon, Aug 18, 2014 at 6:49 PM, Damien Lespiau
wrote:
> On Mon, Aug 18, 2014 at 03:54:02PM +0100, Thomas Daniel wrote:
>> A previous commit broke aliasing PPGTT for lrc, resulting in a kernel oops
>> on boot. Add a check so that is full PPGTT is not in use the context is
>> populated with the aliasing PPGTT.
>>
>> Issue: VIZ-4278
>> Signed-off-by: Thomas Daniel
>> ---
>> drivers/gpu/drm/i915/intel_lrc.c |7 +++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
>> b/drivers/gpu/drm/i915/intel_lrc.c
>> index c096b9b..79a6b91 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -1452,12 +1452,19 @@ static int
>> populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_object
>> *ctx_obj,
>> struct intel_engine_cs *ring, struct intel_ringbuffer
>> *ringbuf)
>> {
>> + struct drm_device *dev = ring->dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> struct drm_i915_gem_object *ring_obj = ringbuf->obj;
>> struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
>
> It's a bit weird to leave ppgtt initialized here when you're going to
> always override it below.
>
>> struct page *page;
>> uint32_t *reg_state;
>> int ret;
>>
>> + if (USES_FULL_PPGTT(dev))
>> + ppgtt = ctx->ppgtt;
>> + else
>> + ppgtt = dev_priv->mm.aliasing_ppgtt;
>> +
>
> The patch drom daniel you mention removes the usage of USES_FULL_PPGTT()
> to directly test ctx->ppgtt. You may want to reproduce that here ie.
>
> ppgtt = ctx->ppgtt;
> if (!ppgtt)
> ppgtt = dev_priv->mm.aliasing_ppgtt;
>
> doesn't really matter either way I guess, but can we at least not set
> then always override ppgtt?
>
> Thanks,
>
> --
> Damien
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: don't warn if backlight unexpectedly enabled
On Tue, Aug 19, 2014 at 4:07 AM, Scot Doyle wrote: > BIOS or firmware can modify hardware state during suspend/resume, > for example on the Toshiba CB35 or Lenovo T400, so log a debug message > instead of a warning if the backlight is unexpectedly enabled. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80930 > Cc: Jani Nikula > Signed-off-by: Scot Doyle This should get cleaned up in the modeset state sanitization we do upon resume, so without someone digging into this bug a bit and coming up with an explanation for why that fails I'm reluctant to merge this. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Disable execlists by default
On Mon, Aug 18, 2014 at 4:55 PM, Paulo Zanoni wrote: > 2014-08-18 5:29 GMT-03:00 Jani Nikula : >> On Fri, 15 Aug 2014, Damien Lespiau wrote: >>> We still have a few missing bits and pieces to have execlists enabled by >>> default eg. the error capture or the render state initialization and so >>> it wouldn't be wise to enable it by default on BDW just yet. >> >> Also, >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82740 > > Yes, please! I can't boot BDW anymore without this. > > Reviewed-by: Paulo Zanoni > Tested-by: Paulo Zanoni So execlist is only possible with full ppgtt (or at least that should be the case) and full ppgtt is disabled by default. Obviously that doesn't match the reality, but the bug also doesn't have dmesg attached to check for that and fix the underlying thing? I'll try to merge this meanwhile if the wifi here doesn't fall over again. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Handle i915_ppgtt_put correctly
On Tue, Aug 19, 2014 at 10:50 AM, Michel Thierry
wrote:
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2223,6 +2223,7 @@ static struct i915_vma *__i915_gem_vma_create(struct
> drm_i915_gem_object *obj,
> INIT_LIST_HEAD(&vma->exec_list);
> vma->vm = vm;
> vma->obj = obj;
> + vma->ppgtt_refcount = 0;
>
> switch (INTEL_INFO(vm->dev)->gen) {
> case 8:
> @@ -2267,8 +2268,10 @@ i915_gem_obj_lookup_or_create_vma(struct
> drm_i915_gem_object *obj,
> if (!vma)
> vma = __i915_gem_vma_create(obj, vm);
>
> - if (!i915_is_ggtt(vm))
> + if (!i915_is_ggtt(vm)) {
> i915_ppgtt_get(i915_vm_to_ppgtt(vm));
> + vma->ppgtt_refcount++;
> + }
Shouldn't we just move the ppgtt into the vma_create hunk (or
function) instead of this complication?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/14] drm/i915: Warn about want_panel_vdd in edp_panel_vdd_off_sync()
On Tue, 19 Aug 2014, Ville Syrjälä wrote:
> On Tue, Aug 19, 2014 at 10:36:52AM +0300, Jani Nikula wrote:
>> On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote:
>> > From: Ville Syrjälä
>> >
>> > If we force vdd off warn if someone is still using it. With this
>> > change the delayed vdd off work needs to check want_panel_vdd
>> > itself to make sure it doesn't try to turn vdd off when someone
>> > is using it.
>>
>> I think this calls for a prep cleanup patch to check and fix the uses of
>> edp_panel_vdd_off(intel_dp, true)
>> vs. edp_panel_vdd_off_sync(intel_dp). In particular, why are there
>> direct calls to the latter all over the place? Seems wrong.
>
> edp_panel_vdd_off() should always be paired with a edp_panel_vdd_on().
> If we were to call edp_panel_vdd_off() without the correct pairing we
> would get a warning due to want_panel_vdd==false, whereas
> edp_panel_vdd_off_sync() will now warn when want_panel_vdd==true.
> The direct calls to edp_panel_vdd_off_sync() are in places where we
> should not have want_panel_vdd==true (eg. system suspend) but we
> just want to force vdd off even if the delayed off work has alrady
> been scheduled.
Okay, care to add some of that as brief documentation comments for the
functions in question, as follow-up? IMO detailed kernel-docs here won't
be read by anyone and will just get stale.
BR,
Jani.
>
>>
>> BR,
>> Jani.
>>
>>
>> >
>> > Signed-off-by: Ville Syrjälä
>> > ---
>> > drivers/gpu/drm/i915/intel_dp.c | 7 +--
>> > 1 file changed, 5 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> > b/drivers/gpu/drm/i915/intel_dp.c
>> > index e6b4d4d..0fb510c 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -1241,7 +1241,9 @@ static void edp_panel_vdd_off_sync(struct intel_dp
>> > *intel_dp)
>> >
>> >WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>> >
>> > - if (intel_dp->want_panel_vdd || !edp_have_panel_vdd(intel_dp))
>> > + WARN_ON(intel_dp->want_panel_vdd);
>> > +
>> > + if (!edp_have_panel_vdd(intel_dp))
>> >return;
>> >
>> >DRM_DEBUG_KMS("Turning eDP VDD off\n");
>> > @@ -1273,7 +1275,8 @@ static void edp_panel_vdd_work(struct work_struct
>> > *__work)
>> >struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> >
>> >drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>> > - edp_panel_vdd_off_sync(intel_dp);
>> > + if (!intel_dp->want_panel_vdd)
>> > + edp_panel_vdd_off_sync(intel_dp);
>> >drm_modeset_unlock(&dev->mode_config.connection_mutex);
>> > }
>> >
>> > --
>> > 1.8.5.5
>> >
>> > ___
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
>
> --
> Ville Syrjälä
> Intel OTC
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [REGRESSION BISECTED] backlight control stops workin with 3.14 and later
On Tue, 19 Aug 2014, Bertrik Sikken wrote: >> On Sun, 17 Aug 2014, Bertrik Sikken wrote: >>> On 15-8-2014 3:43, Jani Nikula wrote: On Thu, 14 Aug 2014, Bertrik Sikken wrote: >>> > Attached is dmesg output from booting kernel 3.14-2 (debian unstable) > with drm.debug=0xe and the samsung_laptop module enabled, from my > Samsung N150plus netbook. Have you tried 3.15? >>> >>> I've built the v3.15 kernel (using the .config file from debian >>> unstable and doing make oldconfig). >>> >>> The backlight is at maximum brightness after boot and I can't control >>> it using the backlight buttons, nor by writing to >>> /sys/class/backlight/samsung/brightness >>> (say half the value or 1/10th of max_brightness) >>> >>> Backlight does work when writing >>> /sys/class/backlight/intel_backlight/brightness >> >> How about disabling samsung backlight module with 3.15? > > I'm not sure what you mean by that. > > As I understand it, there are three ways to control the backlight on this > netbook: using intel_backlight, samsung_laptop (using a "sabi" interface) > and acpi_video. > Backlight control using the samsung_laptop driver no longer seems to work > after the change. If I disable it (e.g. by blacklisting it), I expect it > to no longer work at all obviously. > > What do you want me to test exactly? If the intel_backlight interface works in 3.15, I presume the problem is that you have a non-functional samsung backlight interface that is preferred over intel_backlight by your userspace. BR, Jani. > > Kind regards, > Bertrik > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Rework GPU reset sequence to match driver load & thaw
"Mcaulay, Alistair" writes:
> Hi Mika,
>
> can you please review this patch, and verify it fixes the issues in your
> previous review.
>
> Thanks,
> Alistair.
>
>> -Original Message-
>> From: Mcaulay, Alistair
>> Sent: Friday, August 15, 2014 6:52 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Mcaulay, Alistair
>> Subject: [PATCH v3] drm/i915: Rework GPU reset sequence to match driver
>> load & thaw
>>
>> From: "McAulay, Alistair"
>>
>> This patch is to address Daniels concerns over different code during reset:
>>
>> http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.html
>>
>> "The reason for aiming as hard as possible to use the exact same code for
>> driver load, gpu reset and runtime pm/system resume is that we've simply
>> seen too many bugs due to slight variations and unintended omissions."
>>
>> Tested using igt drv_hangman.
>>
>> V2: Cleaner way of preventing check_wedge returning -EAGAIN
>> V3: Clean the last_context during reset, to ensure do_switch() does the
>> MI_SET_CONTEXT. As per review.
>> Signed-off-by: McAulay, Alistair
>> ---
Reviewed-by: Mika Kuoppala
>> drivers/gpu/drm/i915/i915_drv.c | 6 +++
>> drivers/gpu/drm/i915/i915_drv.h | 3 ++
>> drivers/gpu/drm/i915/i915_gem.c | 4 +-
>> drivers/gpu/drm/i915/i915_gem_context.c | 33 +++-
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 67
>> +
>> drivers/gpu/drm/i915/i915_gem_gtt.h | 3 +-
>> 6 files changed, 28 insertions(+), 88 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c index 5e4fefd..3bfafe6 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -806,7 +806,13 @@ int i915_reset(struct drm_device *dev)
>> !dev_priv->ums.mm_suspended) {
>> dev_priv->ums.mm_suspended = 0;
>>
>> +/* Used to prevent gem_check_wedged returning -EAGAIN
>> during gpu reset */
>> +dev_priv->gpu_error.reload_in_reset = true;
>> +
>> ret = i915_gem_init_hw(dev);
>> +
>> +dev_priv->gpu_error.reload_in_reset = false;
>> +
>> mutex_unlock(&dev->struct_mutex);
>> if (ret) {
>> DRM_ERROR("Failed hw init on reset %d\n", ret); diff
>> --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 991b663..116daff 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1217,6 +1217,9 @@ struct i915_gpu_error {
>>
>> /* For missed irq/seqno simulation. */
>> unsigned int test_irq_rings;
>> +
>> +/* Used to prevent gem_check_wedged returning -EAGAIN during
>> gpu reset */
>> +bool reload_in_reset;
>> };
>>
>> enum modeset_restore {
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> b/drivers/gpu/drm/i915/i915_gem.c index ef047bc..e7396eb 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1085,7 +1085,9 @@ i915_gem_check_wedge(struct i915_gpu_error
>> *error,
>> if (i915_terminally_wedged(error))
>> return -EIO;
>>
>> -return -EAGAIN;
>> +/* Check if GPU Reset is in progress */
>> +if (!error->reload_in_reset)
>> +return -EAGAIN;
>> }
>>
>> return 0;
>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
>> b/drivers/gpu/drm/i915/i915_gem_context.c
>> index de72a28..9378ad8 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> @@ -377,34 +377,17 @@ void i915_gem_context_reset(struct drm_device
>> *dev)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> int i;
>>
>> -/* Prevent the hardware from restoring the last context (which
>> hung) on
>> - * the next switch */
>> for (i = 0; i < I915_NUM_RINGS; i++) {
>> struct intel_engine_cs *ring = &dev_priv->ring[i];
>> -struct intel_context *dctx = ring->default_context;
>> struct intel_context *lctx = ring->last_context;
>>
>> -/* Do a fake switch to the default context */
>> -if (lctx == dctx)
>> -continue;
>> -
>> -if (!lctx)
>> -continue;
>> +if (lctx) {
>> +if (lctx->legacy_hw_ctx.rcs_state && i == RCS)
>> +i915_gem_object_ggtt_unpin(lctx-
>> >legacy_hw_ctx.rcs_state);
>>
>> -if (dctx->legacy_hw_ctx.rcs_state && i == RCS) {
>> -WARN_ON(i915_gem_obj_ggtt_pin(dctx-
>> >legacy_hw_ctx.rcs_state,
>> -
>> get_context_alignment(dev), 0));
>> -/* Fake a finish/inactive */
>> -dctx->legacy_hw_ctx.rcs_state->base.write_domain
>> = 0;
>> -dctx->legacy_hw_ctx.rcs_state->active = 0;
>> +i915_gem_context_unref
Re: [Intel-gfx] [PATCH 12/14] drm/i915: Turn on panel power before doing aux transfers
On Tue, Aug 19, 2014 at 10:33:15AM +0300, Jani Nikula wrote: > On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > On VLV/CHV the panel power sequencer may need to be "kicked" a bit to > > lock onto the new port, and that needs to happen before any aux > > transfers are attempted if we want the aux transfers to actaully > > succeed. So turn on panel power (part of the "kick") before aux > > transfers (DPMS_ON + link training). > > > > This also matches the documented modeset sequence better for pch > > platforms. The documentation doesn't explicitly state anything about the > > DPMS or link training DPCD writes, but the panel power on step is > > always listed before link training is mentioned. > > > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/intel_dp.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index 4952783..28bc652 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -2275,10 +2275,10 @@ static void intel_enable_dp(struct intel_encoder > > *encoder) > > return; > > > > intel_edp_panel_vdd_on(intel_dp); > > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > - intel_dp_start_link_train(intel_dp); > > intel_edp_panel_on(intel_dp); > > intel_edp_panel_vdd_off(intel_dp, true); > > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > + intel_dp_start_link_train(intel_dp); > > Please dig into the git history in this area. I fear this may > regress. We've juggled this too many times... I did. But I couldn't spot much solid analysis of the problems in the earlier patches/reverts. It's mostly been guesswork AFAICS. Most of it seems to be back and forth with the force vdd on/off vs. panel power on, but this patch doesn't change that order. Also: a) we need this patch on VLV/CHV at the very least b) this agrees with the bspec modeset sequence for pch platforms better c) my ILK with CPU eDP seems happy with the new order -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/14] drm/i915: Flatten intel_edp_panel_vdd_on()
On Tue, Aug 19, 2014 at 10:30:25AM +0300, Jani Nikula wrote:
> On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Less pointless indentation is always nice. There will be a bit more
> > code in this function once the power sequencer locking is fixed.
>
> Reviewed-by: Jani Nikula
>
> This could be earlier in the series, right?
Yeah doens't really matter. Or could even be squashed with the locking
patch.
>
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 11 +++
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 0fb510c..7ae9a9a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1221,11 +1221,14 @@ static bool edp_panel_vdd_on(struct intel_dp
> > *intel_dp)
> >
> > void intel_edp_panel_vdd_on(struct intel_dp *intel_dp)
> > {
> > - if (is_edp(intel_dp)) {
> > - bool vdd = edp_panel_vdd_on(intel_dp);
> > + bool vdd;
> >
> > - WARN(!vdd, "eDP VDD already requested on\n");
> > - }
> > + if (!is_edp(intel_dp))
> > + return;
> > +
> > + vdd = edp_panel_vdd_on(intel_dp);
> > +
> > + WARN(!vdd, "eDP VDD already requested on\n");
> > }
> >
> > static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
> > --
> > 1.8.5.5
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/14] drm/i915: edp vdd locking and prep for power sequencer kick
On Tue, Aug 19, 2014 at 11:08:33AM +0300, Jani Nikula wrote: > On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > While wrestling with the VLV/CHV panel power sequencer I noticed the locking > > in our edp vdd code was rather broken. This series aims to fix that by > > introducing a power seqeuencer mutex. I was already thinking about using the > > aux.hw_mutex for this since it's already locked around the aux ->transfer() > > function, but the VLV/CHV multiple power sequencer issue requires a single > > lock instead of per-port. > > For extra kicks, see i915_save_display() and i915_restore_display(). Why > are we doing this to ourselves? Yeah, crap all around. I suppose someone needs to frob the lvds code a bit before we can kill the power sequencer stuff from those two functions. > > BR, > Jani. > > > > > > > At the end of the series there's a bit of reordering of the DP port > > enable/disable sequences to make subsequent power sequencer kick patches > > easier. The last patch fixes the wait_pipe_off() timeouts on my ILK. > > Strictly speaking it shouldn't be part of this series, but I couldn't > > really test this on my ILK without suffering tons of warnings so I > > included it here anyway. > > > > Ville Syrjälä (14): > > drm/i915: Parametrize PANEL_PORT_SELECT_VLV > > drm/i915: Reorganize vlv eDP reboot notifier > > drm/i915: Use intel_edp_panel_vdd_on() in intel_dp_probe_mst() > > drm/i915: Rename edp vdd funcs for consistency > > drm/i915: Add a note explaining vdd on/off handling in > > intel_dp_aux_ch() > > drm/i915: Replace big nested if block with early return > > drm/i915: Warn about want_panel_vdd in edp_panel_vdd_off_sync() > > drm/i915: Flatten intel_edp_panel_vdd_on() > > drm/i915: Fix edp vdd locking > > drm/i915: Track which port is using which pipe's power sequencer > > drm/i915: Be more careful when picking the initial power sequencer > > pipe > > drm/i915: Turn on panel power before doing aux transfers > > drm/i915: Enable DP port earlier > > drm/i915: Move DP port disable to post_disable for pch platforms > > > > drivers/gpu/drm/i915/i915_drv.h | 3 + > > drivers/gpu/drm/i915/i915_reg.h | 3 +- > > drivers/gpu/drm/i915/intel_display.c | 2 + > > drivers/gpu/drm/i915/intel_dp.c | 619 > > +-- > > drivers/gpu/drm/i915/intel_drv.h | 6 + > > 5 files changed, 463 insertions(+), 170 deletions(-) > > > > -- > > 1.8.5.5 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 i-g-t 1/1] tests: Fix seg fault when gem_mmap is run without specifying a subtest
On 18 August 2014 18:43, Mike Mason wrote:
> gem_mmap seg faults when all tests are run together. This occurs because
> the new-object subtest closes the gem object, but short-mmap assumes
> it still exists. Thus gem_mmap__cpu() returns nil for addr and memset()
> seg faults. This patch makes new-object and short-mmap create and
> close their own gem objects.
Both patches (this one and "scripts: Allow multiple -t and -x regular
expressions for run-tests.sh") merged, thanks.
>
> Signed-off-by: Mike Mason
> ---
> tests/gem_mmap.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/gem_mmap.c b/tests/gem_mmap.c
> index 334bd76..33ffe45 100644
> --- a/tests/gem_mmap.c
> +++ b/tests/gem_mmap.c
> @@ -62,10 +62,8 @@ igt_main
> igt_assert(ret == -1 && errno == ENOENT);
> }
>
> - igt_fixture
> - handle = gem_create(fd, OBJECT_SIZE);
> -
> igt_subtest("new-object") {
> + handle = gem_create(fd, OBJECT_SIZE);
> arg.handle = handle;
> arg.offset = 0;
> arg.size = OBJECT_SIZE;
> @@ -94,9 +92,11 @@ igt_main
>
> igt_subtest("short-mmap") {
> igt_assert(OBJECT_SIZE > 4096);
> + handle = gem_create(fd, OBJECT_SIZE);
> addr = gem_mmap__cpu(fd, handle, 4096, PROT_WRITE);
> memset(addr, 0, 4096);
> munmap(addr, 4096);
> + gem_close(fd, handle);
> }
>
> igt_fixture
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/14] drm/i915: Warn about want_panel_vdd in edp_panel_vdd_off_sync()
On Tue, Aug 19, 2014 at 10:36:52AM +0300, Jani Nikula wrote:
> On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > If we force vdd off warn if someone is still using it. With this
> > change the delayed vdd off work needs to check want_panel_vdd
> > itself to make sure it doesn't try to turn vdd off when someone
> > is using it.
>
> I think this calls for a prep cleanup patch to check and fix the uses of
> edp_panel_vdd_off(intel_dp, true)
> vs. edp_panel_vdd_off_sync(intel_dp). In particular, why are there
> direct calls to the latter all over the place? Seems wrong.
edp_panel_vdd_off() should always be paired with a edp_panel_vdd_on().
If we were to call edp_panel_vdd_off() without the correct pairing we
would get a warning due to want_panel_vdd==false, whereas
edp_panel_vdd_off_sync() will now warn when want_panel_vdd==true.
The direct calls to edp_panel_vdd_off_sync() are in places where we
should not have want_panel_vdd==true (eg. system suspend) but we
just want to force vdd off even if the delayed off work has alrady
been scheduled.
>
> BR,
> Jani.
>
>
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index e6b4d4d..0fb510c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1241,7 +1241,9 @@ static void edp_panel_vdd_off_sync(struct intel_dp
> > *intel_dp)
> >
> > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> >
> > - if (intel_dp->want_panel_vdd || !edp_have_panel_vdd(intel_dp))
> > + WARN_ON(intel_dp->want_panel_vdd);
> > +
> > + if (!edp_have_panel_vdd(intel_dp))
> > return;
> >
> > DRM_DEBUG_KMS("Turning eDP VDD off\n");
> > @@ -1273,7 +1275,8 @@ static void edp_panel_vdd_work(struct work_struct
> > *__work)
> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >
> > drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> > - edp_panel_vdd_off_sync(intel_dp);
> > + if (!intel_dp->want_panel_vdd)
> > + edp_panel_vdd_off_sync(intel_dp);
> > drm_modeset_unlock(&dev->mode_config.connection_mutex);
> > }
> >
> > --
> > 1.8.5.5
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 04/14] drm/i915: Rename edp vdd funcs for consistency
From: Ville Syrjälä
edp_* are now the lower level functions and intel_edp_* the higher level
ones. One should use them in pairs.
v2: Don't return void (Jani)
Reviewed-by: Jani Nikula
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 28d0183..dbaac22 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -111,7 +111,7 @@ static struct intel_dp *intel_attached_dp(struct
drm_connector *connector)
}
static void intel_dp_link_down(struct intel_dp *intel_dp);
-static bool _edp_panel_vdd_on(struct intel_dp *intel_dp);
+static bool edp_panel_vdd_on(struct intel_dp *intel_dp);
static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
int
@@ -533,7 +533,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
bool has_aux_irq = HAS_AUX_IRQ(dev);
bool vdd;
- vdd = _edp_panel_vdd_on(intel_dp);
+ vdd = edp_panel_vdd_on(intel_dp);
/* dp aux is extremely sensitive to irq latency, hence request the
* lowest possible wakeup latency and so prevent the cpu from going into
@@ -1165,7 +1165,7 @@ static u32 ironlake_get_pp_control(struct intel_dp
*intel_dp)
return control;
}
-static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
+static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -1216,7 +1216,7 @@ static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
void intel_edp_panel_vdd_on(struct intel_dp *intel_dp)
{
if (is_edp(intel_dp)) {
- bool vdd = _edp_panel_vdd_on(intel_dp);
+ bool vdd = edp_panel_vdd_on(intel_dp);
WARN(!vdd, "eDP VDD already requested on\n");
}
@@ -1299,6 +1299,11 @@ static void edp_panel_vdd_off(struct intel_dp *intel_dp,
bool sync)
edp_panel_vdd_schedule_off(intel_dp);
}
+static void intel_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync)
+{
+ edp_panel_vdd_off(intel_dp, sync);
+}
+
void intel_edp_panel_on(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -2102,7 +2107,7 @@ static void intel_enable_dp(struct intel_encoder *encoder)
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
intel_edp_panel_on(intel_dp);
- edp_panel_vdd_off(intel_dp, true);
+ intel_edp_panel_vdd_off(intel_dp, true);
intel_dp_complete_link_train(intel_dp);
intel_dp_stop_link_train(intel_dp);
}
@@ -3405,7 +3410,7 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)
DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
buf[0], buf[1], buf[2]);
- edp_panel_vdd_off(intel_dp, false);
+ intel_edp_panel_vdd_off(intel_dp, false);
}
static bool
@@ -3429,7 +3434,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp)
intel_dp->is_mst = false;
}
}
- edp_panel_vdd_off(intel_dp, false);
+ intel_edp_panel_vdd_off(intel_dp, false);
drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
return intel_dp->is_mst;
@@ -4527,7 +4532,7 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
/* Cache DPCD and EDID for edp. */
intel_edp_panel_vdd_on(intel_dp);
has_dpcd = intel_dp_get_dpcd(intel_dp);
- edp_panel_vdd_off(intel_dp, false);
+ intel_edp_panel_vdd_off(intel_dp, false);
if (has_dpcd) {
if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11)
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Rework GPU reset sequence to match driver load & thaw
Hi Mika,
can you please review this patch, and verify it fixes the issues in your
previous review.
Thanks,
Alistair.
> -Original Message-
> From: Mcaulay, Alistair
> Sent: Friday, August 15, 2014 6:52 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Mcaulay, Alistair
> Subject: [PATCH v3] drm/i915: Rework GPU reset sequence to match driver
> load & thaw
>
> From: "McAulay, Alistair"
>
> This patch is to address Daniels concerns over different code during reset:
>
> http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.html
>
> "The reason for aiming as hard as possible to use the exact same code for
> driver load, gpu reset and runtime pm/system resume is that we've simply
> seen too many bugs due to slight variations and unintended omissions."
>
> Tested using igt drv_hangman.
>
> V2: Cleaner way of preventing check_wedge returning -EAGAIN
> V3: Clean the last_context during reset, to ensure do_switch() does the
> MI_SET_CONTEXT. As per review.
> Signed-off-by: McAulay, Alistair
> ---
> drivers/gpu/drm/i915/i915_drv.c | 6 +++
> drivers/gpu/drm/i915/i915_drv.h | 3 ++
> drivers/gpu/drm/i915/i915_gem.c | 4 +-
> drivers/gpu/drm/i915/i915_gem_context.c | 33 +++-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 67
> +
> drivers/gpu/drm/i915/i915_gem_gtt.h | 3 +-
> 6 files changed, 28 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c index 5e4fefd..3bfafe6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -806,7 +806,13 @@ int i915_reset(struct drm_device *dev)
> !dev_priv->ums.mm_suspended) {
> dev_priv->ums.mm_suspended = 0;
>
> + /* Used to prevent gem_check_wedged returning -EAGAIN
> during gpu reset */
> + dev_priv->gpu_error.reload_in_reset = true;
> +
> ret = i915_gem_init_hw(dev);
> +
> + dev_priv->gpu_error.reload_in_reset = false;
> +
> mutex_unlock(&dev->struct_mutex);
> if (ret) {
> DRM_ERROR("Failed hw init on reset %d\n", ret); diff
> --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 991b663..116daff 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1217,6 +1217,9 @@ struct i915_gpu_error {
>
> /* For missed irq/seqno simulation. */
> unsigned int test_irq_rings;
> +
> + /* Used to prevent gem_check_wedged returning -EAGAIN during
> gpu reset */
> + bool reload_in_reset;
> };
>
> enum modeset_restore {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c index ef047bc..e7396eb 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1085,7 +1085,9 @@ i915_gem_check_wedge(struct i915_gpu_error
> *error,
> if (i915_terminally_wedged(error))
> return -EIO;
>
> - return -EAGAIN;
> + /* Check if GPU Reset is in progress */
> + if (!error->reload_in_reset)
> + return -EAGAIN;
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index de72a28..9378ad8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -377,34 +377,17 @@ void i915_gem_context_reset(struct drm_device
> *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> int i;
>
> - /* Prevent the hardware from restoring the last context (which
> hung) on
> - * the next switch */
> for (i = 0; i < I915_NUM_RINGS; i++) {
> struct intel_engine_cs *ring = &dev_priv->ring[i];
> - struct intel_context *dctx = ring->default_context;
> struct intel_context *lctx = ring->last_context;
>
> - /* Do a fake switch to the default context */
> - if (lctx == dctx)
> - continue;
> -
> - if (!lctx)
> - continue;
> + if (lctx) {
> + if (lctx->legacy_hw_ctx.rcs_state && i == RCS)
> + i915_gem_object_ggtt_unpin(lctx-
> >legacy_hw_ctx.rcs_state);
>
> - if (dctx->legacy_hw_ctx.rcs_state && i == RCS) {
> - WARN_ON(i915_gem_obj_ggtt_pin(dctx-
> >legacy_hw_ctx.rcs_state,
> -
> get_context_alignment(dev), 0));
> - /* Fake a finish/inactive */
> - dctx->legacy_hw_ctx.rcs_state->base.write_domain
> = 0;
> - dctx->legacy_hw_ctx.rcs_state->active = 0;
> + i915_gem_context_unreference(lctx);
> + ring->last_context = NULL;
> }
> -
> - if (lctx->legacy_hw_ctx.rcs_state && i == RCS)
> -
Re: [Intel-gfx] [REGRESSION BISECTED] backlight control stops workin with 3.14 and later
> On Sun, 17 Aug 2014, Bertrik Sikken wrote: >> On 15-8-2014 3:43, Jani Nikula wrote: >>> On Thu, 14 Aug 2014, Bertrik Sikken wrote: >> Attached is dmesg output from booting kernel 3.14-2 (debian unstable) with drm.debug=0xe and the samsung_laptop module enabled, from my Samsung N150plus netbook. >>> >>> Have you tried 3.15? >> >> I've built the v3.15 kernel (using the .config file from debian >> unstable and doing make oldconfig). >> >> The backlight is at maximum brightness after boot and I can't control >> it using the backlight buttons, nor by writing to >> /sys/class/backlight/samsung/brightness >> (say half the value or 1/10th of max_brightness) >> >> Backlight does work when writing >> /sys/class/backlight/intel_backlight/brightness > > How about disabling samsung backlight module with 3.15? I'm not sure what you mean by that. As I understand it, there are three ways to control the backlight on this netbook: using intel_backlight, samsung_laptop (using a "sabi" interface) and acpi_video. Backlight control using the samsung_laptop driver no longer seems to work after the change. If I disable it (e.g. by blacklisting it), I expect it to no longer work at all obviously. What do you want me to test exactly? Kind regards, Bertrik ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Populate lrc with aliasing ppgtt if required
On Tue, Aug 19, 2014 at 10:13:36AM +0100, Thomas Daniel wrote:
> A previous commit broke aliasing PPGTT for lrc, resulting in a kernel oops
> on boot. Add a check so that is full PPGTT is not in use the context is
> populated with the aliasing PPGTT.
Usually, we add a bit of history to the patch (even for trivial things)
something like:
v2: blah blah blah blah
But doesn't really matter much this time.
Reviewed-by: Damien Lespiau
--
Damien
> Issue: VIZ-4278
> Signed-off-by: Thomas Daniel
> ---
> drivers/gpu/drm/i915/intel_lrc.c |5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index c096b9b..b54f312 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1452,12 +1452,17 @@ static int
> populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_object
> *ctx_obj,
> struct intel_engine_cs *ring, struct intel_ringbuffer
> *ringbuf)
> {
> + struct drm_device *dev = ring->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_i915_gem_object *ring_obj = ringbuf->obj;
> struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
> struct page *page;
> uint32_t *reg_state;
> int ret;
>
> + if (!ppgtt)
> + ppgtt = dev_priv->mm.aliasing_ppgtt;
> +
> ret = i915_gem_object_set_to_cpu_domain(ctx_obj, true);
> if (ret) {
> DRM_DEBUG_DRIVER("Could not set to CPU domain\n");
> --
> 1.7.9.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/bdw: Populate lrc with aliasing ppgtt if required
A previous commit broke aliasing PPGTT for lrc, resulting in a kernel oops
on boot. Add a check so that is full PPGTT is not in use the context is
populated with the aliasing PPGTT.
Issue: VIZ-4278
Signed-off-by: Thomas Daniel
---
drivers/gpu/drm/i915/intel_lrc.c |5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index c096b9b..b54f312 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1452,12 +1452,17 @@ static int
populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_object
*ctx_obj,
struct intel_engine_cs *ring, struct intel_ringbuffer
*ringbuf)
{
+ struct drm_device *dev = ring->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *ring_obj = ringbuf->obj;
struct i915_hw_ppgtt *ppgtt = ctx->ppgtt;
struct page *page;
uint32_t *reg_state;
int ret;
+ if (!ppgtt)
+ ppgtt = dev_priv->mm.aliasing_ppgtt;
+
ret = i915_gem_object_set_to_cpu_domain(ctx_obj, true);
if (ret) {
DRM_DEBUG_DRIVER("Could not set to CPU domain\n");
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Handle i915_ppgtt_put correctly
Unfortunately, the gem_obj/vma relationship is not symmetrical; a gem_obj
can look up for the same vma more than once (where the ppgtt refcount is
incremented), but will free the vma only once (i915_gem_free_object).
This difference in refcount get/put means that the ppgtt is not removed
after the context and vma are destroyed, because sometimes the refcount
will never go back to zero.
Keep track of how many times a gem_obj has looked up for a given vma and
call i915_ppgtt_put accordingly. This will ensure that the ppgtt is also
removed.
OTC-Jira: VIZ-3719
Signed-off-by: Michel Thierry
---
drivers/gpu/drm/i915/i915_gem.c | 8 ++--
drivers/gpu/drm/i915/i915_gem_gtt.c | 5 -
drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 488244a..f8e9431 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4573,8 +4573,12 @@ void i915_gem_vma_destroy(struct i915_vma *vma)
vm = vma->vm;
- if (!i915_is_ggtt(vm))
- i915_ppgtt_put(i915_vm_to_ppgtt(vm));
+ if (!i915_is_ggtt(vm)) {
+ while (vma->ppgtt_refcount > 0) {
+ i915_ppgtt_put(i915_vm_to_ppgtt(vm));
+ vma->ppgtt_refcount--;
+ }
+ }
list_del(&vma->vma_link);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 840365c..db6bbe3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2223,6 +2223,7 @@ static struct i915_vma *__i915_gem_vma_create(struct
drm_i915_gem_object *obj,
INIT_LIST_HEAD(&vma->exec_list);
vma->vm = vm;
vma->obj = obj;
+ vma->ppgtt_refcount = 0;
switch (INTEL_INFO(vm->dev)->gen) {
case 8:
@@ -2267,8 +2268,10 @@ i915_gem_obj_lookup_or_create_vma(struct
drm_i915_gem_object *obj,
if (!vma)
vma = __i915_gem_vma_create(obj, vm);
- if (!i915_is_ggtt(vm))
+ if (!i915_is_ggtt(vm)) {
i915_ppgtt_get(i915_vm_to_ppgtt(vm));
+ vma->ppgtt_refcount++;
+ }
return vma;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 7616876..28e41d7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -122,6 +122,7 @@ struct i915_vma {
struct drm_mm_node node;
struct drm_i915_gem_object *obj;
struct i915_address_space *vm;
+ int ppgtt_refcount;
/** This object's place on the active/inactive lists */
struct list_head mm_list;
--
2.0.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/7] Rename DP training vswing/pre-emph defines
Hi, Did anybody get a chance to review other patches in this series? I got r-b for 2 patches (patches with changes in drm and i915) from Damien. Thanks, Sonika -Original Message- From: Jindal, Sonika Sent: Friday, August 8, 2014 4:24 PM To: intel-gfx@lists.freedesktop.org Cc: dri-de...@lists.freedesktop.org; Jindal, Sonika Subject: [PATCH v2 0/7] Rename DP training vswing/pre-emph defines From: Sonika Jindal Rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP 1.4 where the values are different. Updated in all the drivers as well v2: Keeping the old defines in first patch and removing them in last patch. Used cocci semantic patch to replace the defines. Sonika Jindal (7): drm: Renaming DP training vswing pre emph defines drm/i915: Renaming DP training vswing pre emph defines drm/exynos: Renaming DP training vswing pre emph defines drm/gma500: Renaming DP training vswing pre emph defines drm/radeon: Renaming DP training vswing pre emph defines drm/tegra: Renaming DP training vswing pre emph defines drm: Remove old defines for vswing and pre-emph values drivers/gpu/drm/exynos/exynos_dp_core.c |4 +- drivers/gpu/drm/gma500/cdv_intel_dp.c |4 +- drivers/gpu/drm/gma500/intel_bios.c | 16 +-- drivers/gpu/drm/i915/intel_bios.c | 16 +-- drivers/gpu/drm/i915/intel_dp.c | 194 +++ drivers/gpu/drm/radeon/atombios_dp.c|4 +- drivers/gpu/drm/tegra/dpaux.c |4 +- include/drm/drm_dp_helper.h | 16 +-- 8 files changed, 129 insertions(+), 129 deletions(-) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/14] drm/i915: edp vdd locking and prep for power sequencer kick
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > While wrestling with the VLV/CHV panel power sequencer I noticed the locking > in our edp vdd code was rather broken. This series aims to fix that by > introducing a power seqeuencer mutex. I was already thinking about using the > aux.hw_mutex for this since it's already locked around the aux ->transfer() > function, but the VLV/CHV multiple power sequencer issue requires a single > lock instead of per-port. For extra kicks, see i915_save_display() and i915_restore_display(). Why are we doing this to ourselves? BR, Jani. > > At the end of the series there's a bit of reordering of the DP port > enable/disable sequences to make subsequent power sequencer kick patches > easier. The last patch fixes the wait_pipe_off() timeouts on my ILK. > Strictly speaking it shouldn't be part of this series, but I couldn't > really test this on my ILK without suffering tons of warnings so I > included it here anyway. > > Ville Syrjälä (14): > drm/i915: Parametrize PANEL_PORT_SELECT_VLV > drm/i915: Reorganize vlv eDP reboot notifier > drm/i915: Use intel_edp_panel_vdd_on() in intel_dp_probe_mst() > drm/i915: Rename edp vdd funcs for consistency > drm/i915: Add a note explaining vdd on/off handling in > intel_dp_aux_ch() > drm/i915: Replace big nested if block with early return > drm/i915: Warn about want_panel_vdd in edp_panel_vdd_off_sync() > drm/i915: Flatten intel_edp_panel_vdd_on() > drm/i915: Fix edp vdd locking > drm/i915: Track which port is using which pipe's power sequencer > drm/i915: Be more careful when picking the initial power sequencer > pipe > drm/i915: Turn on panel power before doing aux transfers > drm/i915: Enable DP port earlier > drm/i915: Move DP port disable to post_disable for pch platforms > > drivers/gpu/drm/i915/i915_drv.h | 3 + > drivers/gpu/drm/i915/i915_reg.h | 3 +- > drivers/gpu/drm/i915/intel_display.c | 2 + > drivers/gpu/drm/i915/intel_dp.c | 619 > +-- > drivers/gpu/drm/i915/intel_drv.h | 6 + > 5 files changed, 463 insertions(+), 170 deletions(-) > > -- > 1.8.5.5 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/14] drm/i915: edp vdd locking and prep for power sequencer kick
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > While wrestling with the VLV/CHV panel power sequencer I noticed the locking > in our edp vdd code was rather broken. This series aims to fix that by > introducing a power seqeuencer mutex. I was already thinking about using the > aux.hw_mutex for this since it's already locked around the aux ->transfer() > function, but the VLV/CHV multiple power sequencer issue requires a single > lock instead of per-port. > > At the end of the series there's a bit of reordering of the DP port > enable/disable sequences to make subsequent power sequencer kick patches > easier. The last patch fixes the wait_pipe_off() timeouts on my ILK. > Strictly speaking it shouldn't be part of this series, but I couldn't > really test this on my ILK without suffering tons of warnings so I > included it here anyway. This is what I'd like to happen here: 1) Patches [1] from me and [2] from Clinton get reviewed and merged first, through -fixes. 2) The rest of the patches in my series after [1] get reviewed and merged, through dinq. 3) The first part of this series, up to the locking bits, mostly reviewed now, get refreshed on top the above, should not conflict much, and we can start merging them while... 4) The second part of this series, from about the locking on, get reviewed and merged. I want this order to make backporting steps 1 and 2 as painless as possible. Please help review them, and I'll follow through with this series. BR, Jani. [1] http://mid.gmane.org/e7a47b50ed0f25cafdc26711fc09561ea8af3b81.1407849872.git.jani.nik...@intel.com [2] http://mid.gmane.org/1408394915-21882-1-git-send-email-clinton.a.tay...@intel.com > > Ville Syrjälä (14): > drm/i915: Parametrize PANEL_PORT_SELECT_VLV > drm/i915: Reorganize vlv eDP reboot notifier > drm/i915: Use intel_edp_panel_vdd_on() in intel_dp_probe_mst() > drm/i915: Rename edp vdd funcs for consistency > drm/i915: Add a note explaining vdd on/off handling in > intel_dp_aux_ch() > drm/i915: Replace big nested if block with early return > drm/i915: Warn about want_panel_vdd in edp_panel_vdd_off_sync() > drm/i915: Flatten intel_edp_panel_vdd_on() > drm/i915: Fix edp vdd locking > drm/i915: Track which port is using which pipe's power sequencer > drm/i915: Be more careful when picking the initial power sequencer > pipe > drm/i915: Turn on panel power before doing aux transfers > drm/i915: Enable DP port earlier > drm/i915: Move DP port disable to post_disable for pch platforms > > drivers/gpu/drm/i915/i915_drv.h | 3 + > drivers/gpu/drm/i915/i915_reg.h | 3 +- > drivers/gpu/drm/i915/intel_display.c | 2 + > drivers/gpu/drm/i915/intel_dp.c | 619 > +-- > drivers/gpu/drm/i915/intel_drv.h | 6 + > 5 files changed, 463 insertions(+), 170 deletions(-) > > -- > 1.8.5.5 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/14] drm/i915: Warn about want_panel_vdd in edp_panel_vdd_off_sync()
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> If we force vdd off warn if someone is still using it. With this
> change the delayed vdd off work needs to check want_panel_vdd
> itself to make sure it doesn't try to turn vdd off when someone
> is using it.
I think this calls for a prep cleanup patch to check and fix the uses of
edp_panel_vdd_off(intel_dp, true)
vs. edp_panel_vdd_off_sync(intel_dp). In particular, why are there
direct calls to the latter all over the place? Seems wrong.
BR,
Jani.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dp.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e6b4d4d..0fb510c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1241,7 +1241,9 @@ static void edp_panel_vdd_off_sync(struct intel_dp
> *intel_dp)
>
> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>
> - if (intel_dp->want_panel_vdd || !edp_have_panel_vdd(intel_dp))
> + WARN_ON(intel_dp->want_panel_vdd);
> +
> + if (!edp_have_panel_vdd(intel_dp))
> return;
>
> DRM_DEBUG_KMS("Turning eDP VDD off\n");
> @@ -1273,7 +1275,8 @@ static void edp_panel_vdd_work(struct work_struct
> *__work)
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
>
> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> - edp_panel_vdd_off_sync(intel_dp);
> + if (!intel_dp->want_panel_vdd)
> + edp_panel_vdd_off_sync(intel_dp);
> drm_modeset_unlock(&dev->mode_config.connection_mutex);
> }
>
> --
> 1.8.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/14] drm/i915: Turn on panel power before doing aux transfers
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > On VLV/CHV the panel power sequencer may need to be "kicked" a bit to > lock onto the new port, and that needs to happen before any aux > transfers are attempted if we want the aux transfers to actaully > succeed. So turn on panel power (part of the "kick") before aux > transfers (DPMS_ON + link training). > > This also matches the documented modeset sequence better for pch > platforms. The documentation doesn't explicitly state anything about the > DPMS or link training DPCD writes, but the panel power on step is > always listed before link training is mentioned. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_dp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 4952783..28bc652 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -2275,10 +2275,10 @@ static void intel_enable_dp(struct intel_encoder > *encoder) > return; > > intel_edp_panel_vdd_on(intel_dp); > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > - intel_dp_start_link_train(intel_dp); > intel_edp_panel_on(intel_dp); > intel_edp_panel_vdd_off(intel_dp, true); > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > + intel_dp_start_link_train(intel_dp); Please dig into the git history in this area. I fear this may regress. We've juggled this too many times... BR, Jani. > intel_dp_complete_link_train(intel_dp); > intel_dp_stop_link_train(intel_dp); > } > -- > 1.8.5.5 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/14] drm/i915: Replace big nested if block with early return
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Looks nicer.
Side note, I kind of like adding "No functional changes." in the commit
messages of patches that do not intend to do functional changes. I find
it helpful.
Reviewed-by: Jani Nikula
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dp.c | 40
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 19a818f..e6b4d4d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1232,38 +1232,38 @@ static void edp_panel_vdd_off_sync(struct intel_dp
> *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_digital_port *intel_dig_port =
> + dp_to_dig_port(intel_dp);
> + struct intel_encoder *intel_encoder = &intel_dig_port->base;
> + enum intel_display_power_domain power_domain;
> u32 pp;
> u32 pp_stat_reg, pp_ctrl_reg;
>
> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>
> - if (!intel_dp->want_panel_vdd && edp_have_panel_vdd(intel_dp)) {
> - struct intel_digital_port *intel_dig_port =
> - dp_to_dig_port(intel_dp);
> - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> - enum intel_display_power_domain power_domain;
> + if (intel_dp->want_panel_vdd || !edp_have_panel_vdd(intel_dp))
> + return;
>
> - DRM_DEBUG_KMS("Turning eDP VDD off\n");
> + DRM_DEBUG_KMS("Turning eDP VDD off\n");
>
> - pp = ironlake_get_pp_control(intel_dp);
> - pp &= ~EDP_FORCE_VDD;
> + pp = ironlake_get_pp_control(intel_dp);
> + pp &= ~EDP_FORCE_VDD;
>
> - pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
> - pp_stat_reg = _pp_stat_reg(intel_dp);
> + pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
> + pp_stat_reg = _pp_stat_reg(intel_dp);
>
> - I915_WRITE(pp_ctrl_reg, pp);
> - POSTING_READ(pp_ctrl_reg);
> + I915_WRITE(pp_ctrl_reg, pp);
> + POSTING_READ(pp_ctrl_reg);
>
> - /* Make sure sequencer is idle before allowing subsequent
> activity */
> - DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
> - I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
> + /* Make sure sequencer is idle before allowing subsequent activity */
> + DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
> + I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>
> - if ((pp & POWER_TARGET_ON) == 0)
> - intel_dp->last_power_cycle = jiffies;
> + if ((pp & POWER_TARGET_ON) == 0)
> + intel_dp->last_power_cycle = jiffies;
>
> - power_domain = intel_display_port_power_domain(intel_encoder);
> - intel_display_power_put(dev_priv, power_domain);
> - }
> + power_domain = intel_display_port_power_domain(intel_encoder);
> + intel_display_power_put(dev_priv, power_domain);
> }
>
> static void edp_panel_vdd_work(struct work_struct *__work)
> --
> 1.8.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/14] drm/i915: Flatten intel_edp_panel_vdd_on()
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Less pointless indentation is always nice. There will be a bit more
> code in this function once the power sequencer locking is fixed.
Reviewed-by: Jani Nikula
This could be earlier in the series, right?
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dp.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0fb510c..7ae9a9a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1221,11 +1221,14 @@ static bool edp_panel_vdd_on(struct intel_dp
> *intel_dp)
>
> void intel_edp_panel_vdd_on(struct intel_dp *intel_dp)
> {
> - if (is_edp(intel_dp)) {
> - bool vdd = edp_panel_vdd_on(intel_dp);
> + bool vdd;
>
> - WARN(!vdd, "eDP VDD already requested on\n");
> - }
> + if (!is_edp(intel_dp))
> + return;
> +
> + vdd = edp_panel_vdd_on(intel_dp);
> +
> + WARN(!vdd, "eDP VDD already requested on\n");
> }
>
> static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
> --
> 1.8.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/14] drm/i915: Rename edp vdd funcs for consistency
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> edp_* are now the lower level functions and intel_edp_* the higher level
> ones. One should use them in pairs.
Yeah I should've done this when I added the lower level ones. One thing
you need to fix below though.
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dp.c | 21 +
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 28d0183..30943a5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -111,7 +111,7 @@ static struct intel_dp *intel_attached_dp(struct
> drm_connector *connector)
> }
>
> static void intel_dp_link_down(struct intel_dp *intel_dp);
> -static bool _edp_panel_vdd_on(struct intel_dp *intel_dp);
> +static bool edp_panel_vdd_on(struct intel_dp *intel_dp);
> static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
>
> int
> @@ -533,7 +533,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> bool has_aux_irq = HAS_AUX_IRQ(dev);
> bool vdd;
>
> - vdd = _edp_panel_vdd_on(intel_dp);
> + vdd = edp_panel_vdd_on(intel_dp);
>
> /* dp aux is extremely sensitive to irq latency, hence request the
>* lowest possible wakeup latency and so prevent the cpu from going into
> @@ -1165,7 +1165,7 @@ static u32 ironlake_get_pp_control(struct intel_dp
> *intel_dp)
> return control;
> }
>
> -static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
> +static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> @@ -1216,7 +1216,7 @@ static bool _edp_panel_vdd_on(struct intel_dp *intel_dp)
> void intel_edp_panel_vdd_on(struct intel_dp *intel_dp)
> {
> if (is_edp(intel_dp)) {
> - bool vdd = _edp_panel_vdd_on(intel_dp);
> + bool vdd = edp_panel_vdd_on(intel_dp);
>
> WARN(!vdd, "eDP VDD already requested on\n");
> }
> @@ -1299,6 +1299,11 @@ static void edp_panel_vdd_off(struct intel_dp
> *intel_dp, bool sync)
> edp_panel_vdd_schedule_off(intel_dp);
> }
>
> +static void intel_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync)
> +{
> + return edp_panel_vdd_off(intel_dp, sync);
Please don't return voids. With that fixed,
Reviewed-by: Jani Nikula
> +}
> +
> void intel_edp_panel_on(struct intel_dp *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> @@ -2102,7 +2107,7 @@ static void intel_enable_dp(struct intel_encoder
> *encoder)
> intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> intel_dp_start_link_train(intel_dp);
> intel_edp_panel_on(intel_dp);
> - edp_panel_vdd_off(intel_dp, true);
> + intel_edp_panel_vdd_off(intel_dp, true);
> intel_dp_complete_link_train(intel_dp);
> intel_dp_stop_link_train(intel_dp);
> }
> @@ -3405,7 +3410,7 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)
> DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
> buf[0], buf[1], buf[2]);
>
> - edp_panel_vdd_off(intel_dp, false);
> + intel_edp_panel_vdd_off(intel_dp, false);
> }
>
> static bool
> @@ -3429,7 +3434,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp)
> intel_dp->is_mst = false;
> }
> }
> - edp_panel_vdd_off(intel_dp, false);
> + intel_edp_panel_vdd_off(intel_dp, false);
>
> drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> return intel_dp->is_mst;
> @@ -4527,7 +4532,7 @@ static bool intel_edp_init_connector(struct intel_dp
> *intel_dp,
> /* Cache DPCD and EDID for edp. */
> intel_edp_panel_vdd_on(intel_dp);
> has_dpcd = intel_dp_get_dpcd(intel_dp);
> - edp_panel_vdd_off(intel_dp, false);
> + intel_edp_panel_vdd_off(intel_dp, false);
>
> if (has_dpcd) {
> if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11)
> --
> 1.8.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/14] drm/i915: Use intel_edp_panel_vdd_on() in intel_dp_probe_mst()
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> We want to use the higher level vdd on func here. Not a big deal
> yet (we'd just get the warn when things go awry) but when the
> locking gets fixed this becomes more important.
Reviewed-by: Jani Nikula
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a9ed2a6..28d0183 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3419,7 +3419,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp)
> if (intel_dp->dpcd[DP_DPCD_REV] < 0x12)
> return false;
>
> - _edp_panel_vdd_on(intel_dp);
> + intel_edp_panel_vdd_on(intel_dp);
> if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) {
> if (buf[0] & DP_MST_CAP) {
> DRM_DEBUG_KMS("Sink is MST capable\n");
> --
> 1.8.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/14] drm/i915: Add a note explaining vdd on/off handling in intel_dp_aux_ch()
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Add a comment to explain why we care about the current want_panel_vdd > state in intel_dp_aux_ch(). Reviewed-by: Jani Nikula > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_dp.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 30943a5..19a818f 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -533,6 +533,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > bool has_aux_irq = HAS_AUX_IRQ(dev); > bool vdd; > > + /* > + * We will be called with VDD already enabled for dpcd/edid/oui reads. > + * In such cases we want to leave VDD enabled and it's up to upper > layers > + * to turn it off. But for eg. i2c-dev access we need to turn it on/off > + * ourselves. > + */ > vdd = edp_panel_vdd_on(intel_dp); > > /* dp aux is extremely sensitive to irq latency, hence request the > -- > 1.8.5.5 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/14] drm/i915: Reorganize vlv eDP reboot notifier
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Move the vlv_power_sequencer_pipe() after the IS_VALLEYVIEW() check
> and flatten the rest of the function.
Please imagine adding another platform there, and realize this just adds
unnecessary churn.
BR,
Jani.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dp.c | 24
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 43dd226..a9ed2a6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -347,22 +347,22 @@ static int edp_notify_handler(struct notifier_block
> *this, unsigned long code,
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 pp_div;
> u32 pp_ctrl_reg, pp_div_reg;
> - enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
> + enum pipe pipe;
>
> - if (!is_edp(intel_dp) || code != SYS_RESTART)
> + if (!IS_VALLEYVIEW(dev) || !is_edp(intel_dp) || code != SYS_RESTART)
> return 0;
>
> - if (IS_VALLEYVIEW(dev)) {
> - pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
> - pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
> - pp_div = I915_READ(pp_div_reg);
> - pp_div &= PP_REFERENCE_DIVIDER_MASK;
> + pipe = vlv_power_sequencer_pipe(intel_dp);
>
> - /* 0x1F write to PP_DIV_REG sets max cycle delay */
> - I915_WRITE(pp_div_reg, pp_div | 0x1F);
> - I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
> - msleep(intel_dp->panel_power_cycle_delay);
> - }
> + pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
> + pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
> + pp_div = I915_READ(pp_div_reg);
> + pp_div &= PP_REFERENCE_DIVIDER_MASK;
> +
> + /* 0x1F write to PP_DIV_REG sets max cycle delay */
> + I915_WRITE(pp_div_reg, pp_div | 0x1F);
> + I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
> + msleep(intel_dp->panel_power_cycle_delay);
>
> return 0;
> }
> --
> 1.8.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
