Re: [Intel-gfx] [PATCH] drm/i915: remove pixel doubled HDMI modes from valid modes list
On Tue, Aug 12, 2014 at 07:39:24AM +0100, Chris Wilson wrote: > On Mon, Aug 11, 2014 at 03:33:02PM -0700, clinton.a.tay...@intel.com wrote: > > From: Clint Taylor > > > > Intel HDMI does not correctly configure pixel replicated HDMI modes > > 480i and 576i. Remove support for these modes until DRM has been > > changed to correctly identify SD interlaced modes by reporting there > > true horizontal resolution 720 instead of the pre-pixel doubled 1440. > > > > Signed-off-by: Clint Taylor > > --- > > drivers/gpu/drm/i915/intel_hdmi.c |3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > > b/drivers/gpu/drm/i915/intel_hdmi.c > > index 5f47d35..ab32523 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -873,6 +873,9 @@ intel_hdmi_mode_valid(struct drm_connector *connector, > > if (mode->flags & DRM_MODE_FLAG_DBLSCAN) > > return MODE_NO_DBLESCAN; > > > > If it is just a workaround for a drm bug. You need a big comment > explaining why you w/a rather than pressing for the bug fix. Well there's a very small chance I'll take this anyway - if the drm core is broken we need to take out the mode from the drm core, not here. -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: fix plane/cursor handling when runtime suspended
On Mon, Aug 11, 2014 at 02:57:44PM -0300, Paulo Zanoni wrote:
> 2014-08-11 11:42 GMT-03:00 Ville Syrjälä :
> > On Mon, Aug 11, 2014 at 11:29:21AM -0300, Paulo Zanoni wrote:
> >> 2014-08-11 8:32 GMT-03:00 Ville Syrjälä :
> >> > On Wed, Aug 06, 2014 at 06:26:01PM -0300, Paulo Zanoni wrote:
> >> >> From: Paulo Zanoni
> >> >>
> >> >> If we're runtime suspended and try to use the plane interfaces, we
> >> >> will get a lot of WARNs saying we did the wrong thing.
> >> >>
> >> >> For intel_crtc_update_cursor(), all we need to do is return if the
> >> >> CRTC is not active, since writing the registers won't really have any
> >> >> effect if the screen is not visible, and we will write the registers
> >> >> later when enabling the screen.
> >> >>
> >> >> For all the other cases, we need to get runtime PM references to
> >> >> pin/unpin the objects, and to change the fences. The pin/unpin
> >> >> functions are the ideal places for this, but
> >> >> intel_crtc_cursor_set_obj() doesn't call them, so we also have to add
> >> >> get/put calls inside it. There is no problem if we runtime suspend
> >> >> right after these functions are finished, because the registers
> >> >> weitten are forwarded to system memory.
> >> >>
> >> >> v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel)
> >> >> v3: - Make get/put also surround the fence and unpin calls (Daniel and
> >> >> Ville).
> >> >> - Merge all the plane changes into a single patch since they're
> >> >> the same fix.
> >> >> - Add the comment requested by Daniel.
> >> >>
> >> >> Testcase: igt/pm_rpm/cursor
> >> >> Testcase: igt/pm_rpm/cursor-dpms
> >> >> Testcase: igt/pm_rpm/legacy-planes
> >> >> Testcase: igt/pm_rpm/legacy-planes-dpms
> >> >> Testcase: igt/pm_rpm/universal-planes
> >> >> Testcase: igt/pm_rpm/universal-planes-dpms
> >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645
> >> >> Cc: sta...@vger.kernel.org
> >> >> Signed-off-by: Paulo Zanoni
> >> >> ---
> >> >> drivers/gpu/drm/i915/intel_display.c | 39
> >> >> +++-
> >> >> 1 file changed, 38 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> >> b/drivers/gpu/drm/i915/intel_display.c
> >> >> index 4f659eb..a86d67c 100644
> >> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> >> @@ -2212,6 +2212,15 @@ intel_pin_and_fence_fb_obj(struct drm_device
> >> >> *dev,
> >> >> if (need_vtd_wa(dev) && alignment < 256 * 1024)
> >> >> alignment = 256 * 1024;
> >> >>
> >> >> + /*
> >> >> + * Global gtt pte registers are special registers which actually
> >> >> forward
> >> >> + * writes to a chunk of system memory. Which means that there is
> >> >> no risk
> >> >> + * that the register values disappear as soon as we call
> >> >> + * intel_runtime_pm_put(), so it is correct to wrap only the
> >> >> + * pin/unpin/fence and not more.
> >> >> + */
> >> >> + intel_runtime_pm_get(dev_priv);
> >> >> +
> >> >> dev_priv->mm.interruptible = false;
> >> >> ret = i915_gem_object_pin_to_display_plane(obj, alignment,
> >> >> pipelined);
> >> >> if (ret)
> >> >> @@ -2229,21 +2238,30 @@ intel_pin_and_fence_fb_obj(struct drm_device
> >> >> *dev,
> >> >> i915_gem_object_pin_fence(obj);
> >> >>
> >> >> dev_priv->mm.interruptible = true;
> >> >> + intel_runtime_pm_put(dev_priv);
> >> >> return 0;
> >> >>
> >> >> err_unpin:
> >> >> i915_gem_object_unpin_from_display_plane(obj);
> >> >> err_interruptible:
> >> >> dev_priv->mm.interruptible = true;
> >> >> + intel_runtime_pm_put(dev_priv);
> >> >> return ret;
> >> >> }
> >> >>
> >> >> void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
> >> >> {
> >> >> - WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
> >> >> + struct drm_device *dev = obj->base.dev;
> >> >> + struct drm_i915_private *dev_priv = dev->dev_private;
> >> >> +
> >> >> + WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> >> >> +
> >> >> + intel_runtime_pm_get(dev_priv);
> >> >>
> >> >> i915_gem_object_unpin_fence(obj);
> >> >> i915_gem_object_unpin_from_display_plane(obj);
> >> >> +
> >> >> + intel_runtime_pm_put(dev_priv);
> >> >
> >> > I don't think we touch the hardware during unpin so these aren't
> >> > strictly needed.
> >> >
> >>
> >> Daniel requested them.
> >>
> >>
> >> >> }
> >> >>
> >> >> /* Computes the linear offset to the base tile and adjusts x, y. bytes
> >> >> per pixel
> >> >> @@ -8285,6 +8303,9 @@ static void intel_crtc_update_cursor(struct
> >> >> drm_crtc *crtc,
> >> >> if (base == 0 && intel_crtc->cursor_base == 0)
> >> >> return;
> >> >>
> >> >> + if (!intel_crtc->active)
> >> >> + return;
> >> >> +
> >> >
> >> > Did you actually manage to get by the base==0 check above with a
> >> > disabled pipe? I don't thin
Re: [Intel-gfx] [PATCH] drm/i915: Wait for vblank after enabling the primary plane on BDW
On Mon, Aug 11, 2014 at 04:44:23PM -0300, Paulo Zanoni wrote:
> 2014-06-30 7:10 GMT-03:00 Jani Nikula :
> > On Thu, 26 Jun 2014, Rodrigo Vivi wrote:
> >> I'm sure this might affect Wayne, so, cc'ing him here.
> >>
> >> from my point of view this is right so:
> >> Reviewed-by: Rodrigo Vivi
> >
> > Pushed to -fixes, thanks for the patch and review.
> >
> > BR,
> > Jani.
> >
> >
> >>
> >>
> >> On Tue, Jun 24, 2014 at 3:59 AM, wrote:
> >>
> >>> From: Ville Syrjälä
> >>>
> >>> BDW signals the flip done interrupt immediately after the DSPSURF write
> >>> when the plane is disabled. This is true even if we've already armed
> >>> DSPCNTR to enable the plane at the next vblank. This causes major
> >>> problems for our page flip code which relies on the flip done interrupts
> >>> happening at vblank time.
> >>>
> >>> So what happens is that we enable the plane, and immediately allow
> >>> userspace to submit a page flip. If the plane is still in the process
> >>> of being enabled when the page flip is issued, the flip done gets
> >>> signalled immediately. Our DSPSURFLIVE check catches this to prevent
> >>> premature flip completion, but it also means that we don't get a flip
> >>> done interrupt when the plane actually gets enabled, and so the page
> >>> flip is never completed.
> >>>
> >>> Work around this by re-introducing blocking vblank waits on BDW
> >>> whenever we enable the primary plane.
> >>>
> >>> I removed some of the vblank waits here:
> >>> commit 6304cd91e7f05f8802ea6f91287cac09741d9c46
> >>> Author: Ville Syrjälä
> >>> Date: Fri Apr 25 13:30:12 2014 +0300
> >>>
> >>> drm/i915: Drop the excessive vblank waits from modeset codepaths
> >>>
> >>> To avoid these blocking vblank waits we should start using the vblank
> >>> interrupt instead of the flip done interrupt to complete page flips.
> >>> But that's material for another patch.
> >>>
> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79354
> >>> Tested-by: Guo Jinxian
> >>> Signed-off-by: Ville Syrjälä
> >>> ---
> >>> drivers/gpu/drm/i915/intel_display.c | 9 +
> >>> drivers/gpu/drm/i915/intel_sprite.c | 8
> >>> 2 files changed, 17 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >>> b/drivers/gpu/drm/i915/intel_display.c
> >>> index 9188fed..f92efc6 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -2087,6 +2087,7 @@ void intel_flush_primary_plane(struct
> >>> drm_i915_private *dev_priv,
> >>> static void intel_enable_primary_hw_plane(struct drm_i915_private
> >>> *dev_priv,
> >>> enum plane plane, enum pipe
> >>> pipe)
> >>> {
> >>> + struct drm_device *dev = dev_priv->dev;
> >>> struct intel_crtc *intel_crtc =
> >>> to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> >>> int reg;
> >>> @@ -2106,6 +2107,14 @@ static void intel_enable_primary_hw_plane(struct
> >>> drm_i915_private *dev_priv,
> >>>
> >>> I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE);
> >>> intel_flush_primary_plane(dev_priv, plane);
> >>> +
> >>> + /*
> >>> +* BDW signals flip done immediately if the plane
> >>> +* is disabled, even if the plane enable is already
> >>> +* armed to occur at the next vblank :(
> >>> +*/
> >>> + if (IS_BROADWELL(dev))
> >>> + intel_wait_for_vblank(dev, intel_crtc->pipe);
>
> This chunk triggers "WARN(ret == 0)" from drm_wait_one_vblank when
> using HDMI on BDW.
Are we still calling drm_vblank_off() too soon or something?
>
>
> >>> }
> >>>
> >>> /**
> >>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>> index 1b66ddc..9a17b4e 100644
> >>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>> @@ -691,6 +691,14 @@ intel_post_enable_primary(struct drm_crtc *crtc)
> >>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >>>
> >>> /*
> >>> +* BDW signals flip done immediately if the plane
> >>> +* is disabled, even if the plane enable is already
> >>> +* armed to occur at the next vblank :(
> >>> +*/
> >>> + if (IS_BROADWELL(dev))
> >>> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> >>> +
> >>> + /*
> >>> * FIXME IPS should be fine as long as one plane is
> >>> * enabled, but in practice it seems to have problems
> >>> * when going from primary only to sprite only and vice
> >>> --
> >>> 1.8.5.5
> >>>
> >>> ___
> >>> Intel-gfx mailing list
> >>> Intel-gfx@lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>>
> >>
> >>
> >>
> >> --
> >> Rodrigo Vivi
> >> Blog: http://blog.vivi.eng.br
> >> ___
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.f
[Intel-gfx] [PATCH 1/2] drm/i915/dp: Cache EDID for a detection cycle
As we may query the edid multiple times following a detect, record the
EDID found during output discovery and reuse it. This is a separate
issue from caching the output EDID across detection cycles.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_dp.c | 95
drivers/gpu/drm/i915/intel_drv.h | 1 +
2 files changed, 29 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ed37407..1eef55c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3764,27 +3764,10 @@ intel_dp_get_edid(struct drm_connector *connector,
struct i2c_adapter *adapter)
return drm_get_edid(connector, adapter);
}
-static int
-intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter
*adapter)
-{
- struct intel_connector *intel_connector = to_intel_connector(connector);
-
- /* use cached edid if we have one */
- if (intel_connector->edid) {
- /* invalid edid */
- if (IS_ERR(intel_connector->edid))
- return 0;
-
- return intel_connector_update_modes(connector,
- intel_connector->edid);
- }
-
- return intel_ddc_get_modes(connector, adapter);
-}
-
static enum drm_connector_status
intel_dp_detect(struct drm_connector *connector, bool force)
{
+ struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_dp *intel_dp = intel_attached_dp(connector);
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct intel_encoder *intel_encoder = &intel_dig_port->base;
@@ -3795,21 +3778,20 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
struct edid *edid = NULL;
bool ret;
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
-
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
connector->base.id, connector->name);
+ kfree(intel_connector->detect_edid);
+ intel_connector->detect_edid = NULL;
if (intel_dp->is_mst) {
/* MST devices are disconnected from a monitor POV */
if (intel_encoder->type != INTEL_OUTPUT_EDP)
intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
- status = connector_status_disconnected;
- goto out;
+ return connector_status_disconnected;
}
- intel_dp->has_audio = false;
+ power_domain = intel_display_port_power_domain(intel_encoder);
+ intel_display_power_get(dev_priv, power_domain);
if (HAS_PCH_SPLIT(dev))
status = ironlake_dp_detect(intel_dp);
@@ -3831,15 +3813,13 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
goto out;
}
- if (intel_dp->force_audio != HDMI_AUDIO_AUTO) {
+ edid = intel_dp_get_edid(connector, &intel_dp->aux.ddc);
+ intel_connector->detect_edid = edid;
+
+ if (intel_dp->force_audio != HDMI_AUDIO_AUTO)
intel_dp->has_audio = (intel_dp->force_audio == HDMI_AUDIO_ON);
- } else {
- edid = intel_dp_get_edid(connector, &intel_dp->aux.ddc);
- if (edid) {
- intel_dp->has_audio = drm_detect_monitor_audio(edid);
- kfree(edid);
- }
- }
+ else
+ intel_dp->has_audio = drm_detect_monitor_audio(edid);
if (intel_encoder->type != INTEL_OUTPUT_EDP)
intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
@@ -3852,61 +3832,41 @@ out:
static int intel_dp_get_modes(struct drm_connector *connector)
{
- struct intel_dp *intel_dp = intel_attached_dp(connector);
- struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct intel_connector *intel_connector = to_intel_connector(connector);
- struct drm_device *dev = connector->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- enum intel_display_power_domain power_domain;
- int ret;
-
- /* We should parse the EDID data and find out if it has an audio sink
-*/
-
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
+ struct edid *edid;
- ret = intel_dp_get_edid_modes(connector, &intel_dp->aux.ddc);
- intel_display_power_put(dev_priv, power_domain);
- if (ret)
- return ret;
+ edid = intel_connector->detect_edid;
+ if (edid) {
+ int ret = intel_connector_update_modes(connector, edid);
+ if (ret)
+ return ret;
+ }
/* if eDP has no EDID, fall back to fixed mo
[Intel-gfx] [PATCH 2/2] drm/i915/hdmi: Cache EDID for a detection cycle
As we may query the edid multiple times following a detect, record the
EDID found during output discovery and reuse it. This is a separate
issue from caching the output EDID across detection cycles.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_hdmi.c | 68 ---
1 file changed, 21 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 9169786..f765527 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -988,17 +988,17 @@ intel_hdmi_detect(struct drm_connector *connector, bool
force)
intel_gmbus_get_adapter(dev_priv,
intel_hdmi->ddc_bus));
- if (edid) {
- if (edid->input & DRM_EDID_INPUT_DIGITAL) {
- status = connector_status_connected;
- if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
- intel_hdmi->has_hdmi_sink =
- drm_detect_hdmi_monitor(edid);
- intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
- intel_hdmi->rgb_quant_range_selectable =
- drm_rgb_quant_range_selectable(edid);
- }
- kfree(edid);
+ kfree(to_intel_connector(connector)->detect_edid);
+ to_intel_connector(connector)->detect_edid = edid;
+
+ if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
+ status = connector_status_connected;
+ if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
+ intel_hdmi->has_hdmi_sink =
+ drm_detect_hdmi_monitor(edid);
+ intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
+ intel_hdmi->rgb_quant_range_selectable =
+ drm_rgb_quant_range_selectable(edid);
}
if (status == connector_status_connected) {
@@ -1015,51 +1015,24 @@ intel_hdmi_detect(struct drm_connector *connector, bool
force)
static int intel_hdmi_get_modes(struct drm_connector *connector)
{
- struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
- struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
- struct drm_i915_private *dev_priv = connector->dev->dev_private;
- enum intel_display_power_domain power_domain;
- int ret;
-
- /* We should parse the EDID data and find out if it's an HDMI sink so
-* we can send audio to it.
-*/
-
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
-
- ret = intel_ddc_get_modes(connector,
- intel_gmbus_get_adapter(dev_priv,
-
intel_hdmi->ddc_bus));
+ struct edid *edid;
- intel_display_power_put(dev_priv, power_domain);
+ edid = to_intel_connector(connector)->detect_edid;
+ if (edid == NULL)
+ return 0;
- return ret;
+ return intel_connector_update_modes(connector, edid);
}
static bool
intel_hdmi_detect_audio(struct drm_connector *connector)
{
- struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
- struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
- struct drm_i915_private *dev_priv = connector->dev->dev_private;
- enum intel_display_power_domain power_domain;
- struct edid *edid;
bool has_audio = false;
+ struct edid *edid;
- power_domain = intel_display_port_power_domain(intel_encoder);
- intel_display_power_get(dev_priv, power_domain);
-
- edid = drm_get_edid(connector,
- intel_gmbus_get_adapter(dev_priv,
- intel_hdmi->ddc_bus));
- if (edid) {
- if (edid->input & DRM_EDID_INPUT_DIGITAL)
- has_audio = drm_detect_monitor_audio(edid);
- kfree(edid);
- }
-
- intel_display_power_put(dev_priv, power_domain);
+ edid = to_intel_connector(connector)->detect_edid;
+ if (edid && edid->input & DRM_EDID_INPUT_DIGITAL)
+ has_audio = drm_detect_monitor_audio(edid);
return has_audio;
}
@@ -1479,6 +1452,7 @@ static void chv_hdmi_pre_enable(struct intel_encoder
*encoder)
static void intel_hdmi_destroy(struct drm_connector *connector)
{
+ kfree(to_intel_connector(connector)->detect_edid);
drm_connector_cleanup(connector);
kfree(connector);
}
--
1.9.1
___
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/hdmi: Cache EDID for a detection cycle
On Tue, Aug 12, 2014 at 09:36:04AM +0100, Chris Wilson wrote: > As we may query the edid multiple times following a detect, record the > EDID found during output discovery and reuse it. This is a separate > issue from caching the output EDID across detection cycles. As reference, for a couple of DP monitors attached, "time xrandr" goes from 0.268s to 0.142s -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/15] drm/i915: Move i915_gem_chipset_flush to where it belongs
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Daniel Vetter
> Sent: Wednesday, August 06, 2014 2:05 PM
> To: Intel Graphics Development
> Cc: Daniel Vetter
> Subject: [Intel-gfx] [PATCH 03/15] drm/i915: Move i915_gem_chipset_flush
> to where it belongs
>
> Include depency hell ftw! So need to move this into a real function.
>
> Also fix up the header include order in i915_drv.h: The rule is to
> always include core headers first, then local stuff.
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++
> drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index d349dd75ed69..194367f0ba1a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2556,13 +2556,6 @@ int __must_check
> i915_gem_evict_something(struct drm_device *dev,
> int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);
> int i915_gem_evict_everything(struct drm_device *dev);
>
> -/* belongs in i915_gem_gtt.h */
> -static inline void i915_gem_chipset_flush(struct drm_device *dev)
> -{
> - if (INTEL_INFO(dev)->gen < 6)
> - intel_gtt_chipset_flush();
> -}
> -
> /* i915_gem_stolen.c */
> int i915_gem_init_stolen(struct drm_device *dev);
> int i915_gem_stolen_setup_compression(struct drm_device *dev, int size,
> int fb_cpp);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 1a20e1b4f052..baa94199239b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2031,6 +2031,12 @@ static void gen6_gmch_remove(struct
> i915_address_space *vm)
> teardown_scratch_page(vm->dev);
> }
>
> +void i915_gem_chipset_flush(struct drm_device *dev)
> +{
> + if (INTEL_INFO(dev)->gen < 6)
> + intel_gtt_chipset_flush();
> +}
> +
> static int i915_gmch_probe(struct drm_device *dev,
> size_t *gtt_total,
> size_t *stolen,
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
> b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 57d401343e8d..380e034c66f8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -294,4 +294,6 @@ void i915_gem_restore_gtt_mappings(struct
> drm_device *dev);
> int __must_check i915_gem_gtt_prepare_object(struct
> drm_i915_gem_object *obj);
> void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
>
> +void i915_gem_chipset_flush(struct drm_device *dev);
> +
> #endif
> --
> 1.9.3
Reviewed-by: Michel Thierry
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/15] drm/i915: Allow i915_gem_setup_global_gtt to fail
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Daniel Vetter
> Sent: Wednesday, August 06, 2014 2:05 PM
> To: Intel Graphics Development
> Cc: Daniel Vetter
> Subject: [Intel-gfx] [PATCH 07/15] drm/i915: Allow
> i915_gem_setup_global_gtt to fail
>
> We already needs this just as a safety check in case the preallocation
> reservation dance fails. But we definitely need this to be able to
> move tha aliasing ppgtt setup back out of the context code to this
> place, where it belongs.
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_gem.c | 7 ++-
> drivers/gpu/drm/i915/i915_gem_gtt.c | 16 ++--
> drivers/gpu/drm/i915/i915_gem_gtt.h | 4 ++--
> 3 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c
> index f4e57fe05c6a..d8399ee622b9 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4762,7 +4762,12 @@ int i915_gem_init(struct drm_device *dev)
> DRM_DEBUG_DRIVER("allow wake ack timed
> out\n");
> }
>
> - i915_gem_init_userptr(dev);
> + ret = i915_gem_init_userptr(dev);
> + if (ret) {
> + mutex_unlock(&dev->struct_mutex);
> + return ret;
> + }
> +
> i915_gem_init_global_gtt(dev);
>
> ret = i915_gem_context_init(dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 3753bf184865..4fa7807ed4d5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1709,10 +1709,10 @@ static void i915_gtt_color_adjust(struct
> drm_mm_node *node,
> }
> }
>
> -void i915_gem_setup_global_gtt(struct drm_device *dev,
> -unsigned long start,
> -unsigned long mappable_end,
> -unsigned long end)
> +int i915_gem_setup_global_gtt(struct drm_device *dev,
> + unsigned long start,
> + unsigned long mappable_end,
> + unsigned long end)
> {
> /* Let GEM Manage all of the aperture.
>*
> @@ -1745,8 +1745,10 @@ void i915_gem_setup_global_gtt(struct
> drm_device *dev,
>
> WARN_ON(i915_gem_obj_ggtt_bound(obj));
> ret = drm_mm_reserve_node(&ggtt_vm->mm, &vma-
> >node);
> - if (ret)
> - DRM_DEBUG_KMS("Reservation failed\n");
> + if (ret) {
> + DRM_DEBUG_KMS("Reservation failed: %i\n", ret);
> + return ret;
> + }
> obj->has_global_gtt_mapping = 1;
> }
>
> @@ -1763,6 +1765,8 @@ void i915_gem_setup_global_gtt(struct
> drm_device *dev,
>
> /* And finally clear the reserved guard page */
> ggtt_vm->clear_range(ggtt_vm, end - PAGE_SIZE, PAGE_SIZE, true);
> +
> + return 0;
> }
>
> void i915_gem_init_global_gtt(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
> b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 0b04ef6167f8..bea3541d5525 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -271,8 +271,8 @@ struct i915_hw_ppgtt {
>
> int i915_gem_gtt_init(struct drm_device *dev);
> void i915_gem_init_global_gtt(struct drm_device *dev);
> -void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long
> start,
> -unsigned long mappable_end, unsigned long
> end);
> +int i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long
> start,
> + unsigned long mappable_end, unsigned long
> end);
>
> bool intel_enable_ppgtt(struct drm_device *dev, bool full);
>
Reviewed-by: Michel Thierry
> --
> 1.9.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Rework ppgtt init to no require an aliasing ppgtt
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Daniel Vetter
> Sent: Wednesday, August 06, 2014 7:20 PM
> To: Intel Graphics Development
> Cc: Daniel Vetter
> Subject: [Intel-gfx] [PATCH 1/2] drm/i915: Rework ppgtt init to no require
an
> aliasing ppgtt
>
> Currently we abuse the aliasing ppgtt to set up the ppgtt support in
> general. Which is a bit backwards since with full ppgtt we don't ever
> need the aliasing ppgtt.
>
> So untangling this and separate the ppgtt init from the aliasing
> ppgtt. While at it drag it out of the context enabling (which just
> does a switch to the default context).
>
> Note that we still have the differentiation between synchronous and
> asynchronous ppgtt setup, but that will soon vanish. So also correctly
> wire up the return value handling to be prepared for when ->switch_mm
> drops the synchronous parameter and could start to fail.
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_gem.c | 8
> drivers/gpu/drm/i915/i915_gem_context.c | 7 ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 84
+-
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
> 4 files changed, 42 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c
> index a79deb189146..caf92bac2152 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4742,6 +4742,14 @@ i915_gem_init_hw(struct drm_device *dev)
> if (ret && ret != -EIO) {
> DRM_ERROR("Context enable failed %d\n", ret);
> i915_gem_cleanup_ringbuffer(dev);
> +
> + return ret;
> + }
> +
> + ret = i915_ppgtt_init_hw(dev);
> + if (ret && ret != -EIO) {
> + DRM_ERROR("PPGTT enable failed %d\n", ret);
> + i915_gem_cleanup_ringbuffer(dev);
> }
>
> return ret;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 3b8367aa8404..4af5f05b24e8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -424,13 +424,6 @@ int i915_gem_context_enable(struct
> drm_i915_private *dev_priv)
> struct intel_engine_cs *ring;
> int ret, i;
>
> - /* This is the only place the aliasing PPGTT gets enabled, which
> means
> - * it has to happen before we bail on reset */
> - if (dev_priv->mm.aliasing_ppgtt) {
> - struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
> - ppgtt->enable(ppgtt);
> - }
> -
> /* FIXME: We should make this work, even in reset */
> if (i915_reset_in_progress(&dev_priv->gpu_error))
> return 0;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 4fa7807ed4d5..bf70ab44b968 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -78,7 +78,6 @@ static void ppgtt_bind_vma(struct i915_vma *vma,
> enum i915_cache_level cache_level,
> u32 flags);
> static void ppgtt_unbind_vma(struct i915_vma *vma);
> -static int gen8_ppgtt_enable(struct i915_hw_ppgtt *ppgtt);
>
> static inline gen8_gtt_pte_t gen8_pte_encode(dma_addr_t addr,
>enum i915_cache_level level,
> @@ -615,7 +614,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt
> *ppgtt, uint64_t size)
> kunmap_atomic(pd_vaddr);
> }
>
> - ppgtt->enable = gen8_ppgtt_enable;
> ppgtt->switch_mm = gen8_mm_switch;
> ppgtt->base.clear_range = gen8_ppgtt_clear_range;
> ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
> @@ -836,39 +834,20 @@ static int gen6_mm_switch(struct i915_hw_ppgtt
> *ppgtt,
> return 0;
> }
>
> -static int gen8_ppgtt_enable(struct i915_hw_ppgtt *ppgtt)
> +static void gen8_ppgtt_enable(struct drm_device *dev)
> {
> - struct drm_device *dev = ppgtt->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_engine_cs *ring;
> - int j, ret;
> + int j;
>
> for_each_ring(ring, dev_priv, j) {
> I915_WRITE(RING_MODE_GEN7(ring),
> _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
> -
> - /* We promise to do a switch later with FULL PPGTT. If this
is
> - * aliasing, this is the one and only switch we'll do */
> - if (USES_FULL_PPGTT(dev))
> - continue;
> -
> - ret = ppgtt->switch_mm(ppgtt, ring, true);
> - if (ret)
> - goto err_out;
> }
> -
> - return 0;
> -
> -err_out:
> - for_each_ring(ring, dev_priv, j)
> - I915_WRITE(RING_MODE_GEN7(ring),
> -_MASKED_BIT_DISABLE(GFX_PPGTT_ENABLE));
> - return ret;
> }
>
> -static int gen7_pp
[Intel-gfx] [PATCH] Revert "drm/i915: Drop forcewake w/a for missed interrupts/seqno on Sandybridge"
This reverts commit 67e5871be82fec1451801d448b51d9a403d1ffac.
The missed interrupts are back, in a big fashion. I am observing stalls
followed by the missing interrupt warning on every Sandybridge+ machine
I have (a mixture of Sandybridge, Ivybridge and Haswell).
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index db9c64b..e5d1845 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1282,6 +1282,11 @@ gen6_ring_get_irq(struct intel_engine_cs *ring)
if (!dev->irq_enabled)
return false;
+ /* It looks like we need to prevent the gt from suspending while waiting
+* for an notifiy irq, otherwise irqs seem to get lost on at least the
+* blt/bsd rings on ivb. */
+ gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
+
spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (ring->irq_refcount++ == 0) {
if (HAS_L3_DPF(dev) && ring->id == RCS)
@@ -1313,6 +1318,8 @@ gen6_ring_put_irq(struct intel_engine_cs *ring)
gen5_disable_gt_irq(dev_priv, ring->irq_enable_mask);
}
spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+
+ gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
}
static bool
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/10] tests/kms_cursor_crc: Align opening {
Signed-off-by: Daniel Vetter
---
tests/kms_cursor_crc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index bbbf053d3207..723a6d556b54 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -396,8 +396,7 @@ static void test_cursor_size(data_t *data)
static void run_test_generic(data_t *data, int cursor_max_size)
{
int cursor_size;
- for (cursor_size = 64; cursor_size <= 256; cursor_size *= 2)
- {
+ for (cursor_size = 64; cursor_size <= 256; cursor_size *= 2) {
igt_fixture
igt_require(cursor_max_size >= cursor_size);
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/10] lib/igt_kms: Shuffle kmtests_ functions
Group them a bit both in the header and .c file, and make sure they
appear in the same order in both.
Signed-off-by: Daniel Vetter
---
lib/igt_kms.c | 372 +-
lib/igt_kms.h | 19 +--
2 files changed, 196 insertions(+), 195 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index e28d7a825683..76812a2dff12 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -227,34 +227,6 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id)
return pfci.pipe;
}
-void kmstest_set_connector_dpms(int fd, drmModeConnector *connector, int mode)
-{
- int i, dpms = 0;
- bool found_it = false;
-
- for (i = 0; i < connector->count_props; i++) {
- struct drm_mode_get_property prop;
-
- prop.prop_id = connector->props[i];
- prop.count_values = 0;
- prop.count_enum_blobs = 0;
- if (drmIoctl(fd, DRM_IOCTL_MODE_GETPROPERTY, &prop))
- continue;
-
- if (strcmp(prop.name, "DPMS"))
- continue;
-
- dpms = prop.prop_id;
- found_it = true;
- break;
- }
- igt_assert_f(found_it, "DPMS property not found on %d\n",
-connector->connector_id);
-
- igt_assert(drmModeConnectorSetProperty(fd, connector->connector_id,
- dpms, mode) == 0);
-}
-
static signed long set_vt_mode(unsigned long mode)
{
int fd;
@@ -310,6 +282,115 @@ void kmstest_set_vt_graphics_mode(void)
orig_vt_mode = ret;
}
+static int get_card_number(int fd)
+{
+ struct stat buf;
+
+ /* find the minor number of the device */
+ fstat(fd, &buf);
+
+ return minor(buf.st_rdev) & 0x3f;
+}
+
+static char* get_debugfs_connector_path(int drm_fd, drmModeConnector
*connector,
+ const char *file)
+{
+ char *path;
+
+ asprintf(&path, "/sys/kernel/debug/dri/%d/%s-%d/%s",
+get_card_number(drm_fd),
+kmstest_connector_type_str(connector->connector_type),
+connector->connector_type_id,
+file);
+
+ return path;
+}
+
+/**
+ * kmstest_force_connector:
+ * @fd: drm file descriptor
+ * @connector: connector
+ * @state: state to force on @connector
+ *
+ * Force the specified state on the specified connector.
+ *
+ * Returns: true on success
+ */
+bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
+enum kmstest_force_connector_state state)
+{
+ char *path;
+ const char *value;
+ int debugfs_fd, ret;
+
+ switch (state) {
+ case FORCE_CONNECTOR_ON:
+ value = "on";
+ break;
+ case FORCE_CONNECTOR_DIGITAL:
+ value = "digital";
+ break;
+ case FORCE_CONNECTOR_OFF:
+ value = "off";
+ break;
+
+ default:
+ case FORCE_CONNECTOR_UNSPECIFIED:
+ value = "unspecified";
+ break;
+ }
+
+ path = get_debugfs_connector_path(drm_fd, connector, "force");
+ debugfs_fd = open(path, O_WRONLY | O_TRUNC);
+ free(path);
+
+ if (debugfs_fd == -1) {
+ return false;
+ }
+
+ ret = write(debugfs_fd, value, strlen(value));
+ close(debugfs_fd);
+
+ igt_assert(ret != -1);
+ return (ret == -1) ? false : true;
+}
+
+/**
+ * kmstest_force_edid:
+ * @drm_fd: drm file descriptor
+ * @connector: connector to set @edid on
+ * @edid: An EDID data block
+ * @length: length of the EDID data. #EDID_LENGTH defines the standard EDID
+ * length
+ *
+ * Set the EDID data on @connector to @edid. See #generic_edid and
+ * #kmstest_generic_edid for a set of generic EDID data blocks.
+ *
+ * If @length is zero, the forced EDID will be removed.
+ */
+void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
+ const unsigned char *edid, size_t length)
+{
+ char *path;
+ int debugfs_fd, ret;
+
+ path = get_debugfs_connector_path(drm_fd, connector, "edid_override");
+
+ debugfs_fd = open(path, O_WRONLY | O_TRUNC);
+
+ free(path);
+
+ igt_assert(debugfs_fd != -1);
+
+ if (length == 0)
+ ret = write(debugfs_fd, "reset", 5);
+ else
+ ret = write(debugfs_fd, edid, length);
+ close(debugfs_fd);
+
+ igt_assert(ret != -1);
+}
+
int kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
drmModeModeInfo *mode)
{
@@ -423,120 +504,109 @@ err1:
return -1;
}
-static int get_card_number(int fd)
+void kmstest_free_connector_config(struct kmstest_connector_config *config)
{
- struct stat buf;
-
- /* find the minor number of the device */
- fstat(fd, &buf);
-
- return minor(buf.st_rdev) &
[Intel-gfx] [PATCH 02/10] docs: drop igt_edid again from the documentation
It's not documented and there's really not a lot to it either. We can always readd when it's growing into something bigger. Signed-off-by: Daniel Vetter --- docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml b/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml index 3d9caf89b075..68ca8d4dbe65 100644 --- a/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml +++ b/docs/reference/intel-gpu-tools/intel-gpu-tools-docs.xml @@ -25,7 +25,6 @@ - -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/10] lib/igt_kms: set_vt_graphics_mode is a low-level helper
So give it a kmstest_ prefix and shuffle it around a bit.
Signed-off-by: Daniel Vetter
---
lib/igt_kms.c | 14 +-
lib/igt_kms.h | 9 ++---
tests/kms_cursor_crc.c | 2 +-
tests/kms_fbc_crc.c | 2 +-
tests/kms_fence_pin_leak.c | 2 +-
tests/kms_flip.c| 2 +-
tests/kms_flip_event_leak.c | 2 +-
tests/kms_flip_tiling.c | 2 +-
tests/kms_force_connector.c | 2 +-
tests/kms_mmio_vs_cs_flip.c | 2 +-
tests/kms_pipe_crc_basic.c | 2 +-
tests/kms_plane.c | 2 +-
tests/kms_psr_sink_crc.c| 2 +-
tests/kms_render.c | 2 +-
tests/kms_rotation_crc.c| 2 +-
tests/kms_setmode.c | 2 +-
tests/kms_sink_crc_basic.c | 2 +-
tests/kms_universal_plane.c | 2 +-
tests/pm_lpsp.c | 2 +-
tests/pm_rpm.c | 2 +-
tests/testdisplay.c | 2 +-
21 files changed, 34 insertions(+), 27 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index fec859c01e97..e28d7a825683 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -287,12 +287,16 @@ static void restore_vt_mode_at_exit(int sig)
set_vt_mode(orig_vt_mode);
}
-/*
- * Set the VT to graphics mode and install an exit handler to restore the
- * original mode.
+/**
+ * kmstest_set_vt_graphics_mode:
+ *
+ * Sets the controlling VT (if available) into graphics/raw mode and installs
an
+ * igt exit handler to set the VT back to text mode on exit.
+ *
+ * All kms tests must call this function to make sure that the fbcon doesn't
+ * interfere by e.g. blanking the screen.
*/
-
-void igt_set_vt_graphics_mode(void)
+void kmstest_set_vt_graphics_mode(void)
{
long ret;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 342208a5c86e..09f15455396d 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -36,6 +36,8 @@
#include "igt_fb.h"
+/* Low-level helpers with kmstest_ prefix */
+
enum pipe {
PIPE_A = 0,
PIPE_B,
@@ -163,6 +165,10 @@ void kmstest_unset_all_crtcs(int drm_fd, drmModeResPtr
resources);
* A small modeset API
*/
+/* set vt into graphics mode, required to prevent fbcon from interfering */
+void kmstest_set_vt_graphics_mode(void);
+
+/* High-level kms api with igt_ prefix */
enum igt_commit_style {
COMMIT_LEGACY = 0,
COMMIT_UNIVERSAL,
@@ -239,9 +245,6 @@ struct igt_display {
bool has_universal_planes;
};
-/* set vt into graphics mode, required to prevent fbcon from interfering */
-void igt_set_vt_graphics_mode(void);
-
void igt_display_init(igt_display_t *display, int drm_fd);
void igt_display_fini(igt_display_t *display);
int igt_display_commit2(igt_display_t *display, enum igt_commit_style s);
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index e724073287ae..6af30fda40af 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -440,7 +440,7 @@ igt_main
/* We assume width and height are same so max is assigned width
*/
igt_assert_cmpint(cursor_width, ==, cursor_height);
- igt_set_vt_graphics_mode();
+ kmstest_set_vt_graphics_mode();
igt_require_pipe_crc();
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 4302d1b4b54a..9aeffd09a145 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -499,7 +499,7 @@ igt_main
FILE *status;
data.drm_fd = drm_open_any();
- igt_set_vt_graphics_mode();
+ kmstest_set_vt_graphics_mode();
data.devid = intel_get_drm_devid(data.drm_fd);
diff --git a/tests/kms_fence_pin_leak.c b/tests/kms_fence_pin_leak.c
index 6a05d977217c..51bc72f118bf 100644
--- a/tests/kms_fence_pin_leak.c
+++ b/tests/kms_fence_pin_leak.c
@@ -216,7 +216,7 @@ igt_simple_main
data.devid = intel_get_drm_devid(data.drm_fd);
- igt_set_vt_graphics_mode();
+ kmstest_set_vt_graphics_mode();
data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
igt_assert(data.bufmgr);
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 92f4eb52ebe0..45f1ba2fc9f4 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1596,7 +1596,7 @@ int main(int argc, char **argv)
igt_enable_connectors();
- igt_set_vt_graphics_mode();
+ kmstest_set_vt_graphics_mode();
igt_install_exit_handler(kms_flip_exit_handler);
get_timestamp_format();
diff --git a/tests/kms_flip_event_leak.c b/tests/kms_flip_event_leak.c
index 99243335a17b..8cbec00e0f4b 100644
--- a/tests/kms_flip_event_leak.c
+++ b/tests/kms_flip_event_leak.c
@@ -115,7 +115,7 @@ igt_simple_main
igt_skip_on_simulation();
data.drm_fd = drm_open_any();
- igt_set_vt_graphics_mode();
+ kmstest_set_vt_graphics_mode();
igt_display_init(&data.display, data.drm_fd);
diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
index ca20ad96bc35..6f8ee94133
[Intel-gfx] [PATCH 04/10] lib/igt_kms: Clean up the other _name functions/macros
And remove sprite_name, redundant and won't work due to lack of
dev_priv.
Signed-off-by: Daniel Vetter
---
lib/igt_kms.c | 40 ++--
lib/igt_kms.h | 13 +
2 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index bae43eea174d..fec859c01e97 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -87,6 +87,26 @@ const char *kmstest_pipe_name(enum pipe pipe)
return str[pipe];
}
+/**
+ * kmstest_plane_name:
+ * @plane: display plane
+ *
+ * Returns: String represnting @pipe, e.g. "plane1".
+ */
+const char *kmstest_plane_name(enum igt_plane plane)
+{
+ static const char *names[] = {
+ [IGT_PLANE_1] = "plane1",
+ [IGT_PLANE_2] = "plane2",
+ [IGT_PLANE_3] = "plane3",
+ [IGT_PLANE_CURSOR] = "cursor",
+ };
+
+ igt_assert(plane < ARRAY_SIZE(names) && names[plane]);
+
+ return names[plane];
+}
+
struct type_name {
int type;
const char *name;
@@ -164,6 +184,12 @@ static const char *mode_stereo_name(const drmModeModeInfo
*mode)
}
}
+/**
+ * kmstest_dump_mode:
+ * @mode: libdrm mode structure
+ *
+ * Prints @mode to stdout in a huma-readable form.
+ */
void kmstest_dump_mode(drmModeModeInfo *mode)
{
const char *stereo = mode_stereo_name(mode);
@@ -509,20 +535,6 @@ void kmstest_free_connector_config(struct
kmstest_connector_config *config)
drmModeFreeConnector(config->connector);
}
-const char *plane_name(enum igt_plane p)
-{
- static const char *names[] = {
- [IGT_PLANE_1] = "plane1",
- [IGT_PLANE_2] = "plane2",
- [IGT_PLANE_3] = "plane3",
- [IGT_PLANE_CURSOR] = "cursor",
- };
-
- igt_assert(p < ARRAY_SIZE(names) && names[p]);
-
- return names[p];
-}
-
/*
* A small modeset API
*/
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 0e15b7dde26d..fd6a0e5bf78a 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -51,9 +51,7 @@ enum igt_plane {
IGT_PLANE_CURSOR,
};
-const char *plane_name(enum igt_plane p);
-
-#define sprite_name(p, s) ((p) * dev_priv->num_plane + (s) + 'A')
+const char *kmstest_plane_name(enum igt_plane plane);
enum port {
PORT_A = 0,
@@ -63,7 +61,14 @@ enum port {
PORT_E,
I915_MAX_PORTS
};
-#define port_name(p) ((p) + 'A')
+
+/**
+ * kmstest_port_name:
+ * @port: display plane
+ *
+ * Returns: String represnting @port, e.g. "A".
+ */
+#define kmstest_port_name(port) ((port) + 'A')
enum igt_commit_style {
COMMIT_LEGACY = 0,
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/10] lib/igt_kms: Simplify return value of kmstest_get_connector_config
A plain bool is enough.
Signed-off-by: Daniel Vetter
---
lib/igt_kms.c | 15 ---
lib/igt_kms.h | 7 +++
tests/kms_flip.c | 12 ++--
tests/kms_psr_sink_crc.c | 11 ---
tests/kms_render.c | 6 ++
tests/kms_sink_crc_basic.c | 10 --
tests/testdisplay.c| 4 ++--
7 files changed, 29 insertions(+), 36 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 9af0201abde7..258a48a9ed4d 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -414,9 +414,9 @@ bool kmstest_get_connector_default_mode(int drm_fd,
drmModeConnector *connector,
return true;
}
-int kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
-unsigned long crtc_idx_mask,
-struct kmstest_connector_config *config)
+bool kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
+ unsigned long crtc_idx_mask,
+ struct kmstest_connector_config *config)
{
drmModeRes *resources;
drmModeConnector *connector;
@@ -493,7 +493,7 @@ found:
drmModeFreeResources(resources);
- return 0;
+ return true;
err4:
drmModeFreeEncoder(encoder);
err3:
@@ -501,7 +501,7 @@ err3:
err2:
drmModeFreeResources(resources);
err1:
- return -1;
+ return false;
}
void kmstest_free_connector_config(struct kmstest_connector_config *config)
@@ -651,7 +651,7 @@ static void igt_display_log_shift(igt_display_t *display,
int shift)
static void igt_output_refresh(igt_output_t *output)
{
igt_display_t *display = output->display;
- int ret;
+ bool ret;
unsigned long crtc_idx_mask;
/* we mask out the pipes already in use */
@@ -659,11 +659,12 @@ static void igt_output_refresh(igt_output_t *output)
if (output->valid)
kmstest_free_connector_config(&output->config);
+
ret = kmstest_get_connector_config(display->drm_fd,
output->id,
crtc_idx_mask,
&output->config);
- if (ret == 0)
+ if (ret)
output->valid = true;
else
output->valid = false;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 01bbf5f53b9b..754d8568d709 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -103,7 +103,6 @@ void kmstest_dump_mode(drmModeModeInfo *mode);
int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id);
void kmstest_set_vt_graphics_mode(void);
-
struct kmstest_connector_config {
drmModeCrtc *crtc;
drmModeConnector *connector;
@@ -154,9 +153,9 @@ void kmstest_force_edid(int drm_fd, drmModeConnector
*connector,
bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector
*connector,
drmModeModeInfo *mode);
-int kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
-unsigned long crtc_idx_mask,
-struct kmstest_connector_config *config);
+bool kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
+ unsigned long crtc_idx_mask,
+ struct kmstest_connector_config *config);
void kmstest_free_connector_config(struct kmstest_connector_config *config);
void kmstest_set_connector_dpms(int fd, drmModeConnector *connector, int mode);
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 45f1ba2fc9f4..5cd8c2f96090 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -989,8 +989,8 @@ static void connector_find_preferred_mode(uint32_t
connector_id, int crtc_idx,
{
struct kmstest_connector_config config;
- if (kmstest_get_connector_config(drm_fd, connector_id, 1 << crtc_idx,
-&config) < 0) {
+ if (!kmstest_get_connector_config(drm_fd, connector_id, 1 << crtc_idx,
+ &config)) {
o->mode_valid = 0;
return;
}
@@ -1032,12 +1032,12 @@ static void connector_find_compatible_mode(int
crtc_idx0, int crtc_idx1,
drmModeModeInfo *mode[2];
int n, m;
- if (kmstest_get_connector_config(drm_fd, o->_connector[0],
-1 << crtc_idx0, &config[0]) < 0)
+ if (!kmstest_get_connector_config(drm_fd, o->_connector[0],
+ 1 << crtc_idx0, &config[0]))
return;
- if (kmstest_get_connector_config(drm_fd, o->_connector[1],
-1 << crtc_idx1, &config[1]) < 0) {
+ if (!kmstest_get_connector_config(drm_fd, o->_connector[1],
+ 1 << crtc_idx1, &config[1])) {
kmstest_free_connector_config(&config[0]);
[Intel-gfx] [PATCH 03/10] lib/igt_kms: Unify pipe name helpers
And add api doc while at it.
Signed-off-by: Daniel Vetter
---
lib/igt_debugfs.c | 8 +++---
lib/igt_kms.c | 61 ++---
lib/igt_kms.h | 3 +--
tests/kms_cursor_crc.c | 10 +---
tests/kms_fbc_crc.c | 15 ++-
tests/kms_pipe_crc_basic.c | 4 +--
tests/kms_plane.c | 23 +
tests/kms_psr_sink_crc.c| 2 +-
tests/kms_render.c | 2 +-
tests/kms_setmode.c | 2 +-
tests/kms_universal_plane.c | 12 ++---
11 files changed, 80 insertions(+), 62 deletions(-)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index d4a6cf617ff5..f6f509dd1875 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -294,7 +294,7 @@ static bool igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
{
char buf[64];
- sprintf(buf, "pipe %c %s", pipe_name(pipe_crc->pipe),
+ sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc->pipe),
pipe_crc_source_name(pipe_crc->source));
errno = 0;
write(pipe_crc->ctl_fd, buf, strlen(buf));
@@ -308,7 +308,7 @@ static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
{
char buf[32];
- sprintf(buf, "pipe %c none", pipe_name(pipe));
+ sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe));
write(fd, buf, strlen(buf));
}
@@ -379,7 +379,7 @@ igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source
source)
pipe_crc->ctl_fd = igt_debugfs_open("i915_display_crc_ctl", O_WRONLY);
igt_assert(pipe_crc->ctl_fd != -1);
- sprintf(buf, "i915_pipe_%c_crc", pipe_name(pipe));
+ sprintf(buf, "i915_pipe_%s_crc", kmstest_pipe_name(pipe));
pipe_crc->crc_fd = igt_debugfs_open(buf, O_RDONLY);
igt_assert(pipe_crc->crc_fd != -1);
@@ -445,7 +445,7 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
{
char buf[32];
- sprintf(buf, "pipe %c none", pipe_name(pipe_crc->pipe));
+ sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc->pipe));
write(pipe_crc->ctl_fd, buf, strlen(buf));
}
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index cccb0377b16c..bae43eea174d 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -70,7 +70,14 @@
* Note that this library's header pulls in the [i-g-t
framebuffer](intel-gpu-tools-i-g-t-framebuffer.html)
* library as a dependency.
*/
-const char *kmstest_pipe_str(int pipe)
+
+/**
+ * kmstest_pipe_name:
+ * @pipe: display pipe
+ *
+ * Returns: String represnting @pipe, e.g. "A".
+ */
+const char *kmstest_pipe_name(enum pipe pipe)
{
const char *str[] = { "A", "B", "C" };
@@ -586,8 +593,8 @@ static void igt_output_refresh(igt_output_t *output)
c->connector_type_id);
}
- LOG(display, "%s: Selecting pipe %c\n", output->name,
- pipe_name(output->config.pipe));
+ LOG(display, "%s: Selecting pipe %s\n", output->name,
+ kmstest_pipe_name(output->config.pipe));
display->pipes_in_use |= 1 << output->config.pipe;
}
@@ -902,9 +909,9 @@ static void igt_display_refresh(igt_display_t *display)
igt_assert_f(a->pending_crtc_idx_mask !=
b->pending_crtc_idx_mask,
- "%s and %s are both trying to use pipe
%c\n",
+ "%s and %s are both trying to use pipe
%s\n",
igt_output_name(a), igt_output_name(b),
- pipe_name(ffs(a->pending_crtc_idx_mask) -
1));
+
kmstest_pipe_name(ffs(a->pending_crtc_idx_mask) - 1));
}
}
@@ -1018,9 +1025,9 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
if (plane->fb_changed && fb_id == 0) {
LOG(display,
- "%s: SetPlane pipe %c, plane %d, disabling\n",
+ "%s: SetPlane pipe %s, plane %d, disabling\n",
igt_output_name(output),
- pipe_name(output->config.pipe),
+ kmstest_pipe_name(output->config.pipe),
plane->index);
ret = drmModeSetPlane(display->drm_fd,
@@ -1038,9 +1045,9 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
CHECK_RETURN(ret, fail_on_error);
} else if (plane->fb_changed || plane->position_changed) {
LOG(display,
- "%s: SetPlane %c.%d, fb %u, position (%d, %d)\n",
+ "%s: SetPlane %s.%d, fb %u, position (%d, %d)\n",
igt_output_name(output),
- pipe_name(output->config.pipe),
+ kmstest_pipe_name(output->config.pipe),
plane->index,
fb_id,
plane->crtc_x, plane->crtc_y);
@@ -1092,9 +1099,9 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
[Intel-gfx] [PATCH 10/10] lib/igt_kms: doc for the remaining kmstest_ functions
Plus a bit an overview section explaining the split in the library - a
few people (everyone except me it seems) didn't really understand it.
Signed-off-by: Daniel Vetter
---
lib/igt_kms.c | 54 --
1 file changed, 52 insertions(+), 2 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 258a48a9ed4d..50a3ebdb6bb6 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -64,8 +64,16 @@
*
* This library provides support to enumerate and set modeset configurations.
*
- * Since this library is very much still a work-in-progress and the interfaces
- * still in-flux detailed api documentation is currently still missing.
+ * There are two parts in this library: First the low level helper function
+ * which directly build on top of raw ioctls or the interfaces provided by
+ * libdrm. Those functions all have a kmstest_ prefix.§
+ *
+ * The second part is a high-level library to manage modeset configurations
+ * which abstracts away some of the low-level details like the difference
+ * between legacy and universal plane support for setting cursors or in the
+ * future the difference between legacy and atomic commit. These high-level
+ * functions have all igt_ prefixes. This part is still very much work in
+ * progress and so also lacks a bit documentation for the individual functions.
*
* Note that this library's header pulls in the [i-g-t
framebuffer](intel-gpu-tools-i-g-t-framebuffer.html)
* library as a dependency.
@@ -214,6 +222,14 @@ void kmstest_dump_mode(drmModeModeInfo *mode)
fflush(stdout);
}
+/**
+ * kmstest_set_vt_graphics_mode:
+ * @fd: DRM fd
+ * @crtc_id: DRM CRTC id
+ *
+ * Returns: The pipe number for the given DRM CRTC @crtc_id. This maps directly
+ * to an enum pipe value used in other helper functions.
+ */
int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id)
{
struct drm_i915_get_pipe_from_crtc_id pfci;
@@ -391,6 +407,16 @@ void kmstest_force_edid(int drm_fd, drmModeConnector
*connector,
igt_assert(ret != -1);
}
+/**
+ * kmstest_get_connector_default_mode:
+ * @drm_fd: DRM fd
+ * @connector: libdrm connector
+ * @mode: libdrm mode
+ *
+ * Retrieves the default mode for @connector and stores it in @mode.
+ *
+ * Returns: true on success, false on failure
+ */
bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector
*connector,
drmModeModeInfo *mode)
{
@@ -414,6 +440,16 @@ bool kmstest_get_connector_default_mode(int drm_fd,
drmModeConnector *connector,
return true;
}
+/**
+ * kmstest_get_connector_config:
+ * @drm_fd: DRM fd
+ * @connector_id: DRM connector id
+ * @crtc_idx_mask: mask of allowed DRM CRTC indices
+ * @config: structure filled with the possible configuration
+ *
+ * This tries to find a suitable configuration for the given connector and CRTC
+ * constraint and fills it into @config.
+ */
bool kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
unsigned long crtc_idx_mask,
struct kmstest_connector_config *config)
@@ -504,6 +540,12 @@ err1:
return false;
}
+/**
+ * kmstest_free_connector_config:
+ * @config: connector configuration structure
+ *
+ * Free any resources in @config allocated in kmstest_get_connector_config().
+ */
void kmstest_free_connector_config(struct kmstest_connector_config *config)
{
drmModeFreeCrtc(config->crtc);
@@ -511,6 +553,14 @@ void kmstest_free_connector_config(struct
kmstest_connector_config *config)
drmModeFreeConnector(config->connector);
}
+/**
+ * kmstest_set_connector_dpms:
+ * @fd: DRM fd
+ * @connector: libdrm connector
+ * @mode: DRM DPMS value
+ *
+ * This function sets the DPMS setting of @connector to @mode.
+ */
void kmstest_set_connector_dpms(int fd, drmModeConnector *connector, int mode)
{
int i, dpms = 0;
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/10] lib/igt_kms: doc for type2str functions
Also shuffle things around a bit to make sure the order in the header
matches the order in the .c file.
Signed-off-by: Daniel Vetter
---
lib/igt_kms.h | 53 +
1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index fd6a0e5bf78a..342208a5c86e 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -34,6 +34,8 @@
#include
+#include "igt_fb.h"
+
enum pipe {
PIPE_A = 0,
PIPE_B,
@@ -70,21 +72,29 @@ enum port {
*/
#define kmstest_port_name(port) ((port) + 'A')
-enum igt_commit_style {
- COMMIT_LEGACY = 0,
- COMMIT_UNIVERSAL,
- /* We'll add atomic here eventually. */
-};
+/**
+ * kmstest_encoder_type_str:
+ * @type: DRM_MODE_ENCODER_* enumeration value
+ *
+ * Returns: A string representing the drm encoder @type.
+ */
+const char *kmstest_encoder_type_str(int type);
-typedef enum {
- /* this maps to the kernel API */
- IGT_ROTATION_0 = 1 << 0,
- IGT_ROTATION_90 = 1 << 1,
- IGT_ROTATION_180 = 1 << 2,
- IGT_ROTATION_270 = 1 << 3,
-} igt_rotation_t;
+/**
+ * kmstest_connector_status_str:
+ * @status: DRM_MODE_* connector status value
+ *
+ * Returns: A string representing the drm connector status @status.
+ */
+const char *kmstest_connector_status_str(int status);
-#include "igt_fb.h"
+/**
+ * kmstest_connector_type_str:
+ * @type: DRM_MODE_CONNECTOR_* enumeration value
+ *
+ * Returns: A string representing the drm connector @type.
+ */
+const char *kmstest_connector_type_str(int type);
struct kmstest_connector_config {
drmModeCrtc *crtc;
@@ -143,9 +153,6 @@ void kmstest_free_connector_config(struct
kmstest_connector_config *config);
void kmstest_dump_mode(drmModeModeInfo *mode);
int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id);
-const char *kmstest_encoder_type_str(int type);
-const char *kmstest_connector_status_str(int type);
-const char *kmstest_connector_type_str(int type);
void kmstest_set_connector_dpms(int fd, drmModeConnector *connector, int mode);
bool kmstest_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
const char *name, uint32_t *prop_id, uint64_t *value,
@@ -156,10 +163,24 @@ void kmstest_unset_all_crtcs(int drm_fd, drmModeResPtr
resources);
* A small modeset API
*/
+enum igt_commit_style {
+ COMMIT_LEGACY = 0,
+ COMMIT_UNIVERSAL,
+ /* We'll add atomic here eventually. */
+};
+
typedef struct igt_display igt_display_t;
typedef struct igt_pipe igt_pipe_t;
typedef uint32_t igt_fixed_t; /* 16.16 fixed point */
+typedef enum {
+ /* this maps to the kernel API */
+ IGT_ROTATION_0 = 1 << 0,
+ IGT_ROTATION_90 = 1 << 1,
+ IGT_ROTATION_180 = 1 << 2,
+ IGT_ROTATION_270 = 1 << 3,
+} igt_rotation_t;
+
typedef struct {
/*< private >*/
igt_pipe_t *pipe;
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/10] lib/igt_kms: Simplify return value of kmstest_get_connector_default_mode
A plain bool is good enough, no need for fancy negative error values.
Signed-off-by: Daniel Vetter
---
lib/igt_kms.c | 12 ++--
lib/igt_kms.h | 4 ++--
tests/kms_setmode.c | 4 ++--
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 76812a2dff12..9af0201abde7 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -391,15 +391,15 @@ void kmstest_force_edid(int drm_fd, drmModeConnector
*connector,
igt_assert(ret != -1);
}
-int kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
- drmModeModeInfo *mode)
+bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector
*connector,
+ drmModeModeInfo *mode)
{
int i;
if (!connector->count_modes) {
fprintf(stderr, "no modes for connector %d\n",
connector->connector_id);
- return -1;
+ return false;
}
for (i = 0; i < connector->count_modes; i++) {
@@ -411,7 +411,7 @@ int kmstest_get_connector_default_mode(int drm_fd,
drmModeConnector *connector,
}
}
- return 0;
+ return true;
}
int kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
@@ -480,8 +480,8 @@ int kmstest_get_connector_config(int drm_fd, uint32_t
connector_id,
goto err3;
found:
- if (kmstest_get_connector_default_mode(drm_fd, connector,
- &config->default_mode) < 0)
+ if (!kmstest_get_connector_default_mode(drm_fd, connector,
+ &config->default_mode))
goto err4;
config->connector = connector;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 0acfeba1e491..01bbf5f53b9b 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -152,8 +152,8 @@ bool kmstest_force_connector(int fd, drmModeConnector
*connector,
void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
const unsigned char *edid, size_t length);
-int kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
- drmModeModeInfo *mode);
+bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector
*connector,
+ drmModeModeInfo *mode);
int kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
unsigned long crtc_idx_mask,
struct kmstest_connector_config *config);
diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index f4197854b355..59e2434ccd50 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -517,8 +517,8 @@ static int get_one_connector(drmModeRes *resources, int
connector_id,
}
if (cconf->connected) {
- if (kmstest_get_connector_default_mode(drm_fd, connector,
- &mode) < 0)
+ if (!kmstest_get_connector_default_mode(drm_fd, connector,
+ &mode))
mode = mode_640_480;
} else {
mode = mode_640_480;
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 1/1] drm/i915: Sharing Gfx Clock, Wake and Gunit save/restore logic using common handler for runtime/system s/r paths
From: Sagar Kamble
v1:
Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of
gunit registers need to be followed in
PM suspend and resume path similar to runtime suspend and resume.
v2:
1. Keeping GT access, wake, gunit save/restore related helpers static.
2. Moved GT access check, Wake Control, Gunit state save to end of
i915_drm_freeze.
3. Reusing the sequence in runtime_suspend/resume path at macro level.
v3:
1. Prepared common handlers for platform specific tasks to be done before HW
suspend and after HW resume from
D0i3.
2. Changed commit header.
Cc: Imre Deak
Cc: Paulo Zanoni
Cc: Daniel Vetter
Cc: Jani Nikula
Cc: Goel, Akash
Change-Id: I15cfdeeec9c976d9839bb281f809664f4a0c78a2
Signed-off-by: Sagar Kamble
---
drivers/gpu/drm/i915/i915_drv.c | 130 ++--
1 file changed, 85 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ec96f9a..4440722 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -494,6 +494,11 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
return true;
}
+
+static int pre_hw_suspend_deinit(struct drm_i915_private *dev_priv);
+static int post_hw_resume_init(struct drm_i915_private *dev_priv,
+ bool resume_from_rpm_suspend);
+
static int i915_drm_freeze(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -614,15 +619,16 @@ void intel_console_resume(struct work_struct *work)
static int i915_drm_thaw_early(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ int ret = 0;
- if (IS_HASWELL(dev) || IS_BROADWELL(dev))
- hsw_disable_pc8(dev_priv);
+ /* Restore any platform specific registers/clk state */
+ ret = post_hw_resume_init(dev_priv, false);
intel_uncore_early_sanitize(dev, true);
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
- return 0;
+ return ret;
}
static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
@@ -908,6 +914,7 @@ static int i915_pm_suspend_late(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
struct drm_device *drm_dev = pci_get_drvdata(pdev);
struct drm_i915_private *dev_priv = drm_dev->dev_private;
+ int ret = 0;
/*
* We have a suspedn ordering issue with the snd-hda driver also
@@ -921,13 +928,13 @@ static int i915_pm_suspend_late(struct device *dev)
if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
return 0;
- if (IS_HASWELL(drm_dev) || IS_BROADWELL(drm_dev))
- hsw_enable_pc8(dev_priv);
+ /* Save any platform specific registers/clk state needed post resume */
+ ret = pre_hw_suspend_deinit(dev_priv);
pci_disable_device(pdev);
pci_set_power_state(pdev, PCI_D3hot);
- return 0;
+ return ret;
}
static int i915_pm_resume_early(struct device *dev)
@@ -983,23 +990,26 @@ static int i915_pm_poweroff(struct device *dev)
return i915_drm_freeze(drm_dev);
}
-static int hsw_runtime_suspend(struct drm_i915_private *dev_priv)
+static int hsw_suspend(struct drm_i915_private *dev_priv)
{
hsw_enable_pc8(dev_priv);
return 0;
}
-static int snb_runtime_resume(struct drm_i915_private *dev_priv)
+static int snb_resume(struct drm_i915_private *dev_priv,
+ bool resume_from_rpm_suspend)
{
struct drm_device *dev = dev_priv->dev;
- intel_init_pch_refclk(dev);
+ if (resume_from_rpm_suspend)
+ intel_init_pch_refclk(dev);
return 0;
}
-static int hsw_runtime_resume(struct drm_i915_private *dev_priv)
+static int hsw_resume(struct drm_i915_private *dev_priv,
+ bool resume_from_rpm_suspend)
{
hsw_disable_pc8(dev_priv);
@@ -1295,10 +1305,10 @@ static void vlv_check_no_gt_access(struct
drm_i915_private *dev_priv)
I915_WRITE(VLV_GTLC_PW_STATUS, VLV_GTLC_ALLOWWAKEERR);
}
-static int vlv_runtime_suspend(struct drm_i915_private *dev_priv)
+static int vlv_suspend(struct drm_i915_private *dev_priv)
{
u32 mask;
- int err;
+ int ret = 0;
/*
* Bspec defines the following GT well on flags as debug only, so
@@ -1311,20 +1321,19 @@ static int vlv_runtime_suspend(struct drm_i915_private
*dev_priv)
vlv_check_no_gt_access(dev_priv);
- err = vlv_force_gfx_clock(dev_priv, true);
- if (err)
+ ret = vlv_force_gfx_clock(dev_priv, true);
+ if (ret)
goto err1;
- err = vlv_allow_gt_wake(dev_priv, false);
- if (err)
+ ret = vlv_allow_gt_wake(dev_priv, false);
+ if (ret)
goto err2;
vlv_save_gunit_s0ix_state(dev_priv);
- err = vlv_force_gfx_clock
Re: [Intel-gfx] [PATCH] drm: HDMI pixel replication modes now hactive of 720 for pixel replication
On Tue, Jul 29, 2014 at 02:58:23PM -0700, clinton.a.tay...@intel.com wrote:
> From: Clint Taylor
>
> CEA SD interlaced modes use a horizontal 720 pixels that are pixel replicated
> to 1440. The current driver reports 1440 pixel to the OS and does not set
> pixel replicated modes.
>
> Signed-off-by: Clint Taylor
> ---
> drivers/gpu/drm/drm_edid.c| 68
> ++---
> drivers/gpu/drm/i915/intel_hdmi.c | 13 +++
> 2 files changed, 47 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index dfa9769..5233f4c 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -632,26 +632,26 @@ 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,
> + /* 6 - 720(1440)x480i@60Hz */
> + { DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 27000, 720, 1478,
I think the best thing here might be to halve all the horizontal
timings (and the clock), and maybe do it with an explicit /2 to
remind people that this is a bit special, and also the spec lists
the doubled timings, so it's maybe a bit easier to compare with the
spec that way.
So perhaps something like this:
DRM_MODE(..., 27000/2, 1440/2, 1478/2, ...
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 2422413..f8cdf7f 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -918,6 +918,19 @@ bool intel_hdmi_compute_config(struct intel_encoder
> *encoder,
> intel_hdmi->color_range = 0;
> }
>
> + /* Adjust pipe timings for pixel doubled modes */
> + if ((adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)) {
> + adjusted_mode->hsync_start /= 2;
> + adjusted_mode->hsync_end /= 2;
> + adjusted_mode->htotal /= 2;
> +
> + drm_mode_set_crtcinfo(adjusted_mode, 0);
> +
> + /* Set 2x pixel double on pipe */
> + pipe_config->pixel_multiplier = 2;
> + pipe_config->port_clock = adjusted_mode->crtc_clock;
> + }
If you halve the timings already in drm_edid.c all of this would be
reduced to just:
if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
pipe_config->pixel_multiplier = 2;
> +
> if (intel_hdmi->color_range)
> pipe_config->limited_color_range = true;
>
> --
> 1.7.9.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] Revert "drm/i915: Drop forcewake w/a for missed interrupts/seqno on Sandybridge"
On Tue, Aug 12, 2014 at 10:24:58AM +0100, Chris Wilson wrote: > This reverts commit 67e5871be82fec1451801d448b51d9a403d1ffac. > > The missed interrupts are back, in a big fashion. I am observing stalls > followed by the missing interrupt warning on every Sandybridge+ machine > I have (a mixture of Sandybridge, Ivybridge and Haswell). Bad news, this alleviated the issue on ivb, but I am still hitting it on snb. Oh well, back to the drawing board to figure out what's changed. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/1] drm/i915: Sharing Gfx Clock, Wake and Gunit save/restore logic using common handler for runtime/system s/r paths
On Tue, Aug 12, 2014 at 04:21:55PM +0530, sagar.a.kam...@intel.com wrote:
> From: Sagar Kamble
>
> v1:
> Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of
> gunit registers need to be followed in
> PM suspend and resume path similar to runtime suspend and resume.
>
> v2:
> 1. Keeping GT access, wake, gunit save/restore related helpers static.
> 2. Moved GT access check, Wake Control, Gunit state save to end of
> i915_drm_freeze.
> 3. Reusing the sequence in runtime_suspend/resume path at macro level.
>
> v3:
> 1. Prepared common handlers for platform specific tasks to be done before HW
> suspend and after HW resume from
> D0i3.
> 2. Changed commit header.
>
> Cc: Imre Deak
> Cc: Paulo Zanoni
> Cc: Daniel Vetter
> Cc: Jani Nikula
> Cc: Goel, Akash
> Change-Id: I15cfdeeec9c976d9839bb281f809664f4a0c78a2
> Signed-off-by: Sagar Kamble
Yeah, this looks roughly like what I'd expected, thanks.
I think there's some room now to consolidate hw deinit/init code further
between system s/r and rpm paths, but at least we've started. Please find
one of the rpm domain experts to review this so I can merge it.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c | 130
> ++--
> 1 file changed, 85 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ec96f9a..4440722 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -494,6 +494,11 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
> return true;
> }
>
> +
> +static int pre_hw_suspend_deinit(struct drm_i915_private *dev_priv);
> +static int post_hw_resume_init(struct drm_i915_private *dev_priv,
> + bool resume_from_rpm_suspend);
> +
> static int i915_drm_freeze(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -614,15 +619,16 @@ void intel_console_resume(struct work_struct *work)
> static int i915_drm_thaw_early(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> + int ret = 0;
>
> - if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> - hsw_disable_pc8(dev_priv);
> + /* Restore any platform specific registers/clk state */
> + ret = post_hw_resume_init(dev_priv, false);
>
> intel_uncore_early_sanitize(dev, true);
> intel_uncore_sanitize(dev);
> intel_power_domains_init_hw(dev_priv);
>
> - return 0;
> + return ret;
> }
>
> static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
> @@ -908,6 +914,7 @@ static int i915_pm_suspend_late(struct device *dev)
> struct pci_dev *pdev = to_pci_dev(dev);
> struct drm_device *drm_dev = pci_get_drvdata(pdev);
> struct drm_i915_private *dev_priv = drm_dev->dev_private;
> + int ret = 0;
>
> /*
>* We have a suspedn ordering issue with the snd-hda driver also
> @@ -921,13 +928,13 @@ static int i915_pm_suspend_late(struct device *dev)
> if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> return 0;
>
> - if (IS_HASWELL(drm_dev) || IS_BROADWELL(drm_dev))
> - hsw_enable_pc8(dev_priv);
> + /* Save any platform specific registers/clk state needed post resume */
> + ret = pre_hw_suspend_deinit(dev_priv);
>
> pci_disable_device(pdev);
> pci_set_power_state(pdev, PCI_D3hot);
>
> - return 0;
> + return ret;
> }
>
> static int i915_pm_resume_early(struct device *dev)
> @@ -983,23 +990,26 @@ static int i915_pm_poweroff(struct device *dev)
> return i915_drm_freeze(drm_dev);
> }
>
> -static int hsw_runtime_suspend(struct drm_i915_private *dev_priv)
> +static int hsw_suspend(struct drm_i915_private *dev_priv)
> {
> hsw_enable_pc8(dev_priv);
>
> return 0;
> }
>
> -static int snb_runtime_resume(struct drm_i915_private *dev_priv)
> +static int snb_resume(struct drm_i915_private *dev_priv,
> + bool resume_from_rpm_suspend)
> {
> struct drm_device *dev = dev_priv->dev;
>
> - intel_init_pch_refclk(dev);
> + if (resume_from_rpm_suspend)
> + intel_init_pch_refclk(dev);
>
> return 0;
> }
>
> -static int hsw_runtime_resume(struct drm_i915_private *dev_priv)
> +static int hsw_resume(struct drm_i915_private *dev_priv,
> + bool resume_from_rpm_suspend)
> {
> hsw_disable_pc8(dev_priv);
>
> @@ -1295,10 +1305,10 @@ static void vlv_check_no_gt_access(struct
> drm_i915_private *dev_priv)
> I915_WRITE(VLV_GTLC_PW_STATUS, VLV_GTLC_ALLOWWAKEERR);
> }
>
> -static int vlv_runtime_suspend(struct drm_i915_private *dev_priv)
> +static int vlv_suspend(struct drm_i915_private *dev_priv)
> {
> u32 mask;
> - int err;
> + int ret = 0;
>
> /*
>* Bspec defines the following GT well on flags as debug only, so
> @@ -131
Re: [Intel-gfx] [PATCH 2/4] drm/i915: cancel hotplug and dig_port work during suspend and unload
On Mon, Aug 11, 2014 at 09:54:15PM +0300, Imre Deak wrote:
> Make sure these work handlers don't run after we system suspend or
> unload the driver. Note that we don't cancel the handlers during runtime
> suspend. That could lead to a lockup, since we take a runtime PM ref
> from the handlers themselves. Fortunaltely canceling there is not needed
> since the RPM ref itself provides for the needed serialization.
>
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/i915_drv.c | 8
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 3 +--
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ec96f9a..0653761 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -494,6 +494,13 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
> return true;
> }
>
> +void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
> +{
> + cancel_work_sync(&dev_priv->hotplug_work);
> + cancel_work_sync(&dev_priv->dig_port_work);
Since dig_port_work can queue a hotplug_work shouldn't these two be
swapped?
I wonder if we should also clear hpd_event_bits and
{long,short}_hpd_port_mask before cancelling the works? At least it
might make the works end a bit quicker if the are already running.
I also noticed that we don't seem to grab any rpm/powerwell references
in ->hpd_pulse() or i915_digport_work_func(). That doesn't seem right.
Or maybe you already addressed that in another patch?
> + cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work);
> +}
> +
> static int i915_drm_freeze(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -538,6 +545,7 @@ static int i915_drm_freeze(struct drm_device *dev)
> flush_delayed_work(&dev_priv->rps.delayed_resume_work);
>
> intel_runtime_pm_disable_interrupts(dev);
> + intel_hpd_cancel_work(dev_priv);
>
> intel_suspend_gt_powersave(dev);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 35d150a..8776847 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2218,6 +2218,7 @@ extern void i915_update_gfx_val(struct drm_i915_private
> *dev_priv);
> int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
>
> extern void intel_console_resume(struct work_struct *work);
> +void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
>
> /* i915_irq.c */
> void i915_queue_hangcheck(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b3a3168..ab90301 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13100,8 +13100,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
>* experience fancy races otherwise.
>*/
> drm_irq_uninstall(dev);
> - cancel_work_sync(&dev_priv->hotplug_work);
> - cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work);
> + intel_hpd_cancel_work(dev_priv);
> dev_priv->pm._irqs_disabled = true;
>
> /*
> --
> 1.8.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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 4/4] drm/i915: don't try to retrain a DP link on an inactive CRTC
On Mon, Aug 11, 2014 at 10:04:51PM +0300, Imre Deak wrote:
> Atm we may retrain the DP link even if the CRTC is inactive through
> HPD work->intel_dp_check_link_status(). This in turn can lock up the PHY
> (at least on BYT), since the DP port is disabled.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81948
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/intel_dp.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d7f5d0a..49de9be 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3551,6 +3551,9 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
> if (WARN_ON(!intel_encoder->base.crtc))
> return;
>
> + if (!to_intel_crtc(intel_encoder->base.crtc)->active)
> + return;
Yeah the connectors_active check isn't enough since we don't clear that
when we disable the crtcs for system suspend. Maybe we should do that
instead?
Anyway this check seems fine to me regardless of how we deal with
connectors_active during system suspend, so:
Reviewed-by: Ville Syrjälä
> +
> /* Try to read receiver status if the link appears to be up */
> if (!intel_dp_get_link_status(intel_dp, link_status)) {
> return;
> --
> 1.8.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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 0/2] HDMI detect optimization and support for HDMI compliance
From: Shashank Sharma This patch set adds 2 patches: 1. drm/i915: Optimize HDMI EDID reads This patch adds a EDID caching solution in intel_hdmi_detect function to avoid multiple EDID reads for single HPD. A delayed work function makes sure that the cached EDID gets clear after a time out. 2. drm/i915: Support for HDMI complaince HPD This patch adds support for HDMI analyzers. Most of the analyzers send a soft HPD event, where the HDMI cable removal is not required. In such scenarios, DDC is always up so EDID is always available, for both HPD up and down. So we have to rely on live status register to check the actual HPD status. Shashank Sharma (2): drm/i915: Optimize HDMI EDID reads drm/i915: Support for HDMI complaince HPD drivers/gpu/drm/i915/i915_drv.h| 1 + drivers/gpu/drm/i915/i915_params.c | 6 ++ drivers/gpu/drm/i915/intel_drv.h | 5 + drivers/gpu/drm/i915/intel_hdmi.c | 181 +++-- 4 files changed, 186 insertions(+), 7 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Optimize HDMI EDID reads
From: Shashank Sharma
The current hdmi_detect() function is getting called from
many places, few of these are:
1. HDMI hot plug interrupt bottom half
2. get_resources() IOCTL family
3. drm_helper_probe_single_connector_modes() family
4. output_poll_execute()
5. status_show() etc...
Every time this function is called, it goes and reads HDMI EDID over
DDC channel. Ideally, reading EDID is only required when there is a
real hot plug, and then for all IOCTL and userspace detect functions
can be answered using this same EDID.
The current patch adds EDID caching for a finite duration (1 minute)
This is how it works:
1. With in this caching duration, when usespace get_resource and other
modeset_detect calls get called, we can use the cached EDID.
2. Even the get_mode function can use the cached EDID.
3. A delayed work will clear the cached EDID after the timeout.
4. If there is a real HDMI hotplug, within the caching duration, the
cached EDID is updates, and a new delayed work is scheduled.
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/intel_drv.h | 4 ++
drivers/gpu/drm/i915/intel_hdmi.c | 92 ---
2 files changed, 90 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 28d185d..185a45a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -110,6 +110,8 @@
#define INTEL_DSI_VIDEO_MODE 0
#define INTEL_DSI_COMMAND_MODE 1
+#define INTEL_HDMI_EDID_CACHING_SEC 60
+
struct intel_framebuffer {
struct drm_framebuffer base;
struct drm_i915_gem_object *obj;
@@ -507,6 +509,8 @@ struct intel_hdmi {
enum hdmi_force_audio force_audio;
bool rgb_quant_range_selectable;
enum hdmi_picture_aspect aspect_ratio;
+ struct edid *edid;
+ struct delayed_work edid_work;
void (*write_infoframe)(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 5f47d35..8dc3970 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -962,6 +962,22 @@ bool intel_hdmi_compute_config(struct intel_encoder
*encoder,
return true;
}
+/* Work function to invalidate EDID caching */
+static void intel_hdmi_invalidate_edid(struct work_struct *work)
+{
+ struct intel_hdmi *intel_hdmi = container_of(to_delayed_work(work),
+ struct intel_hdmi, edid_work);
+ struct drm_device *dev = intel_hdmi_to_dev(intel_hdmi);
+ struct drm_mode_config *mode_config = &dev->mode_config;
+
+ mutex_lock(&mode_config->mutex);
+ /* Checkpatch says kfree is NULL protected */
+ kfree(intel_hdmi->edid);
+ intel_hdmi->edid = NULL;
+ mutex_unlock(&mode_config->mutex);
+ DRM_DEBUG_DRIVER("cleaned up cached EDID\n");
+}
+
static enum drm_connector_status
intel_hdmi_detect(struct drm_connector *connector, bool force)
{
@@ -978,15 +994,58 @@ intel_hdmi_detect(struct drm_connector *connector, bool
force)
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
connector->base.id, connector->name);
+ /*
+ * hdmi_detect() gets called from both get_resource()
+ * and HDMI hpd bottom half work function.
+ * Its not required to read EDID for every detect call until it's is
+ * from a hot plug. Lets cache the EDID as soon as we get
+ * a HPD, and then try to re-use this for all the non hpd detact calls
+ * coming with in a finite duration.
+ */
+ if (INTEL_INFO(dev)->gen < 6)
+ /* Do not break old platforms */
+ goto skip_optimization;
+
+ /* If call is from HPD, do check real status by reading EDID */
+ if (!force)
+ goto skip_optimization;
+
+ /* This is a non-hpd call, lets see if we can optimize this */
+ if (intel_hdmi->edid) {
+ /*
+ * So this is a non-hpd call, within the duration when
+ * EDID caching is valid. So previous status (valid)
+ * of connector is re-usable.
+ */
+ if (connector->status != connector_status_unknown) {
+ DRM_DEBUG_DRIVER("Reporting force status\n");
+ return connector->status;
+ }
+ }
+
+skip_optimization:
power_domain = intel_display_port_power_domain(intel_encoder);
intel_display_power_get(dev_priv, power_domain);
intel_hdmi->has_hdmi_sink = false;
intel_hdmi->has_audio = false;
intel_hdmi->rgb_quant_range_selectable = false;
+
+ /*
+ * You are well deserving, dear code, as you have survived
+ * all the optimizations. Now go and enjoy reading EDID
+ */
edid = drm_get_edid(connec
[Intel-gfx] [PATCH 2/2] drm/i915: Support for HDMI complaince HPD
From: Shashank Sharma
During the HDMI complaince tests, most of the HDMI analyzers
issue a soft HPD, to automate the tests. This process keeps
the HDMI cable connected, and DDC chhanel alive.
HDMI detect() logic relies on EDID readability, to decide if
its a HDMI connect or disconnect event. But in this case, the
EDID is always readable, as HDMI cable is physically connected
and DDC channel is UP, so detect() always reports a HDMI connect
even if its intended to be a HDMI disconnect call.
So if HDMI compliance is enabled, we should rely on the live status
register, than EDID availability. This patch adds:
1. One kernel command line parameter for i915 module, which indicates
if we want to support HDMI compliance, for this platform.
2. A new function to read EDID, which gets called only in case of
HDMI compliance support is required. This function reads EDID only
if live status register is up. The normal code flow doesn't get effected
if kernel command line parameter is not enabled.
3. After various experiments on VLV2 board, with various HDMI monitors
we have seen that, with few monitors, the live status register gets
set after a slight delay, and then stays reliably. To support such
monitors, there is a busy-loop added, with a max delay upto 50ms,
with a status check after every 10ms. Please see the comment in
intel_hdmi_get_edid.
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/i915_drv.h| 1 +
drivers/gpu/drm/i915/i915_params.c | 6 +++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_hdmi.c | 91 +-
4 files changed, 97 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2a372f2..19e4f97 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2162,6 +2162,7 @@ struct i915_params {
bool disable_vtd_wa;
int use_mmio_flip;
bool mmio_debug;
+ bool hdmi_compliance_support;
};
extern struct i915_params i915 __read_mostly;
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index 62ee830..27dcc1a 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -50,6 +50,7 @@ struct i915_params i915 __read_mostly = {
.disable_vtd_wa = 0,
.use_mmio_flip = 0,
.mmio_debug = 0,
+ .hdmi_compliance_support = 0,
};
module_param_named(modeset, i915.modeset, int, 0400);
@@ -167,3 +168,8 @@ module_param_named(mmio_debug, i915.mmio_debug, bool, 0600);
MODULE_PARM_DESC(mmio_debug,
"Enable the MMIO debug code (default: false). This may negatively "
"affect performance.");
+
+module_param_named(hdmi_compliance_support, i915.hdmi_compliance_support,
+ bool, 0400);
+MODULE_PARM_DESC(hdmi_compliance_support,
+"Support for HDMI compliance in i915 driver 1=On 0=Off");
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 185a45a..0b3798a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -111,6 +111,7 @@
#define INTEL_DSI_COMMAND_MODE 1
#define INTEL_HDMI_EDID_CACHING_SEC 60
+#define INTEL_HDMI_LIVE_STATUS_DELAY_MS 10
struct intel_framebuffer {
struct drm_framebuffer base;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 8dc3970..06eb9de 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -978,6 +978,90 @@ static void intel_hdmi_invalidate_edid(struct work_struct
*work)
DRM_DEBUG_DRIVER("cleaned up cached EDID\n");
}
+/* Get HDMI live status */
+static bool intel_hdmi_live_status(struct drm_device *dev,
+ struct intel_hdmi *intel_hdmi)
+{
+ uint32_t status_bit;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_digital_port *intel_dig_port =
+ hdmi_to_dig_port(intel_hdmi);
+
+ DRM_DEBUG_KMS("Reading Live status\n");
+
+ /* Live status bit is 29 for PORT_B, 28 for C and 27 for D */
+ status_bit = (PORTB_HOTPLUG_LIVE_STATUS_VLV >>
+ (PORT_B - intel_dig_port->port));
+
+ return I915_READ(PORT_HOTPLUG_STAT) & status_bit;
+}
+
+/* Read DDC and get EDID */
+struct edid *intel_hdmi_get_edid(struct drm_connector *connector, bool force)
+{
+ bool live_status = false;
+ struct edid *new_edid = NULL;
+ struct i2c_adapter *adapter;
+ struct drm_device *dev = connector->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+ u8 retry = 5;
+
+ if (!intel_hdmi) {
+ DRM_ERROR("Invalid input to get hdmi\n");
+ return NULL;
+ }
+
+ /*
+ * Read EDID only when live status is up.
+ * This co
Re: [Intel-gfx] [PATCH 3/4] drm/i915: make sure VDD is turned off during system suspend
On Mon, Aug 11, 2014 at 10:04:50PM +0300, Imre Deak wrote:
> Atm we may leave eDP VDD enabled during system suspend after the CRTCs
> are disabled through an HPD->DPCD read event. So disable VDD during
> suspend at a point when no HPDs can occur.
And vdd itself holds a runtime pm ref so this problem can't occur there.
>
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/i915_drv.c | 15 +++
> drivers/gpu/drm/i915/intel_dp.c | 13 +++--
> drivers/gpu/drm/i915/intel_drv.h | 6 ++
> 3 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 0653761..1c7979e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -501,6 +501,19 @@ void intel_hpd_cancel_work(struct drm_i915_private
> *dev_priv)
> cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work);
> }
>
> +static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + struct intel_encoder *intel_encoder;
> +
> + drm_modeset_lock_all(dev);
> + for_each_intel_encoder(dev, intel_encoder) {
> + if (intel_encoder->suspend)
> + intel_encoder->suspend(intel_encoder);
> + }
> + drm_modeset_unlock_all(dev);
> +}
> +
> static int i915_drm_freeze(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -547,6 +560,8 @@ static int i915_drm_freeze(struct drm_device *dev)
> intel_runtime_pm_disable_interrupts(dev);
> intel_hpd_cancel_work(dev_priv);
>
> + intel_suspend_encoders(dev_priv);
> +
> intel_suspend_gt_powersave(dev);
>
> intel_modeset_suspend_hw(dev);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 34e3c47..d7f5d0a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1291,8 +1291,6 @@ static void edp_panel_vdd_off(struct intel_dp
> *intel_dp, bool sync)
> if (!is_edp(intel_dp))
> return;
>
> - WARN(!intel_dp->want_panel_vdd, "eDP VDD not forced on");
> -
Should we perhaps keep this warn here and call the _sync() version directly
from ->suspend()? I think we already do it that way when tearing down
the encoder.
> intel_dp->want_panel_vdd = false;
>
> if (sync)
> @@ -4003,6 +4001,16 @@ void intel_dp_encoder_destroy(struct drm_encoder
> *encoder)
> kfree(intel_dig_port);
> }
>
> +void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
> +{
> + struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
> +
> + if (!is_edp(intel_dp))
> + return;
edp_panel_vdd_off() already has the same check, but if you convert it
to use the _sync() directly then the check is needed.
> +
> + edp_panel_vdd_off(intel_dp, true);
> +}
> +
> static void intel_dp_encoder_reset(struct drm_encoder *encoder)
> {
> intel_edp_panel_vdd_sanitize(to_intel_encoder(encoder));
> @@ -4722,6 +4730,7 @@ intel_dp_init(struct drm_device *dev, int output_reg,
> enum port port)
> intel_encoder->disable = intel_disable_dp;
> intel_encoder->get_hw_state = intel_dp_get_hw_state;
> intel_encoder->get_config = intel_dp_get_config;
> + intel_encoder->suspend = intel_dp_encoder_suspend;
> if (IS_CHERRYVIEW(dev)) {
> intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
> intel_encoder->pre_enable = chv_pre_enable_dp;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 1b3d1d7..cefd337 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -153,6 +153,12 @@ struct intel_encoder {
>* be set correctly before calling this function. */
> void (*get_config)(struct intel_encoder *,
> struct intel_crtc_config *pipe_config);
> + /*
> + * Called during system suspend after all pending requests for the
> + * encoder are flushed (for example for DP AUX transactions) and
> + * device interrupts are disabled.
> + */
> + void (*suspend)(struct intel_encoder *);
> int crtc_mask;
> enum hpd_pin hpd_pin;
> };
> --
> 1.8.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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 2/4] drm/i915: cancel hotplug and dig_port work during suspend and unload
On Tue, 2014-08-12 at 15:13 +0300, Ville Syrjälä wrote:
> On Mon, Aug 11, 2014 at 09:54:15PM +0300, Imre Deak wrote:
> > Make sure these work handlers don't run after we system suspend or
> > unload the driver. Note that we don't cancel the handlers during runtime
> > suspend. That could lead to a lockup, since we take a runtime PM ref
> > from the handlers themselves. Fortunaltely canceling there is not needed
> > since the RPM ref itself provides for the needed serialization.
> >
> > Signed-off-by: Imre Deak
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 8
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_display.c | 3 +--
> > 3 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index ec96f9a..0653761 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -494,6 +494,13 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
> > return true;
> > }
> >
> > +void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
> > +{
> > + cancel_work_sync(&dev_priv->hotplug_work);
> > + cancel_work_sync(&dev_priv->dig_port_work);
>
> Since dig_port_work can queue a hotplug_work shouldn't these two be
> swapped?
Right, will fix that.
> I wonder if we should also clear hpd_event_bits and
> {long,short}_hpd_port_mask before cancelling the works? At least it
> might make the works end a bit quicker if the are already running.
Makes sense for speed, will fix it. Another thing is that a final
instance of these works can now run with interrupts disabled that could
cause DP AUX timeouts for example. That could be avoided for example by
adding a new dev_priv->hpd_irqs_disabled flag and setting it before
disabling interrupts, but I didn't want to make things more complicated
before getting some feedback.
> I also noticed that we don't seem to grab any rpm/powerwell references
> in ->hpd_pulse() or i915_digport_work_func(). That doesn't seem right.
> Or maybe you already addressed that in another patch?
No, I haven't. I thought it's enough that all low level functions like
DPCD read, link training do take already the needed refs. Isn't that
enough?
--Imre
signature.asc
Description: This is a digitally signed message part
___
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: Support for HDMI complaince HPD
On Tue, Aug 12, 2014 at 06:08:21PM +0530, shashank.sha...@intel.com wrote: > From: Shashank Sharma > > During the HDMI complaince tests, most of the HDMI analyzers > issue a soft HPD, to automate the tests. This process keeps > the HDMI cable connected, and DDC chhanel alive. > > HDMI detect() logic relies on EDID readability, to decide if > its a HDMI connect or disconnect event. But in this case, the > EDID is always readable, as HDMI cable is physically connected > and DDC channel is UP, so detect() always reports a HDMI connect > even if its intended to be a HDMI disconnect call. > > So if HDMI compliance is enabled, we should rely on the live status > register, than EDID availability. This patch adds: > 1. One kernel command line parameter for i915 module, which indicates >if we want to support HDMI compliance, for this platform. I would rather have this as an output property. In fact, I would like the hotplug detection method exposed (and even selectable, but other than this compliance testing, I can't think of a scenario where the kernel shouldn't be able to figure things out for itself). > 2. A new function to read EDID, which gets called only in case of >HDMI compliance support is required. This function reads EDID only >if live status register is up. The normal code flow doesn't get effected >if kernel command line parameter is not enabled. > 3. After various experiments on VLV2 board, with various HDMI monitors >we have seen that, with few monitors, the live status register gets >set after a slight delay, and then stays reliably. To support such >monitors, there is a busy-loop added, with a max delay upto 50ms, >with a status check after every 10ms. Please see the comment in >intel_hdmi_get_edid. Wouldn't a tidier solution be to delay the hpd by 50-100ms after the hotplug interrupt? That may overcome the issue with the live status for all connectors... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: cancel hotplug and dig_port work during suspend and unload
On Tue, Aug 12, 2014 at 03:36:01PM +0300, Imre Deak wrote:
> On Tue, 2014-08-12 at 15:13 +0300, Ville Syrjälä wrote:
> > On Mon, Aug 11, 2014 at 09:54:15PM +0300, Imre Deak wrote:
> > > Make sure these work handlers don't run after we system suspend or
> > > unload the driver. Note that we don't cancel the handlers during runtime
> > > suspend. That could lead to a lockup, since we take a runtime PM ref
> > > from the handlers themselves. Fortunaltely canceling there is not needed
> > > since the RPM ref itself provides for the needed serialization.
> > >
> > > Signed-off-by: Imre Deak
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 8
> > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > drivers/gpu/drm/i915/intel_display.c | 3 +--
> > > 3 files changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index ec96f9a..0653761 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -494,6 +494,13 @@ bool i915_semaphore_is_enabled(struct drm_device
> > > *dev)
> > > return true;
> > > }
> > >
> > > +void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
> > > +{
> > > + cancel_work_sync(&dev_priv->hotplug_work);
> > > + cancel_work_sync(&dev_priv->dig_port_work);
> >
> > Since dig_port_work can queue a hotplug_work shouldn't these two be
> > swapped?
>
> Right, will fix that.
>
> > I wonder if we should also clear hpd_event_bits and
> > {long,short}_hpd_port_mask before cancelling the works? At least it
> > might make the works end a bit quicker if the are already running.
>
> Makes sense for speed, will fix it. Another thing is that a final
> instance of these works can now run with interrupts disabled that could
> cause DP AUX timeouts for example. That could be avoided for example by
> adding a new dev_priv->hpd_irqs_disabled flag and setting it before
> disabling interrupts, but I didn't want to make things more complicated
> before getting some feedback.
>
> > I also noticed that we don't seem to grab any rpm/powerwell references
> > in ->hpd_pulse() or i915_digport_work_func(). That doesn't seem right.
> > Or maybe you already addressed that in another patch?
>
> No, I haven't. I thought it's enough that all low level functions like
> DPCD read, link training do take already the needed refs. Isn't that
> enough?
There's at least the call to ibx_digital_port_connected() which isn't
covered by any rpm/powerwell reference.
--
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 3/4] drm/i915: make sure VDD is turned off during system suspend
On Tue, 2014-08-12 at 15:34 +0300, Ville Syrjälä wrote:
> On Mon, Aug 11, 2014 at 10:04:50PM +0300, Imre Deak wrote:
> > Atm we may leave eDP VDD enabled during system suspend after the CRTCs
> > are disabled through an HPD->DPCD read event. So disable VDD during
> > suspend at a point when no HPDs can occur.
>
> And vdd itself holds a runtime pm ref so this problem can't occur there.
Yep, I'll update the commit message.
> > Signed-off-by: Imre Deak
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 15 +++
> > drivers/gpu/drm/i915/intel_dp.c | 13 +++--
> > drivers/gpu/drm/i915/intel_drv.h | 6 ++
> > 3 files changed, 32 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 0653761..1c7979e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -501,6 +501,19 @@ void intel_hpd_cancel_work(struct drm_i915_private
> > *dev_priv)
> > cancel_delayed_work_sync(&dev_priv->hotplug_reenable_work);
> > }
> >
> > +static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
> > +{
> > + struct drm_device *dev = dev_priv->dev;
> > + struct intel_encoder *intel_encoder;
> > +
> > + drm_modeset_lock_all(dev);
> > + for_each_intel_encoder(dev, intel_encoder) {
> > + if (intel_encoder->suspend)
> > + intel_encoder->suspend(intel_encoder);
> > + }
> > + drm_modeset_unlock_all(dev);
> > +}
> > +
> > static int i915_drm_freeze(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -547,6 +560,8 @@ static int i915_drm_freeze(struct drm_device *dev)
> > intel_runtime_pm_disable_interrupts(dev);
> > intel_hpd_cancel_work(dev_priv);
> >
> > + intel_suspend_encoders(dev_priv);
> > +
> > intel_suspend_gt_powersave(dev);
> >
> > intel_modeset_suspend_hw(dev);
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 34e3c47..d7f5d0a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1291,8 +1291,6 @@ static void edp_panel_vdd_off(struct intel_dp
> > *intel_dp, bool sync)
> > if (!is_edp(intel_dp))
> > return;
> >
> > - WARN(!intel_dp->want_panel_vdd, "eDP VDD not forced on");
> > -
>
> Should we perhaps keep this warn here and call the _sync() version directly
> from ->suspend()? I think we already do it that way when tearing down
> the encoder.
Ok.
> > intel_dp->want_panel_vdd = false;
> >
> > if (sync)
> > @@ -4003,6 +4001,16 @@ void intel_dp_encoder_destroy(struct drm_encoder
> > *encoder)
> > kfree(intel_dig_port);
> > }
> >
> > +void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
> > +{
> > + struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
> > +
> > + if (!is_edp(intel_dp))
> > + return;
>
> edp_panel_vdd_off() already has the same check, but if you convert it
> to use the _sync() directly then the check is needed.
>
> > +
> > + edp_panel_vdd_off(intel_dp, true);
> > +}
> > +
> > static void intel_dp_encoder_reset(struct drm_encoder *encoder)
> > {
> > intel_edp_panel_vdd_sanitize(to_intel_encoder(encoder));
> > @@ -4722,6 +4730,7 @@ intel_dp_init(struct drm_device *dev, int output_reg,
> > enum port port)
> > intel_encoder->disable = intel_disable_dp;
> > intel_encoder->get_hw_state = intel_dp_get_hw_state;
> > intel_encoder->get_config = intel_dp_get_config;
> > + intel_encoder->suspend = intel_dp_encoder_suspend;
> > if (IS_CHERRYVIEW(dev)) {
> > intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
> > intel_encoder->pre_enable = chv_pre_enable_dp;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 1b3d1d7..cefd337 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -153,6 +153,12 @@ struct intel_encoder {
> > * be set correctly before calling this function. */
> > void (*get_config)(struct intel_encoder *,
> >struct intel_crtc_config *pipe_config);
> > + /*
> > +* Called during system suspend after all pending requests for the
> > +* encoder are flushed (for example for DP AUX transactions) and
> > +* device interrupts are disabled.
> > +*/
> > + void (*suspend)(struct intel_encoder *);
> > int crtc_mask;
> > enum hpd_pin hpd_pin;
> > };
> > --
> > 1.8.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop
Re: [Intel-gfx] [PATCH] drm/i915: Wait for vblank after enabling the primary plane on BDW
On Tue, Aug 12, 2014 at 11:24:11AM +0300, Ville Syrjälä wrote:
> On Mon, Aug 11, 2014 at 04:44:23PM -0300, Paulo Zanoni wrote:
> > 2014-06-30 7:10 GMT-03:00 Jani Nikula :
> > > On Thu, 26 Jun 2014, Rodrigo Vivi wrote:
> > >> I'm sure this might affect Wayne, so, cc'ing him here.
> > >>
> > >> from my point of view this is right so:
> > >> Reviewed-by: Rodrigo Vivi
> > >
> > > Pushed to -fixes, thanks for the patch and review.
> > >
> > > BR,
> > > Jani.
> > >
> > >
> > >>
> > >>
> > >> On Tue, Jun 24, 2014 at 3:59 AM, wrote:
> > >>
> > >>> From: Ville Syrjälä
> > >>>
> > >>> BDW signals the flip done interrupt immediately after the DSPSURF write
> > >>> when the plane is disabled. This is true even if we've already armed
> > >>> DSPCNTR to enable the plane at the next vblank. This causes major
> > >>> problems for our page flip code which relies on the flip done interrupts
> > >>> happening at vblank time.
> > >>>
> > >>> So what happens is that we enable the plane, and immediately allow
> > >>> userspace to submit a page flip. If the plane is still in the process
> > >>> of being enabled when the page flip is issued, the flip done gets
> > >>> signalled immediately. Our DSPSURFLIVE check catches this to prevent
> > >>> premature flip completion, but it also means that we don't get a flip
> > >>> done interrupt when the plane actually gets enabled, and so the page
> > >>> flip is never completed.
> > >>>
> > >>> Work around this by re-introducing blocking vblank waits on BDW
> > >>> whenever we enable the primary plane.
> > >>>
> > >>> I removed some of the vblank waits here:
> > >>> commit 6304cd91e7f05f8802ea6f91287cac09741d9c46
> > >>> Author: Ville Syrjälä
> > >>> Date: Fri Apr 25 13:30:12 2014 +0300
> > >>>
> > >>> drm/i915: Drop the excessive vblank waits from modeset codepaths
> > >>>
> > >>> To avoid these blocking vblank waits we should start using the vblank
> > >>> interrupt instead of the flip done interrupt to complete page flips.
> > >>> But that's material for another patch.
> > >>>
> > >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79354
> > >>> Tested-by: Guo Jinxian
> > >>> Signed-off-by: Ville Syrjälä
> > >>> ---
> > >>> drivers/gpu/drm/i915/intel_display.c | 9 +
> > >>> drivers/gpu/drm/i915/intel_sprite.c | 8
> > >>> 2 files changed, 17 insertions(+)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/i915/intel_display.c
> > >>> b/drivers/gpu/drm/i915/intel_display.c
> > >>> index 9188fed..f92efc6 100644
> > >>> --- a/drivers/gpu/drm/i915/intel_display.c
> > >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> > >>> @@ -2087,6 +2087,7 @@ void intel_flush_primary_plane(struct
> > >>> drm_i915_private *dev_priv,
> > >>> static void intel_enable_primary_hw_plane(struct drm_i915_private
> > >>> *dev_priv,
> > >>> enum plane plane, enum pipe
> > >>> pipe)
> > >>> {
> > >>> + struct drm_device *dev = dev_priv->dev;
> > >>> struct intel_crtc *intel_crtc =
> > >>> to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> > >>> int reg;
> > >>> @@ -2106,6 +2107,14 @@ static void intel_enable_primary_hw_plane(struct
> > >>> drm_i915_private *dev_priv,
> > >>>
> > >>> I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE);
> > >>> intel_flush_primary_plane(dev_priv, plane);
> > >>> +
> > >>> + /*
> > >>> +* BDW signals flip done immediately if the plane
> > >>> +* is disabled, even if the plane enable is already
> > >>> +* armed to occur at the next vblank :(
> > >>> +*/
> > >>> + if (IS_BROADWELL(dev))
> > >>> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> >
> > This chunk triggers "WARN(ret == 0)" from drm_wait_one_vblank when
> > using HDMI on BDW.
>
> Are we still calling drm_vblank_off() too soon or something?
Oh it was enable. That could mean we still haven't yet called
drm_vblank_on(). But at least the order in
intel_crtc_enable_planes() seems correct, and also
intel_primary_plane_setplane() should never try enable the
primary plane when the crtc is not active ('visible'
should be false in that case).
>
> >
> >
> > >>> }
> > >>>
> > >>> /**
> > >>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > >>> b/drivers/gpu/drm/i915/intel_sprite.c
> > >>> index 1b66ddc..9a17b4e 100644
> > >>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> > >>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > >>> @@ -691,6 +691,14 @@ intel_post_enable_primary(struct drm_crtc *crtc)
> > >>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > >>>
> > >>> /*
> > >>> +* BDW signals flip done immediately if the plane
> > >>> +* is disabled, even if the plane enable is already
> > >>> +* armed to occur at the next vblank :(
> > >>> +*/
> > >>> + if (IS_BROADWELL(dev))
> > >>> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> > >>> +
> > >>> + /*
> > >>>
Re: [Intel-gfx] [PATCH 2/4] drm/i915: cancel hotplug and dig_port work during suspend and unload
On Tue, 2014-08-12 at 15:53 +0300, Ville Syrjälä wrote:
> On Tue, Aug 12, 2014 at 03:36:01PM +0300, Imre Deak wrote:
> > On Tue, 2014-08-12 at 15:13 +0300, Ville Syrjälä wrote:
> > > On Mon, Aug 11, 2014 at 09:54:15PM +0300, Imre Deak wrote:
> > > > Make sure these work handlers don't run after we system suspend or
> > > > unload the driver. Note that we don't cancel the handlers during runtime
> > > > suspend. That could lead to a lockup, since we take a runtime PM ref
> > > > from the handlers themselves. Fortunaltely canceling there is not needed
> > > > since the RPM ref itself provides for the needed serialization.
> > > >
> > > > Signed-off-by: Imre Deak
> > > > ---
> > > > drivers/gpu/drm/i915/i915_drv.c | 8
> > > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > > drivers/gpu/drm/i915/intel_display.c | 3 +--
> > > > 3 files changed, 10 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > > b/drivers/gpu/drm/i915/i915_drv.c
> > > > index ec96f9a..0653761 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -494,6 +494,13 @@ bool i915_semaphore_is_enabled(struct drm_device
> > > > *dev)
> > > > return true;
> > > > }
> > > >
> > > > +void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
> > > > +{
> > > > + cancel_work_sync(&dev_priv->hotplug_work);
> > > > + cancel_work_sync(&dev_priv->dig_port_work);
> > >
> > > Since dig_port_work can queue a hotplug_work shouldn't these two be
> > > swapped?
> >
> > Right, will fix that.
> >
> > > I wonder if we should also clear hpd_event_bits and
> > > {long,short}_hpd_port_mask before cancelling the works? At least it
> > > might make the works end a bit quicker if the are already running.
> >
> > Makes sense for speed, will fix it. Another thing is that a final
> > instance of these works can now run with interrupts disabled that could
> > cause DP AUX timeouts for example. That could be avoided for example by
> > adding a new dev_priv->hpd_irqs_disabled flag and setting it before
> > disabling interrupts, but I didn't want to make things more complicated
> > before getting some feedback.
> >
> > > I also noticed that we don't seem to grab any rpm/powerwell references
> > > in ->hpd_pulse() or i915_digport_work_func(). That doesn't seem right.
> > > Or maybe you already addressed that in another patch?
> >
> > No, I haven't. I thought it's enough that all low level functions like
> > DPCD read, link training do take already the needed refs. Isn't that
> > enough?
>
> There's at least the call to ibx_digital_port_connected() which isn't
> covered by any rpm/powerwell reference.
Right, haven't noticed :/ I can fix this by taking the display port
power domain around ->hpd_pulse(), similar to all the other DP
encoder/connector handlers (get_modes, detect etc.).
--Imre
signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/15] drm/i915: Cleanup aliasging ppgtt alongside the global gtt
On Fri, Aug 08, 2014 at 01:09:25PM +, Thierry, Michel wrote:
>
>
> > -Original Message-
> > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
> > Of Daniel Vetter
> > Sent: Wednesday, August 06, 2014 2:05 PM
> > To: Intel Graphics Development
> > Cc: Daniel Vetter
> > Subject: [Intel-gfx] [PATCH 14/15] drm/i915: Cleanup aliasging ppgtt
> > alongside the global gtt
> >
> > Also remove related WARN_ONs which seem to have been hit since a rather
> > long time. But apperently no one noticed since our module reload is
> > already WARNING-infested :(
> >
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/i915/i915_dma.c | 4
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++
> > 2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index c176a6c97c80..94afe7c4458b 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1388,7 +1388,6 @@ cleanup_gem:
> > i915_gem_cleanup_ringbuffer(dev);
> > i915_gem_context_fini(dev);
> > mutex_unlock(&dev->struct_mutex);
> > - WARN_ON(dev_priv->mm.aliasing_ppgtt);
> > cleanup_irq:
> > drm_irq_uninstall(dev);
> > cleanup_gem_stolen:
> > @@ -1897,7 +1896,6 @@ int i915_driver_unload(struct drm_device *dev)
> > mutex_lock(&dev->struct_mutex);
> > i915_gem_cleanup_ringbuffer(dev);
> > i915_gem_context_fini(dev);
> > - WARN_ON(dev_priv->mm.aliasing_ppgtt);
> > mutex_unlock(&dev->struct_mutex);
> > i915_gem_cleanup_stolen(dev);
> >
> > @@ -1905,8 +1903,6 @@ int i915_driver_unload(struct drm_device *dev)
> > i915_free_hws(dev);
> > }
> >
> > - WARN_ON(!list_empty(&dev_priv->vm_list));
> > -
> > drm_vblank_cleanup(dev);
> >
> > intel_teardown_gmbus(dev);
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 2eab0b6a32e8..ff031bb1f296 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -1801,6 +1801,12 @@ void i915_global_gtt_cleanup(struct drm_device
> > *dev)
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct i915_address_space *vm = &dev_priv->gtt.base;
> >
> > + if (dev_priv->mm.aliasing_ppgtt) {
> > + struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
> > +
> > + ppgtt->base.cleanup(&ppgtt->base);
> > + }
> > +
> > if (drm_mm_initialized(&vm->mm)) {
> > drm_mm_takedown(&vm->mm);
> > list_del(&vm->global_link);
> > @@ -1808,6 +1814,7 @@ void i915_global_gtt_cleanup(struct drm_device
> > *dev)
> >
> > vm->cleanup(vm);
> > }
> > +
> > static int setup_scratch_page(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > --
> > 1.9.3
>
> Reviewed-by: Michel Thierry
Thanks for the review, all patches merged. Aside I've spotted a bit of
spelling fail in my commit messages - review should also complain about
that.
-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
[Intel-gfx] [PATCH 0/4] drm/i915: backlight sysfs bl_power and brightness == 0
This series adds support for backlight class sysfs bl_power attribute for eDP panels, which allows switching the backlight on/off. This is done using the eDP panel power control as a sub-state of everything else being enabled. Patch 4 also makes 0 brightness switch off the eDP backlight using the same mechanism. This is all eDP specific, with no support for LVDS. I'm not particularly fond of these patches, but there appears to be demand for them, particularly since commit 6dda730e55f412a6dfb181cae6784822ba463847 Author: Jani Nikula Date: Tue Jun 24 18:27:40 2014 +0300 drm/i915: respect the VBT minimum backlight brightness All the discussions seem to just drag on and on without code to speak about, so here goes. I've tried to make this as simple and unintrusive as possible. BR, Jani. Jani Nikula (4): drm/i915/dp: split up panel power control from backlight pwm control drm/i915: add some framework for backlight bl_power support drm/i915/dp: make backlight bl_power control power sequencer backlight drm/i915: switch off backlight for backlight class 0 brightness drivers/gpu/drm/i915/intel_dp.c| 61 ++ drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_panel.c | 27 + 3 files changed, 77 insertions(+), 13 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915/dp: make backlight bl_power control power sequencer backlight
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;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915/dp: split up panel power control from backlight pwm control
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);
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: switch off backlight for backlight class 0 brightness
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 {
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: add some framework for backlight bl_power support
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.
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..c365f2a57c75 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)
+ panel->backlight.device->props.power = FB_BLANK_UNBLANK;
+ else
+ panel->backlight.device->props.power = FB_BLANK_POWERDOWN;
+
/*
* Note: using the same name independent of the connector prevents
* registration of multiple backlight devices in the driver.
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Localise the fbdev console lock frobbing
Rather than take and release the console_lock() around a non-existent
DRM_I915_FBDEV, move the lock acquisation into the callee where it will
be compiled out by the config option entirely. This includes moving the
deferred fb_set_suspend() dance and encapsulating it entirely within
intel_fbdev.c.
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
---
drivers/gpu/drm/i915/i915_dma.c| 5 ++---
drivers/gpu/drm/i915/i915_drv.c| 26 +-
drivers/gpu/drm/i915/i915_drv.h| 8
drivers/gpu/drm/i915/intel_fbdev.c | 33 +
4 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 49149406fcd8..d0b3411fc13c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1339,8 +1339,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_irq;
- INIT_WORK(&dev_priv->console_resume_work, intel_console_resume);
-
intel_modeset_gem_init(dev);
/* Always safe in the mode setting case. */
@@ -1835,6 +1833,8 @@ int i915_driver_unload(struct drm_device *dev)
return ret;
}
+ flush_scheduled_work();
+
i915_perf_unregister(dev);
intel_fini_runtime_pm(dev_priv);
@@ -1859,7 +1859,6 @@ int i915_driver_unload(struct drm_device *dev)
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
intel_fbdev_fini(dev);
intel_modeset_cleanup(dev);
- cancel_work_sync(&dev_priv->console_resume_work);
/*
* free the memory space allocated for the child device
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 35b119072c11..9adafc7c5819 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -558,9 +558,7 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_uncore_forcewake_reset(dev, false);
intel_opregion_fini(dev);
- console_lock();
intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
- console_unlock();
dev_priv->suspend_count++;
@@ -599,18 +597,6 @@ int i915_suspend(struct drm_device *dev, pm_message_t
state)
return 0;
}
-void intel_console_resume(struct work_struct *work)
-{
- struct drm_i915_private *dev_priv =
- container_of(work, struct drm_i915_private,
-console_resume_work);
- struct drm_device *dev = dev_priv->dev;
-
- console_lock();
- intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING);
- console_unlock();
-}
-
static int i915_drm_thaw_early(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -683,17 +669,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
intel_opregion_init(dev);
- /*
-* The console lock can be pretty contented on resume due
-* to all the printk activity. Try to keep it out of the hot
-* path of resume if possible.
-*/
- if (console_trylock()) {
- intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING);
- console_unlock();
- } else {
- schedule_work(&dev_priv->console_resume_work);
- }
+ intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING);
mutex_lock(&dev_priv->modeset_restore_lock);
dev_priv->modeset_restore = MODESET_DONE;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 517a78e27bbb..e38dd39d84ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1603,12 +1603,6 @@ struct drm_i915_private {
struct intel_fbdev *fbdev;
#endif
- /*
-* The console may be contended at resume, but we don't
-* want it to block on it.
-*/
- struct work_struct console_resume_work;
-
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
@@ -2310,8 +2304,6 @@ extern unsigned long i915_gfx_val(struct drm_i915_private
*dev_priv);
extern void i915_update_gfx_val(struct drm_i915_private *dev_priv);
int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool on);
-extern void intel_console_resume(struct work_struct *work);
-
/* i915_irq.c */
void i915_queue_hangcheck(struct drm_device *dev);
__printf(3, 4)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index 5d879d185fe1..4eecb34a0e32 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -26,6 +26,7 @@
#include
#include
+#include
#include
#include
#include
@@ -678,6 +679,19 @@ void intel_fbdev_fini(struct drm_device *dev)
dev_priv->fbdev = NULL;
}
+struct set_suspend_work {
+ struct work_struct work;
+ struct drm_device *dev
Re: [Intel-gfx] xrandr issue
Hi Paulo, Thanks for your response. I am on kernel 3.13.0-32-generic. I was actually able to resolve my problem by installing the latest Intel graphics driver on Ubuntu. Thank You, Brayden Williams On Mon, Aug 11, 2014 at 6:12 AM, Paulo Zanoni wrote: > Hi > > When you have this type of question, please always CC our mailing > list, since you're more likely to get a fast answer when using it. And > it also helps documenting all the common problems people have. > > 2014-08-09 13:31 GMT-03:00 Brayden Williams : > > Hi Paulo, > > > > I have been desperately trying to get my 3 monitor setup to work with > Intel > > graphics in Ubuntu. After hours of Googling I finally came across your > > comment on this bug report > > (https://bugs.freedesktop.org/show_bug.cgi?id=68485) but I am still > having > > trouble getting it working. Ubuntu gives me the following error: > > > > [4.700920] [drm:intel_ddi_pll_mode_set] *ERROR* No WRPLLs available! > > > > The output of my xrandr --verbose command can be found at > > https://gist.github.com/redstar504/6b630e3e664d0db6a2f5 > > > > It seems like the pixel clock is the same for HDMI1 and HDMI2 so this > should > > be working shouldn't it? > > Yes, it should be working, but only if you have a Kernel that actually > contains the patch mentioned in the bugzilla. Which Kernel version are > you running? > > You could also try to set things manually after you boot: > > xrandr --output HDMI1 --mode 0x4a > xrandr --output HDMI2 --mode 0x4a > xrandr --output HDMI3 --mode 0x4a > > If your Kernel is old, you could try to use the DP or VGA outputs > instead of using HDMI for every port. > > DId you try to use any newer Kernels? If this problem happens with the > latest development or stable Kernels, you should probably open another > bug (or reopen the one you mentioned). > > > > > I'd truly appreciate any help you can provide. > > > > Thank You, > > Brayden Williams > > > > -- > Paulo Zanoni > ___ 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: Support for HDMI complaince HPD
Hello Chris, Thanks for your time and comments. I would like to give a brief history of the patch. We tried to apply this optimization by default, and check all the EDID read based on the live status. But not all developers agreed to have this by default, with following reasons: 1. live_status was not very reliable for all platforms, so live_status based solution shouldn't be added. 2. they dint want EDID caching to be by default, as few old platforms were not even HPD capable. So we came up with this intermediate solution to have: 1. Timeout based EDID caching, where cached EDID will be cleared after one minute. 2. An alternative code flow to support HDMI compliance (will be based on the live_status check, and will be only enabled for commercial platforms like android) which need HDMI compliance support. So the kernel command line parameter is to support the need to add this alternative EDID read method. Please le me know your opinion about this, considering the background. Regards Shashank On 8/12/2014 6:17 PM, Chris Wilson wrote: On Tue, Aug 12, 2014 at 06:08:21PM +0530, shashank.sha...@intel.com wrote: From: Shashank Sharma During the HDMI complaince tests, most of the HDMI analyzers issue a soft HPD, to automate the tests. This process keeps the HDMI cable connected, and DDC chhanel alive. HDMI detect() logic relies on EDID readability, to decide if its a HDMI connect or disconnect event. But in this case, the EDID is always readable, as HDMI cable is physically connected and DDC channel is UP, so detect() always reports a HDMI connect even if its intended to be a HDMI disconnect call. So if HDMI compliance is enabled, we should rely on the live status register, than EDID availability. This patch adds: 1. One kernel command line parameter for i915 module, which indicates if we want to support HDMI compliance, for this platform. I would rather have this as an output property. In fact, I would like the hotplug detection method exposed (and even selectable, but other than this compliance testing, I can't think of a scenario where the kernel shouldn't be able to figure things out for itself). 2. A new function to read EDID, which gets called only in case of HDMI compliance support is required. This function reads EDID only if live status register is up. The normal code flow doesn't get effected if kernel command line parameter is not enabled. 3. After various experiments on VLV2 board, with various HDMI monitors we have seen that, with few monitors, the live status register gets set after a slight delay, and then stays reliably. To support such monitors, there is a busy-loop added, with a max delay upto 50ms, with a status check after every 10ms. Please see the comment in intel_hdmi_get_edid. Wouldn't a tidier solution be to delay the hpd by 50-100ms after the hotplug interrupt? That may overcome the issue with the live status for all connectors... -Chris ___ 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: Support for HDMI complaince HPD
On Tue, Aug 12, 2014 at 08:56:44PM +0530, Sharma, Shashank wrote: > Hello Chris, > > Thanks for your time and comments. > I would like to give a brief history of the patch. > > We tried to apply this optimization by default, and check all the > EDID read based on the live status. But not all developers agreed to > have this by default, with following reasons: > 1. live_status was not very reliable for all platforms, so live_status >based solution shouldn't be added. > 2. they dint want EDID caching to be by default, as few old platforms >were not even HPD capable. > > So we came up with this intermediate solution to have: > 1. Timeout based EDID caching, where cached EDID will be cleared after >one minute. > 2. An alternative code flow to support HDMI compliance (will be based >on the live_status check, and will be only enabled for commercial >platforms like android) which need HDMI compliance support. So the >kernel command line parameter is to support the need to add this >alternative EDID read method. > > Please le me know your opinion about this, considering the background. I know. That is orthogonal to the tweaks I was suggesting. Also if you feel you need to add details to your rationale, then your changelog is incomplete. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: Don't try to enable cursor from setplane when crtc is disabled
From: Ville Syrjälä
Make sure the cursor gets fully clipped when enabling it on a disabled
crtc via setplane. This will prevent the lower level code from
attempting to enable the cursor in hardware.
Cc: Paulo Zanoni
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 511c8f4..123cbf1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11701,8 +11701,8 @@ intel_cursor_plane_update(struct drm_plane *plane,
struct drm_crtc *crtc,
};
const struct drm_rect clip = {
/* integer pixels */
- .x2 = intel_crtc->config.pipe_src_w,
- .y2 = intel_crtc->config.pipe_src_h,
+ .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
+ .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
};
bool visible;
int ret;
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: Move CURSIZE setup to i845_update_cursor()
From: Ville Syrjälä
CURSIZE register exists on 845/865 only, so move it to
i845_update_cursor(). Changes to cursor size must be done only when the
cursor is disabled, so do the write just before enabling the cursor.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 123cbf1..744c161 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8028,6 +8028,7 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32
base)
CURSOR_GAMMA_ENABLE |
CURSOR_FORMAT_ARGB);
if (intel_crtc->cursor_cntl != cntl) {
+ I915_WRITE(CURSIZE, (64 << 12) | 64);
I915_WRITE(_CURACNTR, cntl);
POSTING_READ(_CURACNTR);
intel_crtc->cursor_cntl = cntl;
@@ -8177,7 +8178,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
uint32_t width, uint32_t height)
{
struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum pipe pipe = intel_crtc->pipe;
unsigned old_width;
@@ -8250,9 +8250,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
addr = obj->phys_handle->busaddr;
}
- if (IS_GEN2(dev))
- I915_WRITE(CURSIZE, (height << 12) | width);
-
finish:
if (intel_crtc->cursor_bo) {
if (!INTEL_INFO(dev)->cursor_needs_physical)
--
1.8.5.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/4] drm/i915: Cursor fixes and cleanups
From: Ville Syrjälä I had to look at the cursor code recently, and as usual I ended up doing bit of fixing and some cleanups. There's still more that needs doing, but I'll leave that for later and not continue down this path for now. While trying to figure out what this CURSIZE register is I also ended up implementing variable cursor size support for 845/865. I don't have the hardware so I can't say whether it really works. But here's the code anyway. Ville Syrjälä (4): drm/i915: Don't try to enable cursor from setplane when crtc is disabled drm/i915: Move CURSIZE setup to i845_update_cursor() drm/i915: Unify ivb_update_cursor() and i9xx_update_cursor() drm/i915: Add support for variable cursor size on 845/865 drivers/gpu/drm/i915/i915_reg.h | 3 +- drivers/gpu/drm/i915/intel_display.c | 134 +-- drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 67 insertions(+), 71 deletions(-) -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: Add support for variable cursor size on 845/865
From: Ville Syrjälä
845/865 support different cursor sizes as well, albeit a bit differently
than later platforms. Add the necessary code to make them work.
Untested due to lack of hardware.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 3 +-
drivers/gpu/drm/i915/intel_display.c | 86 +---
drivers/gpu/drm/i915/intel_drv.h | 1 +
3 files changed, 64 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f79c20d..203062e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4128,7 +4128,8 @@ enum punit_power_well {
/* Old style CUR*CNTR flags (desktop 8xx) */
#define CURSOR_ENABLE0x8000
#define CURSOR_GAMMA_ENABLE 0x4000
-#define CURSOR_STRIDE_MASK 0x3000
+#define CURSOR_STRIDE_SHIFT 28
+#define CURSOR_STRIDE(x) ((ffs(x)-9) << CURSOR_STRIDE_SHIFT) /*
256,512,1k,2k */
#define CURSOR_PIPE_CSC_ENABLE (1<<24)
#define CURSOR_FORMAT_SHIFT 24
#define CURSOR_FORMAT_MASK (0x07 << CURSOR_FORMAT_SHIFT)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a1cf052..db10870 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8005,30 +8005,53 @@ static void i845_update_cursor(struct drm_crtc *crtc,
u32 base)
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- uint32_t cntl;
+ uint32_t cntl = 0, size = 0;
- if (base != intel_crtc->cursor_base) {
- /* On these chipsets we can only modify the base whilst
-* the cursor is disabled.
-*/
- if (intel_crtc->cursor_cntl) {
- I915_WRITE(_CURACNTR, 0);
- POSTING_READ(_CURACNTR);
- intel_crtc->cursor_cntl = 0;
+ if (base) {
+ unsigned int width = intel_crtc->cursor_width;
+ unsigned int height = intel_crtc->cursor_height;
+ unsigned int stride = roundup_pow_of_two(width) * 4;
+
+ switch (stride) {
+ case 256:
+ case 512:
+ case 1024:
+ case 2048:
+ cntl |= CURSOR_STRIDE(stride);
+ break;
+ default:
+ WARN_ON(1);
+ return;
}
+ cntl |= CURSOR_ENABLE |
+ CURSOR_GAMMA_ENABLE |
+ CURSOR_FORMAT_ARGB;
+
+ size = (height << 12) | width;
+ }
+
+ if (intel_crtc->cursor_cntl != 0 &&
+ (intel_crtc->cursor_base != base ||
+intel_crtc->cursor_size != size ||
+intel_crtc->cursor_cntl != cntl)) {
+ /* On these chipsets we can only modify the base/size/stride
+* whilst the cursor is disabled.
+*/
+ I915_WRITE(_CURACNTR, 0);
+ POSTING_READ(_CURACNTR);
+ intel_crtc->cursor_cntl = 0;
+ }
+
+ if (intel_crtc->cursor_base != base)
I915_WRITE(_CURABASE, base);
- POSTING_READ(_CURABASE);
+
+ if (intel_crtc->cursor_size != size) {
+ I915_WRITE(CURSIZE, size);
+ intel_crtc->cursor_size = size;
}
- /* XXX width must be 64, stride 256 => 0x00 << 28 */
- cntl = 0;
- if (base)
- cntl = (CURSOR_ENABLE |
- CURSOR_GAMMA_ENABLE |
- CURSOR_FORMAT_ARGB);
if (intel_crtc->cursor_cntl != cntl) {
- I915_WRITE(CURSIZE, (64 << 12) | 64);
I915_WRITE(_CURACNTR, cntl);
POSTING_READ(_CURACNTR);
intel_crtc->cursor_cntl = cntl;
@@ -8141,7 +8164,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
struct drm_device *dev = crtc->dev;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum pipe pipe = intel_crtc->pipe;
- unsigned old_width;
+ unsigned old_width, stride;
uint32_t addr;
int ret;
@@ -8155,14 +8178,23 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
}
/* Check for which cursor types we support */
- if (!((width == 64 && height == 64) ||
- (width == 128 && height == 128 && !IS_GEN2(dev)) ||
- (width == 256 && height == 256 && !IS_GEN2(dev {
- DRM_DEBUG("Cursor dimension not supported\n");
- return -EINVAL;
+ if (IS_845G(dev) || IS_I865G(dev)) {
+ if (width == 0 || height == 0 || (width & 63) != 0 ||
+ width > (IS_845G(dev) ? 64 : 512) || height > 1023) {
+
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Add support for variable cursor size on 845/865
On Tue, Aug 12, 2014 at 07:39:55PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > 845/865 support different cursor sizes as well, albeit a bit differently > than later platforms. Add the necessary code to make them work. > > Untested due to lack of hardware. Oh that looks fun. First 3 patches are Reviewed-by: Chris Wilson This (4/4) requires a bit more checking and testing... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Unify ivb_update_cursor() and i9xx_update_cursor()
From: Ville Syrjälä
Ever since
commit 5efb3e2838536832c9b6872512e6b6daf592cee9
Author: Ville Syrjälä
Date: Wed Apr 9 13:28:53 2014 +0300
drm/i915/chv: Add cursor pipe offsets
the only difference between i9xx_update_cursor() and ivb_update_cursor()
was the hsw+ pipe csc handling. Let's unify them and we can rid
outselves of some duplicated code.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 41 +---
1 file changed, 1 insertion(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 744c161..a1cf052 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8062,43 +8062,6 @@ static void i9xx_update_cursor(struct drm_crtc *crtc,
u32 base)
}
cntl |= pipe << 28; /* Connect to correct pipe */
}
- if (intel_crtc->cursor_cntl != cntl) {
- I915_WRITE(CURCNTR(pipe), cntl);
- POSTING_READ(CURCNTR(pipe));
- intel_crtc->cursor_cntl = cntl;
- }
-
- /* and commit changes on next vblank */
- I915_WRITE(CURBASE(pipe), base);
- POSTING_READ(CURBASE(pipe));
-}
-
-static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
-{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int pipe = intel_crtc->pipe;
- uint32_t cntl;
-
- cntl = 0;
- if (base) {
- cntl = MCURSOR_GAMMA_ENABLE;
- switch (intel_crtc->cursor_width) {
- case 64:
- cntl |= CURSOR_MODE_64_ARGB_AX;
- break;
- case 128:
- cntl |= CURSOR_MODE_128_ARGB_AX;
- break;
- case 256:
- cntl |= CURSOR_MODE_256_ARGB_AX;
- break;
- default:
- WARN_ON(1);
- return;
- }
- }
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
cntl |= CURSOR_PIPE_CSC_ENABLE;
@@ -8157,9 +8120,7 @@ static void intel_crtc_update_cursor(struct drm_crtc
*crtc,
I915_WRITE(CURPOS(pipe), pos);
- if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev))
- ivb_update_cursor(crtc, base);
- else if (IS_845G(dev) || IS_I865G(dev))
+ if (IS_845G(dev) || IS_I865G(dev))
i845_update_cursor(crtc, base);
else
i9xx_update_cursor(crtc, base);
--
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] drm/i915: remove pixel doubled HDMI modes from valid modes list
On 08/11/2014 11:59 PM, Daniel Vetter wrote: On Tue, Aug 12, 2014 at 07:39:24AM +0100, Chris Wilson wrote: On Mon, Aug 11, 2014 at 03:33:02PM -0700, clinton.a.tay...@intel.com wrote: From: Clint Taylor Intel HDMI does not correctly configure pixel replicated HDMI modes 480i and 576i. Remove support for these modes until DRM has been changed to correctly identify SD interlaced modes by reporting there true horizontal resolution 720 instead of the pre-pixel doubled 1440. Signed-off-by: Clint Taylor --- drivers/gpu/drm/i915/intel_hdmi.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 5f47d35..ab32523 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -873,6 +873,9 @@ intel_hdmi_mode_valid(struct drm_connector *connector, if (mode->flags & DRM_MODE_FLAG_DBLSCAN) return MODE_NO_DBLESCAN; If it is just a workaround for a drm bug. You need a big comment explaining why you w/a rather than pressing for the bug fix. Well there's a very small chance I'll take this anyway - if the drm core is broken we need to take out the mode from the drm core, not here. -Daniel Agreed this is not the correct solution to fix the issue. Advertising that we support bad geometry is also incorrect. Clint ___ 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: Don't try to enable cursor from setplane when crtc is disabled
2014-08-12 13:39 GMT-03:00 :
> From: Ville Syrjälä
>
> Make sure the cursor gets fully clipped when enabling it on a disabled
> crtc via setplane. This will prevent the lower level code from
> attempting to enable the cursor in hardware.
If this is going to replace part of the fix I recently submitted, it
needs Cc: sta...@vger.kernel.org.
I briefly smoke-tested it and it appears to properly replace the
"intel_crtc->active" early return which you pointed.
>
> Cc: Paulo Zanoni
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 511c8f4..123cbf1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11701,8 +11701,8 @@ intel_cursor_plane_update(struct drm_plane *plane,
> struct drm_crtc *crtc,
> };
> const struct drm_rect clip = {
> /* integer pixels */
> - .x2 = intel_crtc->config.pipe_src_w,
> - .y2 = intel_crtc->config.pipe_src_h,
> + .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
> + .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
> };
> bool visible;
> int ret;
> --
> 1.8.5.5
>
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended
From: Paulo Zanoni
If we're runtime suspended and try to use the plane interfaces, we
will get a lot of WARNs saying we did the wrong thing.
We need to get runtime PM references to pin/unpin the objects, and to
change the fences. The pin/unpin functions are the ideal places for
this, but intel_crtc_cursor_set_obj() doesn't call them, so we also
have to add get/put calls inside it. There is no problem if we runtime
suspend right after these functions are finished, because the
registers written are forwarded to system memory.
Note: for a complete fix of the cursor-dpms test case, we also need
the patch named "drm/i915: Don't try to enable cursor from setplane
when crtc is disabled".
v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel)
v3: - Make get/put also surround the fence and unpin calls (Daniel and
Ville).
- Merge all the plane changes into a single patch since they're
the same fix.
- Add the comment requested by Daniel.
v4: - Remove spurious whitespace (Ville).
v5: - Remove intel_crtc_update_cursor() chunk since Ville did an
equivalent fix in another patch (Ville).
Testcase: igt/pm_rpm/cursor
Testcase: igt/pm_rpm/cursor-dpms
Testcase: igt/pm_rpm/legacy-planes
Testcase: igt/pm_rpm/legacy-planes-dpms
Testcase: igt/pm_rpm/universal-planes
Testcase: igt/pm_rpm/universal-planes-dpms
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645
Cc: sta...@vger.kernel.org
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_display.c | 35 ++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a1cf052..2db9e06 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2149,6 +2149,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
if (need_vtd_wa(dev) && alignment < 256 * 1024)
alignment = 256 * 1024;
+ /*
+* Global gtt pte registers are special registers which actually forward
+* writes to a chunk of system memory. Which means that there is no risk
+* that the register values disappear as soon as we call
+* intel_runtime_pm_put(), so it is correct to wrap only the
+* pin/unpin/fence and not more.
+*/
+ intel_runtime_pm_get(dev_priv);
+
dev_priv->mm.interruptible = false;
ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined);
if (ret)
@@ -2166,21 +2175,30 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
i915_gem_object_pin_fence(obj);
dev_priv->mm.interruptible = true;
+ intel_runtime_pm_put(dev_priv);
return 0;
err_unpin:
i915_gem_object_unpin_from_display_plane(obj);
err_interruptible:
dev_priv->mm.interruptible = true;
+ intel_runtime_pm_put(dev_priv);
return ret;
}
void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
{
- WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
+ struct drm_device *dev = obj->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+
+ intel_runtime_pm_get(dev_priv);
i915_gem_object_unpin_fence(obj);
i915_gem_object_unpin_from_display_plane(obj);
+
+ intel_runtime_pm_put(dev_priv);
}
/* Computes the linear offset to the base tile and adjusts x, y. bytes per
pixel
@@ -8170,6 +8188,16 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
/* we only need to pin inside GTT if cursor is non-phy */
mutex_lock(&dev->struct_mutex);
+
+ /*
+* Global gtt pte registers are special registers which actually forward
+* writes to a chunk of system memory. Which means that there is no risk
+* that the register values disappear as soon as we call
+* intel_runtime_pm_put(), so it is correct to wrap only the
+* pin/unpin/fence and not more.
+*/
+ intel_runtime_pm_get(dev_priv);
+
if (!INTEL_INFO(dev)->cursor_needs_physical) {
unsigned alignment;
@@ -8219,6 +8247,10 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
i915_gem_track_fb(intel_crtc->cursor_bo, obj,
INTEL_FRONTBUFFER_CURSOR(pipe));
+
+ if (obj)
+ intel_runtime_pm_put(dev_priv);
+
mutex_unlock(&dev->struct_mutex);
old_width = intel_crtc->cursor_width;
@@ -8240,6 +8272,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
fail_unpin:
i915_gem_object_unpin_from_display_plane(obj);
fail_locked:
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
fail:
drm_gem_object_unreference_unlocked(&obj->base);
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop
[Intel-gfx] [PATCH 2/5] drm/i915: Do not access stolen memory directly by the CPU, even for error capture
For stolen pages, since it is verboten to access them directly on many
architectures, we have to read them through the GTT aperture. If they
are not accessible through the aperture, then we have to abort.
This was complicated by
commit 8b6124a633d8095b0c8364f585edff9c59568a96
Author: Chris Wilson
Date: Thu Jan 30 14:38:16 2014 +
drm/i915: Don't access snooped pages through the GTT (even for error
capture)
and the desire to use stolen memory for ringbuffers, contexts and
batches in the future.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gpu_error.c | 50 ++-
1 file changed, 31 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 35e70d5..6d280c07 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -561,10 +561,11 @@ static struct drm_i915_error_object *
i915_error_object_create_sized(struct drm_i915_private *dev_priv,
struct drm_i915_gem_object *src,
struct i915_address_space *vm,
- const int num_pages)
+ int num_pages)
{
struct drm_i915_error_object *dst;
- int i;
+ bool use_ggtt;
+ int i = 0;
u32 reloc_offset;
if (src == NULL || src->pages == NULL)
@@ -574,8 +575,32 @@ i915_error_object_create_sized(struct drm_i915_private
*dev_priv,
if (dst == NULL)
return NULL;
- reloc_offset = dst->gtt_offset = i915_gem_obj_offset(src, vm);
- for (i = 0; i < num_pages; i++) {
+ dst->gtt_offset = i915_gem_obj_offset(src, vm);
+
+ reloc_offset = dst->gtt_offset;
+ use_ggtt = (src->cache_level == I915_CACHE_NONE &&
+ i915_is_ggtt(vm) &&
+ src->has_global_gtt_mapping &&
+ reloc_offset + num_pages * PAGE_SIZE <=
dev_priv->gtt.mappable_end);
+
+ /* Cannot access stolen address directly, try to use the aperture */
+ if (src->stolen) {
+ use_ggtt = true;
+
+ if (!src->has_global_gtt_mapping)
+ goto unwind;
+
+ reloc_offset = i915_gem_obj_ggtt_offset(src);
+ if (reloc_offset + num_pages * PAGE_SIZE >
dev_priv->gtt.mappable_end)
+ goto unwind;
+ }
+
+ /* Cannot access snooped pages through the aperture */
+ if (use_ggtt && src->cache_level != I915_CACHE_NONE &&
!HAS_LLC(dev_priv->dev))
+ goto unwind;
+
+ dst->page_count = num_pages;
+ while (num_pages--) {
unsigned long flags;
void *d;
@@ -584,10 +609,7 @@ i915_error_object_create_sized(struct drm_i915_private
*dev_priv,
goto unwind;
local_irq_save(flags);
- if (src->cache_level == I915_CACHE_NONE &&
- reloc_offset < dev_priv->gtt.mappable_end &&
- src->has_global_gtt_mapping &&
- i915_is_ggtt(vm)) {
+ if (use_ggtt) {
void __iomem *s;
/* Simply ignore tiling or any overlapping fence.
@@ -599,14 +621,6 @@ i915_error_object_create_sized(struct drm_i915_private
*dev_priv,
reloc_offset);
memcpy_fromio(d, s, PAGE_SIZE);
io_mapping_unmap_atomic(s);
- } else if (src->stolen) {
- unsigned long offset;
-
- offset = dev_priv->mm.stolen_base;
- offset += src->stolen->start;
- offset += i << PAGE_SHIFT;
-
- memcpy_fromio(d, (void __iomem *) offset, PAGE_SIZE);
} else {
struct page *page;
void *s;
@@ -623,11 +637,9 @@ i915_error_object_create_sized(struct drm_i915_private
*dev_priv,
}
local_irq_restore(flags);
- dst->pages[i] = d;
-
+ dst->pages[i++] = d;
reloc_offset += PAGE_SIZE;
}
- dst->page_count = num_pages;
return dst;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/i915: Suppress a WARN on reading an object back for a GPU hang
Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gpu_error.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 726e6b1..1e05414 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -577,7 +577,10 @@ i915_error_object_create(struct drm_i915_private *dev_priv, if (dst == NULL) return NULL; - dst->gtt_offset = i915_gem_obj_offset(src, vm); + if (i915_gem_obj_bound(src, vm)) + dst->gtt_offset = i915_gem_obj_offset(src, vm); + else + dst->gtt_offset = -1; reloc_offset = dst->gtt_offset; use_ggtt = (src->cache_level == I915_CACHE_NONE && -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915: s/seqno/request/ tracking inside objects
At the heart of this change is that the seqno is a too low level of an
abstraction to handle the growing complexities of command tracking, both
with the introduction of multiple command queues with execbuffer and the
potential for reordering with a scheduler. On top of the seqno we have
the request. Conceptually this is just a fence, but it also has
substantial bookkeeping of its own in order to track the context and
batch in flight, for example. It is the central structure upon which we
can extend with dependency tracking et al.
As regards the objects, they were using the seqno as a simple fence,
upon which is check or even wait upon for command completion. This patch
exchanges that seqno/ring pair with the request itself. For the
majority, lifetime of the request is ordered by how we retire objects
then requests. However, both the unlocked waits and probing elsewhere do
not tie into the normal request lifetimes and so we need to introduce a
kref. Extending the objects to use the request as the fence naturally
extends to segregrating read/write fence tracking. This has significance
for it reduces the number of semaphores we need to emit, reducing the
likelihood of #54226, and improving performance overall.
v2: Rebase and split out the othogonal tweaks.
A silly happened with this patch. It seemed to nullify our earlier
seqno-vs-interrupt w/a. I could not spot why, but gen6+ started to fail
with missed interrupts (a good test of our robustness handling). So I
ripped out the existing ACTHD read and replaced it with a RING_HEAD to
manually check whether the request is complete. That also had the nice
consequence of forcing __wait_request() to being the central arbiter of
request completion.
The keener eyed reviewr will also spot that the reset_counter is moved
into the request simplifing __wait_request() callsites and reducing the
number of atomic reads by virtue of moving the check for a pending GPU
reset to the endpoints of GPU access.
Signed-off-by: Chris Wilson
Cc: Jesse Barnes
Cc: Daniel Vetter
Cc: Oscar Mateo
Cc: Brad Volkin
Cc: "Kukanova, Svetlana"
---
drivers/gpu/drm/i915/i915_debugfs.c | 29 +-
drivers/gpu/drm/i915/i915_dma.c | 2 +
drivers/gpu/drm/i915/i915_drv.h | 121 ++--
drivers/gpu/drm/i915/i915_gem.c | 850 +--
drivers/gpu/drm/i915/i915_gem_context.c | 19 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 37 +-
drivers/gpu/drm/i915/i915_gem_render_state.c | 5 +-
drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +-
drivers/gpu/drm/i915/i915_gpu_error.c| 36 +-
drivers/gpu/drm/i915/i915_irq.c | 28 +-
drivers/gpu/drm/i915/i915_trace.h| 4 +-
drivers/gpu/drm/i915/intel_display.c | 151 ++---
drivers/gpu/drm/i915/intel_drv.h | 8 +-
drivers/gpu/drm/i915/intel_lrc.c | 115 +---
drivers/gpu/drm/i915/intel_overlay.c | 118 ++--
drivers/gpu/drm/i915/intel_ringbuffer.c | 164 +++---
drivers/gpu/drm/i915/intel_ringbuffer.h | 19 +-
17 files changed, 922 insertions(+), 786 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index d42db6b..604a73a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -122,10 +122,11 @@ static inline const char *get_global_flag(struct
drm_i915_gem_object *obj)
static void
describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
{
+ struct i915_gem_request *rq = i915_gem_object_last_read(obj);
struct i915_vma *vma;
int pin_count = 0;
- seq_printf(m, "%pK: %s%s%s %8zdKiB %02x %02x %u %u %u%s%s%s",
+ seq_printf(m, "%pK: %s%s%s %8zdKiB %02x %02x %x %x %x%s%s%s",
&obj->base,
get_pin_flag(obj),
get_tiling_flag(obj),
@@ -133,9 +134,9 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object
*obj)
obj->base.size / 1024,
obj->base.read_domains,
obj->base.write_domain,
- obj->last_read_seqno,
- obj->last_write_seqno,
- obj->last_fenced_seqno,
+ i915_request_seqno(rq),
+ i915_request_seqno(obj->last_write.request),
+ i915_request_seqno(obj->last_fence.request),
i915_cache_level_str(obj->cache_level),
obj->dirty ? " dirty" : "",
obj->madv == I915_MADV_DONTNEED ? " purgeable" : "");
@@ -168,8 +169,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object
*obj)
*t = '\0';
seq_printf(m, " (%s mappable)", s);
}
- if (obj->ring != NULL)
- seq_printf(m, " (%s)", obj->ring->name);
+ if (rq)
+ seq_printf(m, " (%s)", rq->ring->name);
if (obj->frontbuffer_bits)
seq_printf(m, " (frontbuffe
[Intel-gfx] [PATCH 1/5] drm/i915: Print captured bo for all VM in error state
The current error state harks back to the era of just a single VM. For
full-ppgtt, we capture every bo on every VM. It behoves us to then print
every bo for every VM, which we currently fail to do and so miss vital
information in the error state.
v2: Use the vma address rather than -1!
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_gpu_error.c | 80 ---
2 files changed, 58 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1bf2cea..e0dcd70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -396,6 +396,7 @@ struct drm_i915_error_state {
pid_t pid;
char comm[TASK_COMM_LEN];
} ring[I915_NUM_RINGS];
+
struct drm_i915_error_buffer {
u32 size;
u32 name;
@@ -414,6 +415,7 @@ struct drm_i915_error_state {
} **active_bo, **pinned_bo;
u32 *active_bo_count, *pinned_bo_count;
+ u32 vm_count;
};
struct intel_connector;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index fc11ac6..35e70d5 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -192,10 +192,10 @@ static void print_error_buffers(struct
drm_i915_error_state_buf *m,
struct drm_i915_error_buffer *err,
int count)
{
- err_printf(m, "%s [%d]:\n", name, count);
+ err_printf(m, " %s [%d]:\n", name, count);
while (count--) {
- err_printf(m, " %08x %8u %02x %02x %x %x",
+ err_printf(m, "%08x %8u %02x %02x %x %x",
err->gtt_offset,
err->size,
err->read_domains,
@@ -393,15 +393,17 @@ int i915_error_state_to_str(struct
drm_i915_error_state_buf *m,
i915_ring_error_state(m, dev, &error->ring[i]);
}
- if (error->active_bo)
+ for (i = 0; i < error->vm_count; i++) {
+ err_printf(m, "vm[%d]\n", i);
+
print_error_buffers(m, "Active",
- error->active_bo[0],
- error->active_bo_count[0]);
+ error->active_bo[i],
+ error->active_bo_count[i]);
- if (error->pinned_bo)
print_error_buffers(m, "Pinned",
- error->pinned_bo[0],
- error->pinned_bo_count[0]);
+ error->pinned_bo[i],
+ error->pinned_bo_count[i]);
+ }
for (i = 0; i < ARRAY_SIZE(error->ring); i++) {
obj = error->ring[i].batchbuffer;
@@ -644,13 +646,15 @@ unwind:
(src)->base.size>>PAGE_SHIFT)
static void capture_bo(struct drm_i915_error_buffer *err,
- struct drm_i915_gem_object *obj)
+ struct i915_vma *vma)
{
+ struct drm_i915_gem_object *obj = vma->obj;
+
err->size = obj->base.size;
err->name = obj->base.name;
err->rseqno = obj->last_read_seqno;
err->wseqno = obj->last_write_seqno;
- err->gtt_offset = i915_gem_obj_ggtt_offset(obj);
+ err->gtt_offset = vma->node.start;
err->read_domains = obj->base.read_domains;
err->write_domain = obj->base.write_domain;
err->fence_reg = obj->fence_reg;
@@ -674,7 +678,7 @@ static u32 capture_active_bo(struct drm_i915_error_buffer
*err,
int i = 0;
list_for_each_entry(vma, head, mm_list) {
- capture_bo(err++, vma->obj);
+ capture_bo(err++, vma);
if (++i == count)
break;
}
@@ -683,21 +687,27 @@ static u32 capture_active_bo(struct drm_i915_error_buffer
*err,
}
static u32 capture_pinned_bo(struct drm_i915_error_buffer *err,
-int count, struct list_head *head)
+int count, struct list_head *head,
+struct i915_address_space *vm)
{
struct drm_i915_gem_object *obj;
- int i = 0;
+ struct drm_i915_error_buffer * const first = err;
+ struct drm_i915_error_buffer * const last = err + count;
list_for_each_entry(obj, head, global_list) {
- if (!i915_gem_obj_is_pinned(obj))
- continue;
+ struct i915_vma *vma;
- capture_bo(err++, obj);
- if (++i == count)
+ if (err == last)
break;
+
+ list_for_each_entry(vma, &obj->vma_list, vma_link)
+ if (vma->vm == vm && vma->pin_count > 0) {
+ capture_
[Intel-gfx] [PATCH 3/5] drm/i915: Remove num_pages parameter to i915_error_object_create()
For cleanliness, i915_error_object_create() was written to handle the
NULL pointer in a central location. The macro that wrapped it and passed
it a num_pages to use, was not safe. As we now never limit the num_pages
to use (we did so at one point to only capture the first page of the
context), we can remove the redundant macro and be NULL safe again.
Signed-off-by: Chris Wilson
Cc: Jesse Barnes
Cc: John Harrison
---
drivers/gpu/drm/i915/i915_gpu_error.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 6d280c07..726e6b1 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -558,12 +558,12 @@ static void i915_error_state_free(struct kref *error_ref)
}
static struct drm_i915_error_object *
-i915_error_object_create_sized(struct drm_i915_private *dev_priv,
- struct drm_i915_gem_object *src,
- struct i915_address_space *vm,
- int num_pages)
+i915_error_object_create(struct drm_i915_private *dev_priv,
+struct drm_i915_gem_object *src,
+struct i915_address_space *vm)
{
struct drm_i915_error_object *dst;
+ int num_pages;
bool use_ggtt;
int i = 0;
u32 reloc_offset;
@@ -571,6 +571,8 @@ i915_error_object_create_sized(struct drm_i915_private
*dev_priv,
if (src == NULL || src->pages == NULL)
return NULL;
+ num_pages = src->base.size >> PAGE_SHIFT;
+
dst = kmalloc(sizeof(*dst) + num_pages * sizeof(u32 *), GFP_ATOMIC);
if (dst == NULL)
return NULL;
@@ -649,13 +651,8 @@ unwind:
kfree(dst);
return NULL;
}
-#define i915_error_object_create(dev_priv, src, vm) \
- i915_error_object_create_sized((dev_priv), (src), (vm), \
- (src)->base.size>>PAGE_SHIFT)
-
#define i915_error_ggtt_object_create(dev_priv, src) \
- i915_error_object_create_sized((dev_priv), (src),
&(dev_priv)->gtt.base, \
- (src)->base.size>>PAGE_SHIFT)
+ i915_error_object_create((dev_priv), (src), &(dev_priv)->gtt.base)
static void capture_bo(struct drm_i915_error_buffer *err,
struct i915_vma *vma)
@@ -1004,8 +1001,7 @@ static void i915_gem_record_rings(struct drm_device *dev,
request->batch_obj,
vm);
- if (HAS_BROKEN_CS_TLB(dev_priv->dev) &&
- ring->scratch.obj)
+ if (HAS_BROKEN_CS_TLB(dev_priv->dev))
error->ring[i].wa_batchbuffer =
i915_error_ggtt_object_create(dev_priv,
ring->scratch.obj);
@@ -1027,9 +1023,8 @@ static void i915_gem_record_rings(struct drm_device *dev,
error->ring[i].ringbuffer =
i915_error_ggtt_object_create(dev_priv,
ring->buffer->obj);
- if (ring->status_page.obj)
- error->ring[i].hws_page =
- i915_error_ggtt_object_create(dev_priv,
ring->status_page.obj);
+ error->ring[i].hws_page =
+ i915_error_ggtt_object_create(dev_priv,
ring->status_page.obj);
i915_gem_record_active_context(ring, error, &error->ring[i]);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended
On Tue, Aug 12, 2014 at 03:55:12PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni
>
> If we're runtime suspended and try to use the plane interfaces, we
> will get a lot of WARNs saying we did the wrong thing.
>
> We need to get runtime PM references to pin/unpin the objects, and to
> change the fences. The pin/unpin functions are the ideal places for
> this, but intel_crtc_cursor_set_obj() doesn't call them, so we also
> have to add get/put calls inside it. There is no problem if we runtime
> suspend right after these functions are finished, because the
> registers written are forwarded to system memory.
>
> Note: for a complete fix of the cursor-dpms test case, we also need
> the patch named "drm/i915: Don't try to enable cursor from setplane
> when crtc is disabled".
>
> v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel)
> v3: - Make get/put also surround the fence and unpin calls (Daniel and
> Ville).
> - Merge all the plane changes into a single patch since they're
> the same fix.
> - Add the comment requested by Daniel.
> v4: - Remove spurious whitespace (Ville).
> v5: - Remove intel_crtc_update_cursor() chunk since Ville did an
> equivalent fix in another patch (Ville).
>
> Testcase: igt/pm_rpm/cursor
> Testcase: igt/pm_rpm/cursor-dpms
> Testcase: igt/pm_rpm/legacy-planes
> Testcase: igt/pm_rpm/legacy-planes-dpms
> Testcase: igt/pm_rpm/universal-planes
> Testcase: igt/pm_rpm/universal-planes-dpms
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645
> Cc: sta...@vger.kernel.org
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/intel_display.c | 35 ++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index a1cf052..2db9e06 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2149,6 +2149,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
> if (need_vtd_wa(dev) && alignment < 256 * 1024)
> alignment = 256 * 1024;
>
> + /*
> + * Global gtt pte registers are special registers which actually forward
> + * writes to a chunk of system memory. Which means that there is no risk
> + * that the register values disappear as soon as we call
> + * intel_runtime_pm_put(), so it is correct to wrap only the
> + * pin/unpin/fence and not more.
> + */
> + intel_runtime_pm_get(dev_priv);
> +
> dev_priv->mm.interruptible = false;
> ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined);
> if (ret)
> @@ -2166,21 +2175,30 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
> i915_gem_object_pin_fence(obj);
>
> dev_priv->mm.interruptible = true;
> + intel_runtime_pm_put(dev_priv);
> return 0;
>
> err_unpin:
> i915_gem_object_unpin_from_display_plane(obj);
> err_interruptible:
> dev_priv->mm.interruptible = true;
> + intel_runtime_pm_put(dev_priv);
> return ret;
> }
>
> void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
> {
> - WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
> + struct drm_device *dev = obj->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> +
> + intel_runtime_pm_get(dev_priv);
>
> i915_gem_object_unpin_fence(obj);
> i915_gem_object_unpin_from_display_plane(obj);
> +
> + intel_runtime_pm_put(dev_priv);
> }
framebuffer objects are pinned for a very long time, and the fbcon is
permanently pinned. This should have the effect of disabling rpm
entirely.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended
2014-08-12 16:09 GMT-03:00 Chris Wilson :
> On Tue, Aug 12, 2014 at 03:55:12PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni
>>
>> If we're runtime suspended and try to use the plane interfaces, we
>> will get a lot of WARNs saying we did the wrong thing.
>>
>> We need to get runtime PM references to pin/unpin the objects, and to
>> change the fences. The pin/unpin functions are the ideal places for
>> this, but intel_crtc_cursor_set_obj() doesn't call them, so we also
>> have to add get/put calls inside it. There is no problem if we runtime
>> suspend right after these functions are finished, because the
>> registers written are forwarded to system memory.
>>
>> Note: for a complete fix of the cursor-dpms test case, we also need
>> the patch named "drm/i915: Don't try to enable cursor from setplane
>> when crtc is disabled".
>>
>> v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel)
>> v3: - Make get/put also surround the fence and unpin calls (Daniel and
>> Ville).
>> - Merge all the plane changes into a single patch since they're
>> the same fix.
>> - Add the comment requested by Daniel.
>> v4: - Remove spurious whitespace (Ville).
>> v5: - Remove intel_crtc_update_cursor() chunk since Ville did an
>> equivalent fix in another patch (Ville).
>>
>> Testcase: igt/pm_rpm/cursor
>> Testcase: igt/pm_rpm/cursor-dpms
>> Testcase: igt/pm_rpm/legacy-planes
>> Testcase: igt/pm_rpm/legacy-planes-dpms
>> Testcase: igt/pm_rpm/universal-planes
>> Testcase: igt/pm_rpm/universal-planes-dpms
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Paulo Zanoni
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 35
>> ++-
>> 1 file changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index a1cf052..2db9e06 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2149,6 +2149,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
>> if (need_vtd_wa(dev) && alignment < 256 * 1024)
>> alignment = 256 * 1024;
>>
>> + /*
>> + * Global gtt pte registers are special registers which actually
>> forward
>> + * writes to a chunk of system memory. Which means that there is no
>> risk
>> + * that the register values disappear as soon as we call
>> + * intel_runtime_pm_put(), so it is correct to wrap only the
>> + * pin/unpin/fence and not more.
>> + */
>> + intel_runtime_pm_get(dev_priv);
>> +
>> dev_priv->mm.interruptible = false;
>> ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined);
>> if (ret)
>> @@ -2166,21 +2175,30 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
>> i915_gem_object_pin_fence(obj);
>>
>> dev_priv->mm.interruptible = true;
>> + intel_runtime_pm_put(dev_priv);
>> return 0;
>>
>> err_unpin:
>> i915_gem_object_unpin_from_display_plane(obj);
>> err_interruptible:
>> dev_priv->mm.interruptible = true;
>> + intel_runtime_pm_put(dev_priv);
>> return ret;
>> }
>>
>> void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
>> {
>> - WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
>> + struct drm_device *dev = obj->base.dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> + WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>> +
>> + intel_runtime_pm_get(dev_priv);
>>
>> i915_gem_object_unpin_fence(obj);
>> i915_gem_object_unpin_from_display_plane(obj);
>> +
>> + intel_runtime_pm_put(dev_priv);
>> }
>
> framebuffer objects are pinned for a very long time, and the fbcon is
> permanently pinned. This should have the effect of disabling rpm
> entirely.
But we just get/put RPM around this function, not for the whole time
while the object is pinned.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended
On Tue, Aug 12, 2014 at 04:12:38PM -0300, Paulo Zanoni wrote: > But we just get/put RPM around this function, not for the whole time > while the object is pinned. Ah misread, saw pin->get, unpin->put and assumed the symmetry. But why unpin then? It doesn't touch any hardware registers. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended
2014-08-12 16:28 GMT-03:00 Chris Wilson : > On Tue, Aug 12, 2014 at 04:12:38PM -0300, Paulo Zanoni wrote: >> But we just get/put RPM around this function, not for the whole time >> while the object is pinned. > > Ah misread, saw pin->get, unpin->put and assumed the symmetry. But why > unpin then? It doesn't touch any hardware registers. Only because Daniel asked it on a conversation we had on IRC, and I automatically assumed the patch would be rejected if I didn't include it :) Since both you and VIlle pointed that out, I should probably submit yet another version, without the unpin part, and let Daniel choose which one to merge... > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended
On Tue, Aug 12, 2014 at 9:33 PM, Paulo Zanoni wrote: > 2014-08-12 16:28 GMT-03:00 Chris Wilson : >> On Tue, Aug 12, 2014 at 04:12:38PM -0300, Paulo Zanoni wrote: >>> But we just get/put RPM around this function, not for the whole time >>> while the object is pinned. >> >> Ah misread, saw pin->get, unpin->put and assumed the symmetry. But why >> unpin then? It doesn't touch any hardware registers. > > Only because Daniel asked it on a conversation we had on IRC, and I > automatically assumed the patch would be rejected if I didn't include > it :) > > Since both you and VIlle pointed that out, I should probably submit > yet another version, without the unpin part, and let Daniel choose > which one to merge... Hm, I've indeed forgotten about the lazy unbinding. But that poses the question about the final bo unref. For example: 1) create bo, gtt mmap it to force it into existence (and into the global gtt) 2) unmap binding 3) wait for rpm entry 4) unref bo ... causing pte writes for the global gtt unbinding while runtime suspended or not? boom or not boom? Maybe the bug is simply in a different function ;-) -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: fix plane/cursor handling when runtime suspended
On Tue, Aug 12, 2014 at 10:19:20PM +0200, Daniel Vetter wrote: > On Tue, Aug 12, 2014 at 9:33 PM, Paulo Zanoni wrote: > > 2014-08-12 16:28 GMT-03:00 Chris Wilson : > >> On Tue, Aug 12, 2014 at 04:12:38PM -0300, Paulo Zanoni wrote: > >>> But we just get/put RPM around this function, not for the whole time > >>> while the object is pinned. > >> > >> Ah misread, saw pin->get, unpin->put and assumed the symmetry. But why > >> unpin then? It doesn't touch any hardware registers. > > > > Only because Daniel asked it on a conversation we had on IRC, and I > > automatically assumed the patch would be rejected if I didn't include > > it :) > > > > Since both you and VIlle pointed that out, I should probably submit > > yet another version, without the unpin part, and let Daniel choose > > which one to merge... > > Hm, I've indeed forgotten about the lazy unbinding. But that poses the > question about the final bo unref. For example: > 1) create bo, gtt mmap it to force it into existence (and into the global gtt) > 2) unmap binding > 3) wait for rpm entry > 4) unref bo ... causing pte writes for the global gtt unbinding while > runtime suspended or not? > > boom or not boom? > > Maybe the bug is simply in a different function ;-) Yes. If you get serious about it, you will want to move the lazy stuff into its own workqueue to be run the next time the device is awake. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended
On Tue, Aug 12, 2014 at 10:30 PM, Chris Wilson wrote: > On Tue, Aug 12, 2014 at 10:19:20PM +0200, Daniel Vetter wrote: >> On Tue, Aug 12, 2014 at 9:33 PM, Paulo Zanoni wrote: >> > 2014-08-12 16:28 GMT-03:00 Chris Wilson : >> >> On Tue, Aug 12, 2014 at 04:12:38PM -0300, Paulo Zanoni wrote: >> >>> But we just get/put RPM around this function, not for the whole time >> >>> while the object is pinned. >> >> >> >> Ah misread, saw pin->get, unpin->put and assumed the symmetry. But why >> >> unpin then? It doesn't touch any hardware registers. >> > >> > Only because Daniel asked it on a conversation we had on IRC, and I >> > automatically assumed the patch would be rejected if I didn't include >> > it :) >> > >> > Since both you and VIlle pointed that out, I should probably submit >> > yet another version, without the unpin part, and let Daniel choose >> > which one to merge... >> >> Hm, I've indeed forgotten about the lazy unbinding. But that poses the >> question about the final bo unref. For example: >> 1) create bo, gtt mmap it to force it into existence (and into the global >> gtt) >> 2) unmap binding >> 3) wait for rpm entry >> 4) unref bo ... causing pte writes for the global gtt unbinding while >> runtime suspended or not? >> >> boom or not boom? >> >> Maybe the bug is simply in a different function ;-) > > Yes. If you get serious about it, you will want to move the lazy stuff > into its own workqueue to be run the next time the device is awake. 4b) shrinker happens and unbinds (potentially purgeable) buffer objects. In that case I don't think the core mm would be happy if we'd indefinitely delay this until someone wiggles the mouse. Especially if the compositor wants that memory to render the frame it needs to switch everything on 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: fix plane/cursor handling when runtime suspended
On Tue, Aug 12, 2014 at 10:37:21PM +0200, Daniel Vetter wrote: > On Tue, Aug 12, 2014 at 10:30 PM, Chris Wilson > wrote: > > On Tue, Aug 12, 2014 at 10:19:20PM +0200, Daniel Vetter wrote: > >> On Tue, Aug 12, 2014 at 9:33 PM, Paulo Zanoni wrote: > >> > 2014-08-12 16:28 GMT-03:00 Chris Wilson : > >> >> On Tue, Aug 12, 2014 at 04:12:38PM -0300, Paulo Zanoni wrote: > >> >>> But we just get/put RPM around this function, not for the whole time > >> >>> while the object is pinned. > >> >> > >> >> Ah misread, saw pin->get, unpin->put and assumed the symmetry. But why > >> >> unpin then? It doesn't touch any hardware registers. > >> > > >> > Only because Daniel asked it on a conversation we had on IRC, and I > >> > automatically assumed the patch would be rejected if I didn't include > >> > it :) > >> > > >> > Since both you and VIlle pointed that out, I should probably submit > >> > yet another version, without the unpin part, and let Daniel choose > >> > which one to merge... > >> > >> Hm, I've indeed forgotten about the lazy unbinding. But that poses the > >> question about the final bo unref. For example: > >> 1) create bo, gtt mmap it to force it into existence (and into the global > >> gtt) > >> 2) unmap binding > >> 3) wait for rpm entry > >> 4) unref bo ... causing pte writes for the global gtt unbinding while > >> runtime suspended or not? > >> > >> boom or not boom? > >> > >> Maybe the bug is simply in a different function ;-) > > > > Yes. If you get serious about it, you will want to move the lazy stuff > > into its own workqueue to be run the next time the device is awake. > > 4b) shrinker happens and unbinds (potentially purgeable) buffer objects. > > In that case I don't think the core mm would be happy if we'd > indefinitely delay this until someone wiggles the mouse. You underestimate just how much we can delay it ;-) But for your next trick, you could unbind the buffer without touching the ptes since the gpu is not using those pages... Diminishing returns I guess. > Especially if > the compositor wants that memory to render the frame it needs to > switch everything on again ... But's true without rpm anyway. It would need to enable the device to render, whether or not the system is thrashing. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ 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: Support for HDMI complaince HPD
> I know. That is orthogonal to the tweaks I was suggesting. Also if you > feel you need to add details to your rationale, then your changelog is > incomplete. > -Chris Thanks Chris, Please find my comments inline to your previous mail, with suggestions. On 8/12/2014 6:17 PM, Chris Wilson wrote: On Tue, Aug 12, 2014 at 06:08:21PM +0530, shashank.sha...@intel.com wrote: From: Shashank Sharma During the HDMI complaince tests, most of the HDMI analyzers issue a soft HPD, to automate the tests. This process keeps the HDMI cable connected, and DDC chhanel alive. HDMI detect() logic relies on EDID readability, to decide if its a HDMI connect or disconnect event. But in this case, the EDID is always readable, as HDMI cable is physically connected and DDC channel is UP, so detect() always reports a HDMI connect even if its intended to be a HDMI disconnect call. So if HDMI compliance is enabled, we should rely on the live status register, than EDID availability. This patch adds: 1. One kernel command line parameter for i915 module, which indicates if we want to support HDMI compliance, for this platform. I would rather have this as an output property. In fact, I would like the hotplug detection method exposed (and even selectable, but other than this compliance testing, I can't think of a scenario where the kernel shouldn't be able to figure things out for itself). Considering the history of the case, can you please elaborate this suggestion ? I dont think I am getting it right. 2. A new function to read EDID, which gets called only in case of HDMI compliance support is required. This function reads EDID only if live status register is up. The normal code flow doesn't get effected if kernel command line parameter is not enabled. 3. After various experiments on VLV2 board, with various HDMI monitors we have seen that, with few monitors, the live status register gets set after a slight delay, and then stays reliably. To support such monitors, there is a busy-loop added, with a max delay upto 50ms, with a status check after every 10ms. Please see the comment in intel_hdmi_get_edid. Wouldn't a tidier solution be to delay the hpd by 50-100ms after the hotplug interrupt? That may overcome the issue with the live status for all connectors... -Chris There would be few problems in this case: 1. We dont want this scenario to come into picture for DP, as DP HPD pulse can be as small as 2ms. 2. Not all the HDMI monitors show this problem, but a significant subset of popular monitors do. 3. In HDCP compliance testing, we send a HPD pulse train of UP and Down, where down pulse can be as small as 100ms. If we increase the delay by 100ms, we will definitely miss the HPD down pulse. 4. The method what we are using is a busy waiting check, where we delay the pulse for 50ms, but take a sample of live_status per 10ms, so if the live status is up with a delay of 20ms, we needn't to waste another 30. 5. We want this code routine only to be executed for commercial (like android) platforms, whereas others get the routine code. @ Danvet: Do you want to add something here ? Regards Shashank ___ 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: Support for HDMI complaince HPD
On Wed, Aug 13, 2014 at 11:34:02AM +0530, Sharma, Shashank wrote: > > I know. That is orthogonal to the tweaks I was suggesting. Also if you > > feel you need to add details to your rationale, then your changelog is > > incomplete. > > -Chris > > Thanks Chris, > Please find my comments inline to your previous mail, with suggestions. > > On 8/12/2014 6:17 PM, Chris Wilson wrote: > >On Tue, Aug 12, 2014 at 06:08:21PM +0530, shashank.sha...@intel.com wrote: > >>From: Shashank Sharma > >> > >>During the HDMI complaince tests, most of the HDMI analyzers > >>issue a soft HPD, to automate the tests. This process keeps > >>the HDMI cable connected, and DDC chhanel alive. > >> > >>HDMI detect() logic relies on EDID readability, to decide if > >>its a HDMI connect or disconnect event. But in this case, the > >>EDID is always readable, as HDMI cable is physically connected > >>and DDC channel is UP, so detect() always reports a HDMI connect > >>even if its intended to be a HDMI disconnect call. > >> > >>So if HDMI compliance is enabled, we should rely on the live status > >>register, than EDID availability. This patch adds: > >>1. One kernel command line parameter for i915 module, which indicates > >>if we want to support HDMI compliance, for this platform. > > > >I would rather have this as an output property. In fact, I would like > >the hotplug detection method exposed (and even selectable, but other > >than this compliance testing, I can't think of a scenario where the > >kernel shouldn't be able to figure things out for itself). > Considering the history of the case, can you please elaborate this > suggestion ? I dont think I am getting it right. Instead of (or in addition to) adding a kernel parameter, you add an output property so that it can be adjusted on the fly for individual monitors. > > > >>2. A new function to read EDID, which gets called only in case of > >>HDMI compliance support is required. This function reads EDID only > >>if live status register is up. The normal code flow doesn't get effected > >>if kernel command line parameter is not enabled. > >>3. After various experiments on VLV2 board, with various HDMI monitors > >>we have seen that, with few monitors, the live status register gets > >>set after a slight delay, and then stays reliably. To support such > >>monitors, there is a busy-loop added, with a max delay upto 50ms, > >>with a status check after every 10ms. Please see the comment in > >>intel_hdmi_get_edid. > > > >Wouldn't a tidier solution be to delay the hpd by 50-100ms after the > >hotplug interrupt? That may overcome the issue with the live status for > >all connectors... > >-Chris > > > There would be few problems in this case: > 1. We dont want this scenario to come into picture for DP, as DP HPD >pulse can be as small as 2ms. If the live status is asserted and deasserted within 2ms, do you care? Or perhaps you are talking about something else entirely. > 2. Not all the HDMI monitors show this problem, but a significant >subset of popular monitors do. > 3. In HDCP compliance testing, we send a HPD pulse train of UP and >Down, where down pulse can be as small as 100ms. If we increase the >delay by 100ms, we will definitely miss the HPD down pulse. And? If the monitor is only plugged in for less than 0.1s do I really want to waste 1-2s of user time reconfiguring the desktop and applications before undoing all the changes? There is no point in compliance testing if it does not actually test the code going to be used. > 4. The method what we are using is a busy waiting check, where we delay >the pulse for 50ms, but take a sample of live_status per 10ms, so if >the live status is up with a delay of 20ms, we needn't to waste >another 30. Yes. You block using 100% of the cpu in an uninterruptable context for a significant period of time. DO NOT DO THIS. > 5. We want this code routine only to be executed for commercial (like >android) platforms, whereas others get the routine code. In other words, you want to ignore years of real world compatibity testing and larger user bases. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
