Re: [Intel-gfx] gstreamer-vaapi does not work with g45
But isn't this a decision to be made by the mainline maintainers? Obviously, but why shouldn't they reject a widely requested feature when the code is mature enough? There was nothing ever rejected, as no patch was ever recieved. So I was told on the MPlayer-users mailing list. MPlayer developers do not crawl the web in search for possible improvements. To get a new feature included in MPlayer, it is best to send a patch to MPlayer-dev-eng list, like it was done with vdpau. I can't do that, I'm no programmer of the C family of languages. Sorry. Greets, Kiste ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: clarify IBX dp workaround
Instead of checking for !CPT, check for IBX to make it clearer that
this is a IBX-specific workaround. No functional change because we
smash the PPT PCH into the HAS_PCH_CPT check and atm DP isn't enabled
on the haswell LPT PCH yet.
See Bspec Vol 3, Part 3, Section 4.[3-5].1 about DP[BCD], bit 30 for
details of this workaround.
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9b2effc..df3b027 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1910,7 +1910,7 @@ intel_dp_link_down(struct intel_dp *intel_dp)
DP |= DP_LINK_TRAIN_OFF;
}
- if (!HAS_PCH_CPT(dev)
+ if (HAS_PCH_IBX(dev)
I915_READ(intel_dp-output_reg) DP_PIPEB_SELECT) {
struct drm_crtc *crtc = intel_dp-base.base.crtc;
--
1.7.7.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: implement IBX hdmi transcoder select workaround
Bspec Vol 3, Part 3, Section 3.8.1.1, bit 30:
[DevIBX] Writing to this bit only takes effect when port is enabled.
Due to hardware issue it is required that this bit be cleared when port
is disabled. To clear this bit software must temporarily enable this
port on transcoder A.
Unfortunately the public Bspec misses totally out on the same language
for HDMIB. Internal Bspec also mentions that one of the bad
side-effects is that DPx can fail to light up on transcoder A if HDMIx
is disabled but using transcoder B.
I've found this while reviewing Bsepc. We already implement the same
workaround for the DP ports.
Also replace a magic 1 with PIPE_B I've found while looking through the
code.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_hdmi.c | 29 -
1 files changed, 28 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 4c6f141..4ebdcf1 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -382,7 +382,7 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder,
if (HAS_PCH_CPT(dev))
sdvox |= PORT_TRANS_SEL_CPT(intel_crtc-pipe);
- else if (intel_crtc-pipe == 1)
+ else if (intel_crtc-pipe == PIPE_B)
sdvox |= SDVO_PIPE_B_SELECT;
I915_WRITE(intel_hdmi-sdvox_reg, sdvox);
@@ -405,6 +405,33 @@ static void intel_hdmi_dpms(struct drm_encoder *encoder,
int mode)
temp = I915_READ(intel_hdmi-sdvox_reg);
+ /* HW workaround for IBX, we need to move the port to transcoder A
+* before disabling it. */
+ if (HAS_PCH_IBX(dev)) {
+ struct drm_crtc *crtc = encoder-crtc;
+ struct intel_crtc *intel_crtc;
+
+ if (crtc)
+ intel_crtc = to_intel_crtc(crtc);
+ else
+ intel_crtc = NULL;
+
+ if (mode != DRM_MODE_DPMS_ON (temp SDVO_PIPE_B_SELECT)) {
+ temp = ~SDVO_PIPE_B_SELECT;
+ I915_WRITE(intel_hdmi-sdvox_reg, temp);
+ POSTING_READ(intel_hdmi-sdvox_reg);
+
+ if (intel_crtc)
+ intel_wait_for_vblank(dev, intel_crtc-pipe);
+ else
+ msleep(50);
+ } else if (mode == DRM_MODE_DPMS_ON){
+ /* Restore the transcoder select bit. */
+ if (intel_crtc-pipe == PIPE_B)
+ enable_bits |= SDVO_PIPE_B_SELECT;
+ }
+ }
+
/* HW workaround, need to toggle enable bit off and on for 12bpc, but
* we do this anyway which shows more stable in testing.
*/
--
1.7.7.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: assert that the IBX port transcoder select w/a is implemented
Let's be a bit more paranoid here. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3c71850..a9bc673 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1224,6 +1224,9 @@ static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv, WARN(dp_pipe_enabled(dev_priv, pipe, port_sel, val), PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n, reg, pipe_name(pipe)); + + WARN(HAS_PCH_IBX(dev_priv-dev) (val SDVO_PIPE_B_SELECT), +IBX PCH dp port still using transcoder B\n); } static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv, @@ -1233,6 +1236,9 @@ static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv, WARN(hdmi_pipe_enabled(dev_priv, val, pipe), PCH HDMI (0x%08x) enabled on transcoder %c, should be disabled\n, reg, pipe_name(pipe)); + + WARN(HAS_PCH_IBX(dev_priv-dev) (val SDVO_PIPE_B_SELECT), +IBX PCH hdmi port still using transcoder B\n); } static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, -- 1.7.7.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: don't chnage the original mode in dp_mode_fixup
We should only frob adjusted_mode. This is in preparation of
a massive patch by Laurent Pinchart to make the mode argument
const.
The only thing we actually touch is mode-clock, but only if
it's a panel. And in that case we also set adjusted_mode-clock
to the same value. All the generic code already uses the
adjusted_mode exclusively, so we only have to move the dp
link bw calculations over to that. This requires a small
changes so that the shared code with mode_valid doesn't
touch the mode argument.
Also mark the mode argument of pch_panel_fitting const.
Reported-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c| 19 +++
drivers/gpu/drm/i915/intel_drv.h |2 +-
drivers/gpu/drm/i915/intel_panel.c |2 +-
3 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 296cfc2..05c4748 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -222,7 +222,7 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
static bool
intel_dp_adjust_dithering(struct intel_dp *intel_dp,
struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ bool adjust_mode)
{
int max_link_clock =
intel_dp_link_clock(intel_dp_max_link_bw(intel_dp));
int max_lanes = intel_dp_max_lane_count(intel_dp);
@@ -236,8 +236,8 @@ intel_dp_adjust_dithering(struct intel_dp *intel_dp,
if (mode_rate max_rate)
return false;
- if (adjusted_mode)
- adjusted_mode-private_flags
+ if (adjust_mode)
+ mode-private_flags
|= INTEL_MODE_DP_FORCE_6BPC;
return true;
@@ -260,7 +260,7 @@ intel_dp_mode_valid(struct drm_connector *connector,
return MODE_PANEL;
}
- if (!intel_dp_adjust_dithering(intel_dp, mode, NULL))
+ if (!intel_dp_adjust_dithering(intel_dp, mode, false))
return MODE_CLOCK_HIGH;
if (mode-clock 1)
@@ -698,25 +698,20 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct
drm_display_mode *mode,
intel_fixed_panel_mode(intel_dp-panel_fixed_mode,
adjusted_mode);
intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
mode, adjusted_mode);
- /*
-* the mode-clock is used to calculate the DataLink M/N
-* of the pipe. For the eDP the fixed clock should be used.
-*/
- mode-clock = intel_dp-panel_fixed_mode-clock;
}
- if (mode-flags DRM_MODE_FLAG_DBLCLK)
+ if (adjusted_mode-flags DRM_MODE_FLAG_DBLCLK)
return false;
DRM_DEBUG_KMS(DP link computation with max lane count %i
max bw %02x pixel clock %iKHz\n,
max_lane_count, bws[max_clock], mode-clock);
- if (!intel_dp_adjust_dithering(intel_dp, mode, adjusted_mode))
+ if (!intel_dp_adjust_dithering(intel_dp, adjusted_mode, true))
return false;
bpp = adjusted_mode-private_flags INTEL_MODE_DP_FORCE_6BPC ? 18 : 24;
- mode_rate = intel_dp_link_required(mode-clock, bpp);
+ mode_rate = intel_dp_link_required(adjusted_mode-clock, bpp);
for (lane_count = 1; lane_count = max_lane_count; lane_count = 1) {
for (clock = 0; clock = max_clock; clock++) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3e09188..3e3b60c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -372,7 +372,7 @@ extern void intel_fixed_panel_mode(struct drm_display_mode
*fixed_mode,
struct drm_display_mode *adjusted_mode);
extern void intel_pch_panel_fitting(struct drm_device *dev,
int fitting_mode,
- struct drm_display_mode *mode,
+ const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode);
extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
extern u32 intel_panel_get_backlight(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 2a1625d..7180cc8 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -56,7 +56,7 @@ intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
void
intel_pch_panel_fitting(struct drm_device *dev,
int fitting_mode,
- struct drm_display_mode *mode,
+ const struct drm_display_mode
Re: [Intel-gfx] [PATCH 3/3] drm/i915: assert that the IBX port transcoder select w/a is implemented
On Wed, 30 May 2012 12:31:58 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: Let's be a bit more paranoid here. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3c71850..a9bc673 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1224,6 +1224,9 @@ static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv, WARN(dp_pipe_enabled(dev_priv, pipe, port_sel, val), PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n, reg, pipe_name(pipe)); + + WARN(HAS_PCH_IBX(dev_priv-dev) (val SDVO_PIPE_B_SELECT), + IBX PCH dp port still using transcoder B\n); Only an issue if both pipe B selected and enabled, right? -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 1/3] drm/i915: clarify IBX dp workaround
On Wed, 30 May 2012 12:31:56 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: Instead of checking for !CPT, check for IBX to make it clearer that this is a IBX-specific workaround. No functional change because we smash the PPT PCH into the HAS_PCH_CPT check and atm DP isn't enabled on the haswell LPT PCH yet. See Bspec Vol 3, Part 3, Section 4.[3-5].1 about DP[BCD], bit 30 for details of this workaround. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -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/3] drm/i915: implement IBX hdmi transcoder select workaround
On Wed, 30 May 2012 12:31:57 +0200, Daniel Vetter daniel.vet...@ffwll.ch
wrote:
Bspec Vol 3, Part 3, Section 3.8.1.1, bit 30:
[DevIBX] Writing to this bit only takes effect when port is enabled.
Due to hardware issue it is required that this bit be cleared when port
is disabled. To clear this bit software must temporarily enable this
port on transcoder A.
Unfortunately the public Bspec misses totally out on the same language
for HDMIB. Internal Bspec also mentions that one of the bad
side-effects is that DPx can fail to light up on transcoder A if HDMIx
is disabled but using transcoder B.
I've found this while reviewing Bsepc. We already implement the same
workaround for the DP ports.
Also replace a magic 1 with PIPE_B I've found while looking through the
code.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_hdmi.c | 29 -
1 files changed, 28 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 4c6f141..4ebdcf1 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -382,7 +382,7 @@ static void intel_hdmi_mode_set(struct drm_encoder
*encoder,
if (HAS_PCH_CPT(dev))
sdvox |= PORT_TRANS_SEL_CPT(intel_crtc-pipe);
- else if (intel_crtc-pipe == 1)
+ else if (intel_crtc-pipe == PIPE_B)
sdvox |= SDVO_PIPE_B_SELECT;
I915_WRITE(intel_hdmi-sdvox_reg, sdvox);
@@ -405,6 +405,33 @@ static void intel_hdmi_dpms(struct drm_encoder *encoder,
int mode)
temp = I915_READ(intel_hdmi-sdvox_reg);
+ /* HW workaround for IBX, we need to move the port to transcoder A
+ * before disabling it. */
+ if (HAS_PCH_IBX(dev)) {
+ struct drm_crtc *crtc = encoder-crtc;
+ struct intel_crtc *intel_crtc;
+
+ if (crtc)
+ intel_crtc = to_intel_crtc(crtc);
+ else
+ intel_crtc = NULL;
If you extracted pipe = crtc ? to_intel_crtc(crtc)-pipe : -1; then that
would make the following code less ugly.
+
+ if (mode != DRM_MODE_DPMS_ON (temp SDVO_PIPE_B_SELECT)) {
Split this into a nested if:
if (mode != DPMS_MODE_DPMS_ON) {
if (temp SDVO_PIPE_B_SELECT) {
// w/a
}
} else {
// restore
}
+ temp = ~SDVO_PIPE_B_SELECT;
+ I915_WRITE(intel_hdmi-sdvox_reg, temp);
+ POSTING_READ(intel_hdmi-sdvox_reg);
Should we not worry about the HW w/a requiring this to be written twice?
static void intel_hdmi_write_sdvox(intel_hdmi, u32 val)
{
struct drm_i915_private *dev_priv = intel_hdmi-base.dev-dev_private;
I915_WRITE(intel_hdmi-sdvox_reg, val);
POSTING_READ(intel_hdmi-sdvo_reg);
/* HW workaround, need to write this twice for issues that may result
* in first write getting masked.
*/
if (dev_priv-info.has_pch_split) {
I915_WRITE(intel_hdmi-sdvox_reg, val);
POSTING_READ(intel_hdmi-sdvox_reg);
}
}
+
+ if (intel_crtc)
+ intel_wait_for_vblank(dev, intel_crtc-pipe);
+ else
+ msleep(50);
+ } else if (mode == DRM_MODE_DPMS_ON){
+ /* Restore the transcoder select bit. */
+ if (intel_crtc-pipe == PIPE_B)
+ enable_bits |= SDVO_PIPE_B_SELECT;
+ }
+ }
+
/* HW workaround, need to toggle enable bit off and on for 12bpc, but
* we do this anyway which shows more stable in testing.
*/
--
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: don't chnage the original mode in dp_mode_fixup
On Wed, 30 May 2012 12:28:04 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: We should only frob adjusted_mode. This is in preparation of a massive patch by Laurent Pinchart to make the mode argument const. The only thing we actually touch is mode-clock, but only if it's a panel. And in that case we also set adjusted_mode-clock to the same value. All the generic code already uses the adjusted_mode exclusively, so we only have to move the dp link bw calculations over to that. This requires a small changes so that the shared code with mode_valid doesn't touch the mode argument. Separate patch please, I'm sure you are right, but that is the scary one... Also mark the mode argument of pch_panel_fitting const. Reported-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_dp.c| 19 +++ drivers/gpu/drm/i915/intel_drv.h |2 +- drivers/gpu/drm/i915/intel_panel.c |2 +- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 296cfc2..05c4748 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -222,7 +222,7 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes) static bool intel_dp_adjust_dithering(struct intel_dp *intel_dp, struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) + bool adjust_mode) Would this look more pleasant if you rewrote this function so that the adjustment of flags was done in the caller? -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 3/3] drm/i915: assert that the IBX port transcoder select w/a is implemented
On Wed, May 30, 2012 at 11:27:46AM +0100, Chris Wilson wrote: On Wed, 30 May 2012 12:31:58 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: Let's be a bit more paranoid here. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3c71850..a9bc673 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1224,6 +1224,9 @@ static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv, WARN(dp_pipe_enabled(dev_priv, pipe, port_sel, val), PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n, reg, pipe_name(pipe)); + + WARN(HAS_PCH_IBX(dev_priv-dev) (val SDVO_PIPE_B_SELECT), +IBX PCH dp port still using transcoder B\n); Only an issue if both pipe B selected and enabled, right? The problem is actually when it's disabled, but still selects transcoder B. The complicating issue is that we can only change the transcoder while the port is enabled. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: don't chnage the original mode in dp_mode_fixup
On Wed, May 30, 2012 at 11:58:43AM +0100, Chris Wilson wrote: On Wed, 30 May 2012 12:28:04 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: We should only frob adjusted_mode. This is in preparation of a massive patch by Laurent Pinchart to make the mode argument const. The only thing we actually touch is mode-clock, but only if it's a panel. And in that case we also set adjusted_mode-clock to the same value. All the generic code already uses the adjusted_mode exclusively, so we only have to move the dp link bw calculations over to that. This requires a small changes so that the shared code with mode_valid doesn't touch the mode argument. Separate patch please, I'm sure you are right, but that is the scary one... Will do. Also mark the mode argument of pch_panel_fitting const. Reported-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_dp.c| 19 +++ drivers/gpu/drm/i915/intel_drv.h |2 +- drivers/gpu/drm/i915/intel_panel.c |2 +- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 296cfc2..05c4748 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -222,7 +222,7 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes) static bool intel_dp_adjust_dithering(struct intel_dp *intel_dp, struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) + bool adjust_mode) Would this look more pleasant if you rewrote this function so that the adjustment of flags was done in the caller? Well, I've added the adjusted_mode parameter originally to exactly avoid this duplication of code between mode_fixup and mode_valid (which caused a bug). Atm we could just pass back a needs_6bpc_dither flag, but I guess we'll eventually need similar logic for higher bpc (and maybe other funky stuff). So I think setting the flag here is ok and the least ugly version. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: adjusted_mode-clock in the dp mode_fixup
... instead of changing mode-clock, which we should leave as-is.
We only touch that if it's a panel, and then adjusted mode-clock
equals adjusted_mode-clock. Outside of intel_dp.c we only use
ajusted_mode-clock in the mode_set functions.
Within intel_dp.c we only use it to calculate the dp dithering
and link bw parameters, so that's the only thing we need to fix
up.
As a temporary ugliness (until the cleanup in the next patch) we
pass the adjusted_mode into dp_dither for both parameters (because
that one still looks at mode-clock).
Note that we do overwrite adjusted_mode-clock with the selected dp
link clock, but that only happens after we've calculated everything we
need based on the dotclock of the adjusted output configuration.
v2: Adjust the debug message to also use adjusted_mode-clock.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 296cfc2..a9dffa6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -698,11 +698,6 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct
drm_display_mode *mode,
intel_fixed_panel_mode(intel_dp-panel_fixed_mode,
adjusted_mode);
intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
mode, adjusted_mode);
- /*
-* the mode-clock is used to calculate the DataLink M/N
-* of the pipe. For the eDP the fixed clock should be used.
-*/
- mode-clock = intel_dp-panel_fixed_mode-clock;
}
if (mode-flags DRM_MODE_FLAG_DBLCLK)
@@ -710,13 +705,13 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct
drm_display_mode *mode,
DRM_DEBUG_KMS(DP link computation with max lane count %i
max bw %02x pixel clock %iKHz\n,
- max_lane_count, bws[max_clock], mode-clock);
+ max_lane_count, bws[max_clock], adjusted_mode-clock);
- if (!intel_dp_adjust_dithering(intel_dp, mode, adjusted_mode))
+ if (!intel_dp_adjust_dithering(intel_dp, adjusted_mode, adjusted_mode))
return false;
bpp = adjusted_mode-private_flags INTEL_MODE_DP_FORCE_6BPC ? 18 : 24;
- mode_rate = intel_dp_link_required(mode-clock, bpp);
+ mode_rate = intel_dp_link_required(adjusted_mode-clock, bpp);
for (lane_count = 1; lane_count = max_lane_count; lane_count = 1) {
for (clock = 0; clock = max_clock; clock++) {
--
1.7.10
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: don't chnage the original mode in dp_mode_fixup
We should only frob adjusted_mode. This is in preparation of
a massive patch by Laurent Pinchart to make the mode argument
const.
After the prevous prep patch to use adjusted_mode-clock instead of
mode-clock the only thing left is to clean up things a bit. I've
opted to pass in an adjust_mode param to dp_adjust_dithering because
that way we can be sure to avoid duplicating this logic - which was
the cause behind a dp link bw calculation bug in the past.
Also mark the mode argument of pch_panel_fitting const.
v2: Split up the mode-clock = adjusted_mode-clock change,
as suggested by Chris Wilson.
Reported-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c| 12 ++--
drivers/gpu/drm/i915/intel_drv.h |2 +-
drivers/gpu/drm/i915/intel_panel.c |2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a9dffa6..c5c5669 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -222,7 +222,7 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
static bool
intel_dp_adjust_dithering(struct intel_dp *intel_dp,
struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+ bool adjust_mode)
{
int max_link_clock =
intel_dp_link_clock(intel_dp_max_link_bw(intel_dp));
int max_lanes = intel_dp_max_lane_count(intel_dp);
@@ -236,8 +236,8 @@ intel_dp_adjust_dithering(struct intel_dp *intel_dp,
if (mode_rate max_rate)
return false;
- if (adjusted_mode)
- adjusted_mode-private_flags
+ if (adjust_mode)
+ mode-private_flags
|= INTEL_MODE_DP_FORCE_6BPC;
return true;
@@ -260,7 +260,7 @@ intel_dp_mode_valid(struct drm_connector *connector,
return MODE_PANEL;
}
- if (!intel_dp_adjust_dithering(intel_dp, mode, NULL))
+ if (!intel_dp_adjust_dithering(intel_dp, mode, false))
return MODE_CLOCK_HIGH;
if (mode-clock 1)
@@ -700,14 +700,14 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct
drm_display_mode *mode,
mode, adjusted_mode);
}
- if (mode-flags DRM_MODE_FLAG_DBLCLK)
+ if (adjusted_mode-flags DRM_MODE_FLAG_DBLCLK)
return false;
DRM_DEBUG_KMS(DP link computation with max lane count %i
max bw %02x pixel clock %iKHz\n,
max_lane_count, bws[max_clock], adjusted_mode-clock);
- if (!intel_dp_adjust_dithering(intel_dp, adjusted_mode, adjusted_mode))
+ if (!intel_dp_adjust_dithering(intel_dp, adjusted_mode, true))
return false;
bpp = adjusted_mode-private_flags INTEL_MODE_DP_FORCE_6BPC ? 18 : 24;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3e09188..3e3b60c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -372,7 +372,7 @@ extern void intel_fixed_panel_mode(struct drm_display_mode
*fixed_mode,
struct drm_display_mode *adjusted_mode);
extern void intel_pch_panel_fitting(struct drm_device *dev,
int fitting_mode,
- struct drm_display_mode *mode,
+ const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode);
extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
extern u32 intel_panel_get_backlight(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 2a1625d..7180cc8 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -56,7 +56,7 @@ intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
void
intel_pch_panel_fitting(struct drm_device *dev,
int fitting_mode,
- struct drm_display_mode *mode,
+ const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
{
struct drm_i915_private *dev_priv = dev-dev_private;
--
1.7.10
___
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: adjusted_mode-clock in the dp mode_fixup
On Wed, May 30, 2012 at 01:18:35PM +0100, Chris Wilson wrote:
On Wed, 30 May 2012 13:52:02 +0200, Daniel Vetter daniel.vet...@ffwll.ch
wrote:
... instead of changing mode-clock, which we should leave as-is.
We only touch that if it's a panel, and then adjusted mode-clock
equals adjusted_mode-clock. Outside of intel_dp.c we only use
ajusted_mode-clock in the mode_set functions.
Within intel_dp.c we only use it to calculate the dp dithering
and link bw parameters, so that's the only thing we need to fix
up.
As a temporary ugliness (until the cleanup in the next patch) we
pass the adjusted_mode into dp_dither for both parameters (because
that one still looks at mode-clock).
Note that we do overwrite adjusted_mode-clock with the selected dp
link clock, but that only happens after we've calculated everything we
need based on the dotclock of the adjusted output configuration.
v2: Adjust the debug message to also use adjusted_mode-clock.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 296cfc2..a9dffa6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -698,11 +698,6 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
struct drm_display_mode *mode,
intel_fixed_panel_mode(intel_dp-panel_fixed_mode,
adjusted_mode);
intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
mode, adjusted_mode);
- /*
-* the mode-clock is used to calculate the DataLink M/N
-* of the pipe. For the eDP the fixed clock should be used.
-*/
- mode-clock = intel_dp-panel_fixed_mode-clock;
But in ironlake_crtc_mode_set() we have:
if (is_cpu_edp) {
target_clock = mode-clock;
} else {
if (is_dp)
target_clock = mode-clock;
else
target_clock = adjusted_mode-clock;
}
It would seem like eDP would have had mode-clock adjusted previously.
Similarly PCH eDP uses the adjusted mode-clock in intel_dp_set_m_n().
Well, adjusted_mode-clock after we've run intel_dp_mode_fixup should be
the same in all cases, because we overwrite it with the fixed dp link
clock we're selecting. The target_clock otoh looks more worrisome. I guess
I've managed to not hit this because I don't have an eDP panel (where we
change mode-clock), but still I guess we need to clean this up a bit.
I'll try to come up with a way to avoid this maze.
-Daniel
--
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: reuse the sdvo tv clock adjustment in ilk mode_set
Jesse extracted this nice helper in his i9xx_crtc_mode_set refactor,
but we have the identical code in ironlake_ccrtc_mode_set. And that
function is huge, so extracting some code full of magic numbers is
always nice.
Noticed while trying to get a handle on our dp clock code.
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 23 ---
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 9147894..e483148 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4405,25 +4405,10 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
clock,
reduced_clock);
}
- /* SDVO TV has fixed PLL values depend on its clock range,
- this mirrors vbios setting. */
- if (is_sdvo is_tv) {
- if (adjusted_mode-clock = 10
-adjusted_mode-clock 140500) {
- clock.p1 = 2;
- clock.p2 = 10;
- clock.n = 3;
- clock.m1 = 16;
- clock.m2 = 8;
- } else if (adjusted_mode-clock = 140500
- adjusted_mode-clock = 20) {
- clock.p1 = 1;
- clock.p2 = 10;
- clock.n = 6;
- clock.m1 = 12;
- clock.m2 = 8;
- }
- }
+
+ if (is_sdvo is_tv)
+ i9xx_adjust_sdvo_tv_clock(adjusted_mode, clock);
+
/* FDI link */
pixel_multiplier = intel_mode_get_pixel_multiplier(adjusted_mode);
--
1.7.10
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: reuse the sdvo tv clock adjustment in ilk mode_set
On Wed, 30 May 2012 14:52:26 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: Jesse extracted this nice helper in his i9xx_crtc_mode_set refactor, but we have the identical code in ironlake_ccrtc_mode_set. And that function is huge, so extracting some code full of magic numbers is always nice. Noticed while trying to get a handle on our dp clock code. Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -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] drm/i915: compute the target_clock for edp directly
Instead of abusing into mode-clock, because we should touch that
one at all. First prep step to constify the mode argument to the
intel_dp_mode_fixup function.
The next patch will stop us from modifying mode-clock.
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 16
drivers/gpu/drm/i915/intel_dp.c | 12
drivers/gpu/drm/i915/intel_drv.h |2 ++
3 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 9147894..a5d06ed 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4431,16 +4431,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
/* CPU eDP doesn't require FDI link, so just set DP M/N
according to current link config */
if (is_cpu_edp) {
- target_clock = mode-clock;
intel_edp_link_config(edp_encoder, lane, link_bw);
} else {
- /* [e]DP over FDI requires target mode clock
- instead of link clock */
- if (is_dp)
- target_clock = mode-clock;
- else
- target_clock = adjusted_mode-clock;
-
/* FDI is a binary signal running at ~2.7GHz, encoding
* each output octet as 10 bits. The actual frequency
* is stored as a divider into a 100MHz clock, and the
@@ -4451,6 +4443,14 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
}
+ /* [e]DP over FDI requires target mode clock instead of link clock. */
+ if (edp_encoder)
+ target_clock = intel_edp_target_clock(edp_encoder, mode);
+ else if (is_dp)
+ target_clock = mode-clock;
+ else
+ target_clock = adjusted_mode-clock;
+
/* determine panel color depth */
temp = I915_READ(PIPECONF(pipe));
temp = ~PIPE_BPC_MASK;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 296cfc2..4190ed6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -152,6 +152,18 @@ intel_edp_link_config(struct intel_encoder *intel_encoder,
*link_bw = 27;
}
+int
+intel_edp_target_clock(struct intel_encoder *intel_encoder,
+ struct drm_display_mode *mode)
+{
+ struct intel_dp *intel_dp = container_of(intel_encoder, struct
intel_dp, base);
+
+ if (intel_dp-panel_fixed_mode)
+ return intel_dp-panel_fixed_mode-clock;
+ else
+ return mode-clock;
+}
+
static int
intel_dp_max_lane_count(struct intel_dp *intel_dp)
{
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3e09188..706c21c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -360,6 +360,8 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct
drm_display_mode *mode,
struct drm_display_mode *adjusted_mode);
extern bool intel_dpd_is_edp(struct drm_device *dev);
extern void intel_edp_link_config(struct intel_encoder *, int *, int *);
+extern int intel_edp_target_clock(struct intel_encoder *,
+ struct drm_display_mode *mode);
extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
extern int intel_plane_init(struct drm_device *dev, enum pipe pipe);
extern void intel_flush_display_plane(struct drm_i915_private *dev_priv,
--
1.7.10
___
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: don't chnage the original mode in dp_mode_fixup
Am Mittwoch, den 30.05.2012, 13:52 +0200 schrieb Daniel Vetter: Typo in »change« in the commit message. We should only frob adjusted_mode. This is in preparation of a massive patch by Laurent Pinchart to make the mode argument const. After the prevous prep patch to use adjusted_mode-clock instead of previous mode-clock the only thing left is to clean up things a bit. I've opted to pass in an adjust_mode param to dp_adjust_dithering because that way we can be sure to avoid duplicating this logic - which was the cause behind a dp link bw calculation bug in the past. Also mark the mode argument of pch_panel_fitting const. v2: Split up the mode-clock = adjusted_mode-clock change, as suggested by Chris Wilson. Reported-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_dp.c| 12 ++-- drivers/gpu/drm/i915/intel_drv.h |2 +- drivers/gpu/drm/i915/intel_panel.c |2 +- 3 files changed, 8 insertions(+), 8 deletions(-) […] Thanks, Paul 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] drm/i915: there's no cxsr on ilk
On 05/30/2012 12:15 PM, Daniel Vetter wrote: Already discovered in commit 5a117db77e47e3946d1aaa7ce8deafafd9d76746 Author: Eugeni Dodonoveugeni.dodo...@intel.com Date: Thu Jan 5 09:34:29 2012 -0200 drm/i915: there is no pipe CxSR on ironlake but we've failed to rip out the code from the ironlake specific code. Cc: Eugeni Dodonoveugeni.dodo...@intel.com Signed-Off-by: Daniel Vetterdaniel.vet...@ffwll.ch Reviewed-by: Eugeni Dodonov eugeni.dodo...@intel.com The code newer got called anyway since the last patch, so let's put it to rest. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: extract object unplug code from busy/wait_timeout ioctl
Both need to do the same dance (or at least should). Some slight
changes:
- busy_ioctl now unconditionally checks for olr. Before emitting a
require flush would have prevent the olr check and hence required a
second call to the busy ioctl to really emit the request.
- the timeout wait now also retires request. Not really required for
abi-reasons, but makes a notch more sense imo.
I've tested this by pimping the i-g-t test some more and also checking
the polling behviour of the wait_rendering_timeout ioctl versus what
busy_ioctl returns.
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem.c | 61 ++
1 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d2eaa00..521e294 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2000,6 +2000,31 @@ i915_gem_object_wait_rendering(struct
drm_i915_gem_object *obj)
}
/**
+ * Ensures that an object will eventually get non-busy by flushing any required
+ * write domains, emitting any outstanding lazy request and retiring and
+ * completed requests. The unplug moniker is stolen from the linux block layer.
+ */
+static int
+i915_gem_unplug_object(struct drm_i915_gem_object *obj)
+{
+ int ret;
+
+ if (obj-active) {
+ ret = i915_gem_object_flush_gpu_write_domain(obj);
+ if (ret)
+ return ret;
+
+ ret = i915_gem_check_olr(obj-ring,
+obj-last_rendering_seqno);
+ if (ret)
+ return ret;
+ i915_gem_retire_requests_ring(obj-ring);
+ }
+
+ return 0;
+}
+
+/**
* i915_gem_wait_ioctl - implements DRM_IOCTL_I915_GEM_WAIT
* @DRM_IOCTL_ARGS: standard ioctl arguments
*
@@ -2043,11 +2068,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
return -ENOENT;
}
- /* Need to make sure the object is flushed first. This non-obvious
-* flush is required to enforce that (active !olr) == no wait
-* necessary.
-*/
- ret = i915_gem_object_flush_gpu_write_domain(obj);
+ /* Need to make sure the object gets un-active eventually. */
+ ret = i915_gem_unplug_object(obj);
if (ret)
goto out;
@@ -2059,10 +2081,6 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
if (seqno == 0)
goto out;
- ret = i915_gem_check_olr(ring, seqno);
- if (ret)
- goto out;
-
/* Do this after OLR check to make sure we make forward progress polling
* on this IOCTL with a 0 timeout (like busy ioctl)
*/
@@ -3302,30 +3320,9 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
* become non-busy without any further actions, therefore emit any
* necessary flushes here.
*/
- args-busy = obj-active;
- if (args-busy) {
- /* Unconditionally flush objects, even when the gpu still uses
this
-* object. Userspace calling this function indicates that it
wants to
-* use this buffer rather sooner than later, so issuing the
required
-* flush earlier is beneficial.
-*/
- if (obj-base.write_domain I915_GEM_GPU_DOMAINS) {
- ret = i915_gem_flush_ring(obj-ring,
- 0, obj-base.write_domain);
- } else {
- ret = i915_gem_check_olr(obj-ring,
-obj-last_rendering_seqno);
- }
+ ret = i915_gem_unplug_object(obj);
- /* Update the active list for the hardware's current position.
-* Otherwise this only updates on a delayed timer or when irqs
-* are actually unmasked, and our working set ends up being
-* larger than required.
-*/
- i915_gem_retire_requests_ring(obj-ring);
-
- args-busy = obj-active;
- }
+ args-busy = obj-active;
drm_gem_object_unreference(obj-base);
unlock:
--
1.7.7.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: extract object unplug code from busy/wait_timeout ioctl
On Wed, 30 May 2012 20:21:33 +0200
Daniel Vetter daniel.vet...@ffwll.ch wrote:
Both need to do the same dance (or at least should). Some slight
changes:
- busy_ioctl now unconditionally checks for olr. Before emitting a
require flush would have prevent the olr check and hence required a
second call to the busy ioctl to really emit the request.
- the timeout wait now also retires request. Not really required for
abi-reasons, but makes a notch more sense imo.
The one thing I dislike about the retiring behavior is it is still not
guaranteed. After some coffee, I've remembered a bit about this. Since
we do an non-blocking wait, it's possible for userspace to race against
itself and rebusy the object. Therefore, the client should never assume
the call to wait_ioctl is the same as busy. (Busy has a similar, shorter
race; but also has a little smarter semantics). Therefore, I don't think
this buys us much, but I also don't think it hurts. So I'd vote on
letting Eric decide if he wants this or not.
I've tested this by pimping the i-g-t test some more and also checking
the polling behviour of the wait_rendering_timeout ioctl versus what
busy_ioctl returns.
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem.c | 61 ++
1 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d2eaa00..521e294 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2000,6 +2000,31 @@ i915_gem_object_wait_rendering(struct
drm_i915_gem_object *obj)
}
/**
+ * Ensures that an object will eventually get non-busy by flushing any
required
+ * write domains, emitting any outstanding lazy request and retiring and
+ * completed requests. The unplug moniker is stolen from the linux block
layer.
+ */
I'd prefer something like, unbusy but whatever.
+static int
+i915_gem_unplug_object(struct drm_i915_gem_object *obj)
+{
+ int ret;
+
+ if (obj-active) {
+ ret = i915_gem_object_flush_gpu_write_domain(obj);
+ if (ret)
+ return ret;
+
+ ret = i915_gem_check_olr(obj-ring,
+ obj-last_rendering_seqno);
+ if (ret)
+ return ret;
+ i915_gem_retire_requests_ring(obj-ring);
+ }
+
+ return 0;
+}
+
+/**
* i915_gem_wait_ioctl - implements DRM_IOCTL_I915_GEM_WAIT
* @DRM_IOCTL_ARGS: standard ioctl arguments
*
@@ -2043,11 +2068,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void
*data, struct drm_file *file)
return -ENOENT;
}
- /* Need to make sure the object is flushed first. This non-obvious
- * flush is required to enforce that (active !olr) == no wait
- * necessary.
- */
- ret = i915_gem_object_flush_gpu_write_domain(obj);
+ /* Need to make sure the object gets un-active eventually. */
+ ret = i915_gem_unplug_object(obj);
if (ret)
goto out;
I can't remember the reason offhand, but I vaguely recall Chris demanded
we flush the write domain even if the object isn't active. Perhaps you
can look up the email, or he can chime in. I forget the exact reason.
@@ -2059,10 +2081,6 @@ i915_gem_wait_ioctl(struct drm_device *dev, void
*data, struct drm_file *file)
if (seqno == 0)
goto out;
- ret = i915_gem_check_olr(ring, seqno);
- if (ret)
- goto out;
-
/* Do this after OLR check to make sure we make forward progress polling
* on this IOCTL with a 0 timeout (like busy ioctl)
*/
@@ -3302,30 +3320,9 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
* become non-busy without any further actions, therefore emit any
* necessary flushes here.
*/
- args-busy = obj-active;
- if (args-busy) {
- /* Unconditionally flush objects, even when the gpu still uses
this
- * object. Userspace calling this function indicates that it
wants to
- * use this buffer rather sooner than later, so issuing the
required
- * flush earlier is beneficial.
- */
- if (obj-base.write_domain I915_GEM_GPU_DOMAINS) {
- ret = i915_gem_flush_ring(obj-ring,
- 0, obj-base.write_domain);
- } else {
- ret = i915_gem_check_olr(obj-ring,
- obj-last_rendering_seqno);
- }
+ ret = i915_gem_unplug_object(obj);
- /* Update the active list for the hardware's current position.
- * Otherwise this only updates on a delayed timer or when irqs
- * are actually unmasked, and our working set ends up being
-
Re: [Intel-gfx] [PATCH 13/14] drm/i915: add some barriers when changing DIPs
On Mon, May 28, 2012 at 04:43:00PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com On IVB and older, we basically have two registers: the control and the data register. We write a few consecutitve times to the control register, and we need these writes to arrive exactly in the specified order. Also, when we're changing the data register, we need to guarantee that anything written to the control register already arrived (since changing the control register can change where the data register points to). Also, we need to make sure all the writes to the data register happen exactly in the specified order, and we also *can't* read the data register during this process, since reading and/or writing it will change the place it points to. So invoke the better safe than sorry rule and just be careful and put barriers everywhere :) On HSW we still have a control register that we write many times, but we have many data registers. Demanded-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Ok, I've all patches from this series for -next safe for the patches 10, 12 and 14: - for 14 I've already dumped a bikeshed - 10 and 12 I like, but I fear we'll get too many merge conflicts against -fixes if I merge them this early. I'd still like to include them for 3.6, so can you please resend these two later in the 3.5 cycle, when things have settled a bit for -fixes? Thanks a lot for digging into this infoframe maze. Cheers, Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: extract object unplug code from busy/wait_timeout ioctl
On Wed, May 30, 2012 at 01:41:28PM -0700, Ben Widawsky wrote:
On Wed, 30 May 2012 20:21:33 +0200
Daniel Vetter daniel.vet...@ffwll.ch wrote:
Both need to do the same dance (or at least should). Some slight
changes:
- busy_ioctl now unconditionally checks for olr. Before emitting a
require flush would have prevent the olr check and hence required a
second call to the busy ioctl to really emit the request.
- the timeout wait now also retires request. Not really required for
abi-reasons, but makes a notch more sense imo.
The one thing I dislike about the retiring behavior is it is still not
guaranteed. After some coffee, I've remembered a bit about this. Since
we do an non-blocking wait, it's possible for userspace to race against
itself and rebusy the object. Therefore, the client should never assume
the call to wait_ioctl is the same as busy. (Busy has a similar, shorter
race; but also has a little smarter semantics). Therefore, I don't think
this buys us much, but I also don't think it hurts. So I'd vote on
letting Eric decide if he wants this or not.
Well, as I've said in the commit message, we don't need the retire_request
in the wait_ioctl for abi reasons - it doesn't change anything.
But if you use the wait_ioctl as a poll (with timeout=0) we will never
transition the object out of the active state (well, as long as the retire
work doesn't run at least), so we'll always go to through the __wait_seqno
call to notice that the seqno passed already. Purely academic optimization
because such a usage-pattern would be nuts, but it nicely unifies how we
handle things in the busy ioctl (which I like as a cleanup).
I've tested this by pimping the i-g-t test some more and also checking
the polling behviour of the wait_rendering_timeout ioctl versus what
busy_ioctl returns.
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem.c | 61
++
1 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index d2eaa00..521e294 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2000,6 +2000,31 @@ i915_gem_object_wait_rendering(struct
drm_i915_gem_object *obj)
}
/**
+ * Ensures that an object will eventually get non-busy by flushing any
required
+ * write domains, emitting any outstanding lazy request and retiring and
+ * completed requests. The unplug moniker is stolen from the linux block
layer.
+ */
I'd prefer something like, unbusy but whatever.
Considered and I've thought that's not a proper word. And unbusy isn't
quite correct either, because this only ensures that the object will get
unbusy eventually, if you keep on calling this function (due to the
retire_request in there). And _eventually_unbusy_object sounds horrible to
me. I admit that unplug is a misdenomer, too, but I lack good ideas.
+static int
+i915_gem_unplug_object(struct drm_i915_gem_object *obj)
+{
+ int ret;
+
+ if (obj-active) {
+ ret = i915_gem_object_flush_gpu_write_domain(obj);
+ if (ret)
+ return ret;
+
+ ret = i915_gem_check_olr(obj-ring,
+obj-last_rendering_seqno);
+ if (ret)
+ return ret;
+ i915_gem_retire_requests_ring(obj-ring);
+ }
+
+ return 0;
+}
+
+/**
* i915_gem_wait_ioctl - implements DRM_IOCTL_I915_GEM_WAIT
* @DRM_IOCTL_ARGS: standard ioctl arguments
*
@@ -2043,11 +2068,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void
*data, struct drm_file *file)
return -ENOENT;
}
- /* Need to make sure the object is flushed first. This non-obvious
-* flush is required to enforce that (active !olr) == no wait
-* necessary.
-*/
- ret = i915_gem_object_flush_gpu_write_domain(obj);
+ /* Need to make sure the object gets un-active eventually. */
+ ret = i915_gem_unplug_object(obj);
if (ret)
goto out;
I can't remember the reason offhand, but I vaguely recall Chris demanded
we flush the write domain even if the object isn't active. Perhaps you
can look up the email, or he can chime in. I forget the exact reason.
If an object has non-zero write_domain, we keep it on the flushing_list
and also keep the object active. If that's not the case somewhere, we have
a bug. So the flush is required to eventually unbusy the object.
@@ -2059,10 +2081,6 @@ i915_gem_wait_ioctl(struct drm_device *dev, void
*data, struct drm_file *file)
if (seqno == 0)
goto out;
- ret = i915_gem_check_olr(ring, seqno);
- if (ret)
- goto out;
-
/* Do this after OLR check to make sure we make forward progress polling
* on this IOCTL with a 0 timeout (like busy ioctl)
Re: [Intel-gfx] Lots of i915/drm spew on 3.4
On Wed, May 30, 2012 at 11:31 PM, Dave Jones da...@redhat.com wrote: On this hardware: 00:02.0 VGA compatible controller: Intel Corporation 82945G/GZ Integrated Graphics Controller (rev 02) I get this every boot with Linus current tree (up to af56e0aa35f3ae2a4c1a6d1000702df1dd78cb76) Just a quick question, is this a regression? If so, can you please attach the output of xrandr --verbose from a noisy and a quite kernel (otherwise just please attach it from this noisy kernel). Thanks, Daniel -- Daniel Vetter daniel.vet...@ffwll.ch - +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
