Re: [Intel-gfx] [PATCH 06/13] drm/i915: add pipe_config-has_pch_encoder

2013-03-27 Thread Jesse Barnes 
On Wed, 27 Mar 2013 00:44:55 +0100
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 This is used way too often in the enable/disable paths. And will
 be even more useful in the future.
 
 Note that correct semantics of this change highly depend upon
 correct updating of intel_crtc-config: Like with all other
 modeset state, we need to call -disable with the old config,
 but -mode_set and -enable with the new config.
 
 v2: Do not yet use the flag in the -disable callbacks - atm we don't
 yet have support for the information stored in the pipe_config in the
 hw state readout code, so this will be wrong at boot-up/resume.
 
 v3: Rebased on top of the hdmi/dp ddi encoder merging.
 
 v4: Fixup stupid rebase error which lead to a NULL vfunc deref.
 
 v5: On haswell the VGA port is on the PCH!
 
 v6: s/IS_HASWELL/HAS_DDI/, spotted by Paulo Zanoni. Also add a missing
 parameter name in a function declaration.
 
 v7: Don't forget to git add ...

Looks like you got all the outputs covered.  But we always seem to get
this bit wrong, so we'll need to test on all the configs we know about
at least...

+   if (HAS_PCH_SPLIT(dev)  !HAS_DDI(dev)  !is_cpu_edp(intel_dp))
+   pipe_config-has_pch_encoder = true;
+

This could just do
 if (intel_dp-is_pch_edp)
pipe_config-has_pch_encoder = true;
right?  Since we cover the other cases in dp_init_connector?

But either way:

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/13] drm/i915: add pipe_config-has_pch_encoder

2013-03-27 Thread Daniel Vetter 
On Wed, Mar 27, 2013 at 6:06 PM, Jesse Barnes jbar...@virtuousgeek.org wrote:
 +   if (HAS_PCH_SPLIT(dev)  !HAS_DDI(dev)  !is_cpu_edp(intel_dp))
 +   pipe_config-has_pch_encoder = true;
 +

 This could just do
  if (intel_dp-is_pch_edp)
 pipe_config-has_pch_encoder = true;
 right?  Since we cover the other cases in dp_init_connector?

That would give you two wrong case currently:
- hsw port D eDP would be marked as pch port
- any pch non-eDP DP ports would not be marked as pch ports

The ugly thing with this patch here is that this property is actually
fixed to the encoder, but I dynamically compute it in compute_config.
We have a few other such cases (e.g. the cpu transcoder for edp on
hsw). But I've figured there's no point in adding something clever,
which then updates the pipe_config according to connected encoders
with data structures ...
-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 06/13] drm/i915: add pipe_config-has_pch_encoder

2013-03-26 Thread Daniel Vetter 
This is used way too often in the enable/disable paths. And will
be even more useful in the future.

Note that correct semantics of this change highly depend upon
correct updating of intel_crtc-config: Like with all other
modeset state, we need to call -disable with the old config,
but -mode_set and -enable with the new config.

v2: Do not yet use the flag in the -disable callbacks - atm we don't
yet have support for the information stored in the pipe_config in the
hw state readout code, so this will be wrong at boot-up/resume.

v3: Rebased on top of the hdmi/dp ddi encoder merging.

v4: Fixup stupid rebase error which lead to a NULL vfunc deref.

v5: On haswell the VGA port is on the PCH!

v6: s/IS_HASWELL/HAS_DDI/, spotted by Paulo Zanoni. Also add a missing
parameter name in a function declaration.

v7: Don't forget to git add ...

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/intel_crt.c | 12 +++
 drivers/gpu/drm/i915/intel_ddi.c | 16 +++
 drivers/gpu/drm/i915/intel_display.c | 40 
 drivers/gpu/drm/i915/intel_dp.c  | 16 +--
 drivers/gpu/drm/i915/intel_drv.h | 13 ++--
 drivers/gpu/drm/i915/intel_hdmi.c| 14 -
 drivers/gpu/drm/i915/intel_lvds.c|  2 ++
 drivers/gpu/drm/i915/intel_sdvo.c|  3 +++
 8 files changed, 54 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 0e9e9e5..1d8d63a 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -199,10 +199,14 @@ static int intel_crt_mode_valid(struct drm_connector 
*connector,
return MODE_OK;
 }
 
-static bool intel_crt_mode_fixup(struct drm_encoder *encoder,
-const struct drm_display_mode *mode,
-struct drm_display_mode *adjusted_mode)
+static bool intel_crt_compute_config(struct intel_encoder *encoder,
+struct intel_crtc_config *pipe_config)
 {
+   struct drm_device *dev = encoder-base.dev;
+
+   if (HAS_PCH_SPLIT(dev))
+   pipe_config-has_pch_encoder = true;
+
return true;
 }
 
@@ -676,7 +680,6 @@ static void intel_crt_reset(struct drm_connector *connector)
  */
 
 static const struct drm_encoder_helper_funcs crt_encoder_funcs = {
-   .mode_fixup = intel_crt_mode_fixup,
.mode_set = intel_crt_mode_set,
 };
 
@@ -768,6 +771,7 @@ void intel_crt_init(struct drm_device *dev)
else
crt-adpa_reg = ADPA;
 
+   crt-base.compute_config = intel_crt_compute_config;
crt-base.disable = intel_disable_crt;
crt-base.enable = intel_enable_crt;
if (I915_HAS_HOTPLUG(dev))
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 258e38e..baeb470 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1476,19 +1476,17 @@ static void intel_ddi_destroy(struct drm_encoder 
*encoder)
intel_dp_encoder_destroy(encoder);
 }
 
-static bool intel_ddi_mode_fixup(struct drm_encoder *encoder,
-const struct drm_display_mode *mode,
-struct drm_display_mode *adjusted_mode)
+static bool intel_ddi_compute_config(struct intel_encoder *encoder,
+struct intel_crtc_config *pipe_config)
 {
-   struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
-   int type = intel_encoder-type;
+   int type = encoder-type;
 
-   WARN(type == INTEL_OUTPUT_UNKNOWN, mode_fixup() on unknown output!\n);
+   WARN(type == INTEL_OUTPUT_UNKNOWN, compute_config() on unknown 
output!\n);
 
if (type == INTEL_OUTPUT_HDMI)
-   return intel_hdmi_mode_fixup(encoder, mode, adjusted_mode);
+   return intel_hdmi_compute_config(encoder, pipe_config);
else
-   return intel_dp_mode_fixup(encoder, mode, adjusted_mode);
+   return intel_dp_compute_config(encoder, pipe_config);
 }
 
 static const struct drm_encoder_funcs intel_ddi_funcs = {
@@ -1496,7 +1494,6 @@ static const struct drm_encoder_funcs intel_ddi_funcs = {
 };
 
 static const struct drm_encoder_helper_funcs intel_ddi_helper_funcs = {
-   .mode_fixup = intel_ddi_mode_fixup,
.mode_set = intel_ddi_mode_set,
 };
 
@@ -1536,6 +1533,7 @@ void intel_ddi_init(struct drm_device *dev, enum port 
port)
 DRM_MODE_ENCODER_TMDS);
drm_encoder_helper_add(encoder, intel_ddi_helper_funcs);
 
+   intel_encoder-compute_config = intel_ddi_compute_config;
intel_encoder-enable = intel_enable_ddi;
intel_encoder-pre_enable = intel_ddi_pre_enable;
intel_encoder-disable = intel_disable_ddi;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3672b8d..fda0754 100644
---