Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot
2014-07-07 17:01 GMT-03:00 clinton.a.tay...@intel.com:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Ver5: Move registration of reboot notifier to edp_connector_init,
Added warning comment to handler about lack of PM notification.
Not everything I pointed was implemented, but at least the ghost eDP
bug was fixed, so the current patch is already an improvement:
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 42
++
drivers/gpu/drm/i915/intel_drv.h |2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec489..7204dee 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -336,6 +338,37 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing
+ This function only applicable when panel PM state is not to be tracked */
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART)
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(pp_div_reg);
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
+
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3818,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -4262,6 +4299,11 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
}
mutex_unlock(dev-mode_config.mutex);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call = edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
+
intel_panel_init(intel_connector-panel, fixed_mode, downclock_mode);
intel_panel_setup_backlight(connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd..87d1715 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -541,6 +541,8 @@ struct intel_dp {
unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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/vlv: T12 eDP panel timing enforcement during reboot
On Tue, Jul 08, 2014 at 11:06:00AM -0300, Paulo Zanoni wrote:
2014-07-07 17:01 GMT-03:00 clinton.a.tay...@intel.com:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Ver5: Move registration of reboot notifier to edp_connector_init,
Added warning comment to handler about lack of PM notification.
Not everything I pointed was implemented, but at least the ghost eDP
bug was fixed, so the current patch is already an improvement:
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
Picked up for -fixes, thanks for the patch.
-Daniel
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 42
++
drivers/gpu/drm/i915/intel_drv.h |2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index b5ec489..7204dee 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -336,6 +338,37 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return
VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing
+ This function only applicable when panel PM state is not to be tracked
*/
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART)
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(pp_div_reg);
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS |
PANEL_POWER_OFF);
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
+
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3818,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -4262,6 +4299,11 @@ static bool intel_edp_init_connector(struct intel_dp
*intel_dp,
}
mutex_unlock(dev-mode_config.mutex);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call = edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
+
intel_panel_init(intel_connector-panel, fixed_mode,
downclock_mode);
intel_panel_setup_backlight(connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd..87d1715 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -541,6 +541,8 @@ struct intel_dp {
unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot
On Thu, Jul 03, 2014 at 11:44:34PM +0300, Jani Nikula wrote:
On Thu, 03 Jul 2014, Clint Taylor clinton.a.tay...@intel.com wrote:
On 07/02/2014 01:35 AM, Jani Nikula wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
[Jani: rebased on current -nightly.]
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
I ended up doing the rebase myself, but I'd like to have a quick review
before pushing.
Quick review complete. Everything appears OK.
See Paulo's review; want to take over?
Imo this is -fixes material, but it looks like it's not yet complete. I'll
wait for a resend safe when someone yells at me that the current patch is
ok-ish (maybe just needs a pimped commit message or a comment somewhere to
address Paulo's concerns).
-Daniel
Jani.
Thanks,
Jani.
---
drivers/gpu/drm/i915/intel_dp.c | 40
drivers/gpu/drm/i915/intel_drv.h | 2 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index b5ec48913b47..f0d23c435cf6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -336,6 +338,36 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12
timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+ edp_notifier);
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART)
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
+
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3817,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -4353,6 +4389,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call =
edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
}
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd94d90..87d1715db21d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -541,6 +541,8 @@ struct intel_dp {
unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
--
Jani Nikula, Intel Open Source Technology Center
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot
2014-07-03 19:07 GMT-03:00 Clint Taylor clinton.a.tay...@intel.com:
On 07/02/2014 07:40 AM, Paulo Zanoni wrote:
2014-07-02 5:35 GMT-03:00 Jani Nikula jani.nik...@intel.com:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
[Jani: rebased on current -nightly.]
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
I ended up doing the rebase myself, but I'd like to have a quick review
before pushing.
Thanks,
Jani.
---
drivers/gpu/drm/i915/intel_dp.c | 40
drivers/gpu/drm/i915/intel_drv.h | 2 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index b5ec48913b47..f0d23c435cf6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -336,6 +338,36 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return
VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12
timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(*
intel_dp),
+edp_notifier);
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART)
What if someone does a power off and _very quickly_ starts the system
again? =P
insert same statement for the other code possibilities
If someone removes and applies power within ~300ms this W/A will break down
and the power sequence will not meet the eDP T12 timing. Since the PPS
sequencer does not allow modifications to the default time intervals during
the initial sequence the only way to guarantee we meet T12 time would be to
delay display block power ungate for 300ms. Further delay of the boot time
was not an acceptable solution for the customers.
My suggestion here was just to not-return in more cases, instead of
only SYS_RESTART.
Also, depending based on the suggestions below, you may not need the
is_edp() check (or you may want to convert it to a WARN).
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
This check is not really needed. It could also be an early return or a
WARN.
Since we currently don't handle PCH offsets this was a safe way to allowing
adding of the PCH functionality later if necessary.
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
Or pp_div = I915_READ(pp_div_reg);, since we just defined it :)
Agreed that's another way to do the same thing.
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS |
PANEL_POWER_OFF);
So this is basically turning the panel off and turning the force VDD
bit off too, and we're not putting any power domain references back.
Even though this might not be a big problem since we're shutting down
the machine anyway, did we attach a serial cable and check if this
won't print any WARNs while shutting down? Shouldn't we try to make
this function call the other functions that shut down stuff instead of
forcing the panel down here?
Development of this W/A was done with serial port attached. This function is
the last method called in the I915 driver before power is removed. At reboot
notifier time there is no error handling possible calling the normal
shutdown functions does more housekeeping then we need for a system that
will not have power in 2 ms.
For this code, even if we don't change it, I think we should at least
put a comment here describing this is an acceptable solution for a
machine shutdown, but that this code should not be reused in other
cases since we're forcing a panel shutdown without respecting
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot
On 07/02/2014 01:35 AM, Jani Nikula wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
[Jani: rebased on current -nightly.]
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
I ended up doing the rebase myself, but I'd like to have a quick review
before pushing.
Quick review complete. Everything appears OK.
Thanks,
Jani.
---
drivers/gpu/drm/i915/intel_dp.c | 40
drivers/gpu/drm/i915/intel_drv.h | 2 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec48913b47..f0d23c435cf6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -336,6 +338,36 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART)
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
+
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3817,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -4353,6 +4389,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call =
edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
}
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd94d90..87d1715db21d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -541,6 +541,8 @@ struct intel_dp {
unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot
On Thu, 03 Jul 2014, Clint Taylor clinton.a.tay...@intel.com wrote:
On 07/02/2014 01:35 AM, Jani Nikula wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
[Jani: rebased on current -nightly.]
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
I ended up doing the rebase myself, but I'd like to have a quick review
before pushing.
Quick review complete. Everything appears OK.
See Paulo's review; want to take over?
Jani.
Thanks,
Jani.
---
drivers/gpu/drm/i915/intel_dp.c | 40
drivers/gpu/drm/i915/intel_drv.h | 2 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index b5ec48913b47..f0d23c435cf6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -336,6 +338,36 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing
*/
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+ edp_notifier);
+struct drm_device *dev = intel_dp_to_dev(intel_dp);
+struct drm_i915_private *dev_priv = dev-dev_private;
+u32 pp_div;
+u32 pp_ctrl_reg, pp_div_reg;
+enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+if (!is_edp(intel_dp) || code != SYS_RESTART)
+return 0;
+
+if (IS_VALLEYVIEW(dev)) {
+pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+/* 0x1F write to PP_DIV_REG sets max cycle delay */
+I915_WRITE(pp_div_reg, pp_div | 0x1F);
+I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+msleep(intel_dp-panel_power_cycle_delay);
+}
+
+return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3817,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
+if (intel_dp-edp_notifier.notifier_call) {
+unregister_reboot_notifier(intel_dp-edp_notifier);
+intel_dp-edp_notifier.notifier_call = NULL;
+}
}
kfree(intel_dig_port);
}
@@ -4353,6 +4389,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+if (IS_VALLEYVIEW(dev)) {
+intel_dp-edp_notifier.notifier_call =
edp_notify_handler;
+register_reboot_notifier(intel_dp-edp_notifier);
+}
}
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd94d90..87d1715db21d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -541,6 +541,8 @@ struct intel_dp {
unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot
On 07/02/2014 07:40 AM, Paulo Zanoni wrote:
2014-07-02 5:35 GMT-03:00 Jani Nikula jani.nik...@intel.com:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
[Jani: rebased on current -nightly.]
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
I ended up doing the rebase myself, but I'd like to have a quick review
before pushing.
Thanks,
Jani.
---
drivers/gpu/drm/i915/intel_dp.c | 40
drivers/gpu/drm/i915/intel_drv.h | 2 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec48913b47..f0d23c435cf6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -336,6 +338,36 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART)
What if someone does a power off and _very quickly_ starts the system again? =P
insert same statement for the other code possibilities
If someone removes and applies power within ~300ms this W/A will break
down and the power sequence will not meet the eDP T12 timing. Since the
PPS sequencer does not allow modifications to the default time intervals
during the initial sequence the only way to guarantee we meet T12 time
would be to delay display block power ungate for 300ms. Further delay of
the boot time was not an acceptable solution for the customers.
Also, depending based on the suggestions below, you may not need the
is_edp() check (or you may want to convert it to a WARN).
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
This check is not really needed. It could also be an early return or a WARN.
Since we currently don't handle PCH offsets this was a safe way to
allowing adding of the PCH functionality later if necessary.
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
Or pp_div = I915_READ(pp_div_reg);, since we just defined it :)
Agreed that's another way to do the same thing.
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
So this is basically turning the panel off and turning the force VDD
bit off too, and we're not putting any power domain references back.
Even though this might not be a big problem since we're shutting down
the machine anyway, did we attach a serial cable and check if this
won't print any WARNs while shutting down? Shouldn't we try to make
this function call the other functions that shut down stuff instead of
forcing the panel down here?
Development of this W/A was done with serial port attached. This
function is the last method called in the I915 driver before power is
removed. At reboot notifier time there is no error handling possible
calling the normal shutdown functions does more housekeeping then we
need for a system that will not have power in 2 ms.
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
+
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3817,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot.
On Tue, 03 Jun 2014, clinton.a.tay...@intel.com wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
Clint, the patch no longer applies cleanly. Please submit a rebased
version on top of current -nightly. Sorry for the extra trouble.
BR,
Jani.
---
drivers/gpu/drm/i915/intel_dp.c | 42
++
drivers/gpu/drm/i915/intel_drv.h |2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5ca68aa9..cede9bc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -302,6 +304,38 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+ edp_notifier);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART )
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg,
+PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
+
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3344,6 +3378,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
mutex_lock(dev-mode_config.mutex);
edp_panel_vdd_off_sync(intel_dp);
mutex_unlock(dev-mode_config.mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -3782,6 +3820,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call =
edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
}
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 328b1a7..6d0d96e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -510,6 +510,8 @@ struct intel_dp {
unsigned long last_power_on;
unsigned long last_backlight_off;
bool psr_setup_done;
+ struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot
2014-07-02 5:35 GMT-03:00 Jani Nikula jani.nik...@intel.com:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
[Jani: rebased on current -nightly.]
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
I ended up doing the rebase myself, but I'd like to have a quick review
before pushing.
Thanks,
Jani.
---
drivers/gpu/drm/i915/intel_dp.c | 40
drivers/gpu/drm/i915/intel_drv.h | 2 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec48913b47..f0d23c435cf6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -336,6 +338,36 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART)
What if someone does a power off and _very quickly_ starts the system again? =P
insert same statement for the other code possibilities
Also, depending based on the suggestions below, you may not need the
is_edp() check (or you may want to convert it to a WARN).
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
This check is not really needed. It could also be an early return or a WARN.
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
Or pp_div = I915_READ(pp_div_reg);, since we just defined it :)
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
So this is basically turning the panel off and turning the force VDD
bit off too, and we're not putting any power domain references back.
Even though this might not be a big problem since we're shutting down
the machine anyway, did we attach a serial cable and check if this
won't print any WARNs while shutting down? Shouldn't we try to make
this function call the other functions that shut down stuff instead of
forcing the panel down here?
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
+
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3817,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
drm_modeset_lock(dev-mode_config.connection_mutex, NULL);
edp_panel_vdd_off_sync(intel_dp);
drm_modeset_unlock(dev-mode_config.connection_mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -4353,6 +4389,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp,
power_seq);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call =
edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
Why not put this inside intel_edp_init_connector? If you keep it here,
you also have to undo the notifier at the point
intel_dp_init_connector returns false (a few lines below). If you move
to the
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot.
On Tue, 03 Jun 2014, clinton.a.tay...@intel.com wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Ver4: Minor issue changes
Reviewed-by: Jani Nikula jani.nik...@intel.com
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 42
++
drivers/gpu/drm/i915/intel_drv.h |2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5ca68aa9..cede9bc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -302,6 +304,38 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+ edp_notifier);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp) || code != SYS_RESTART )
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg, pp_div | 0x1F);
+ I915_WRITE(pp_ctrl_reg,
+PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
+
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3344,6 +3378,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
mutex_lock(dev-mode_config.mutex);
edp_panel_vdd_off_sync(intel_dp);
mutex_unlock(dev-mode_config.mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -3782,6 +3820,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call =
edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
}
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 328b1a7..6d0d96e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -510,6 +510,8 @@ struct intel_dp {
unsigned long last_power_on;
unsigned long last_backlight_off;
bool psr_setup_done;
+ struct notifier_block edp_notifier;
+
bool use_tps3;
struct intel_connector *attached_connector;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot.
On Mon, 02 Jun 2014, clinton.a.tay...@intel.com wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Ver3: moved SYS_RESTART check earlier, new name for pp_div.
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 42
++
drivers/gpu/drm/i915/intel_drv.h |2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5ca68aa9..fb7725a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -302,6 +304,38 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+ edp_notifier);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp_div;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if ((!is_edp(intel_dp))
+ (code != SYS_RESTART ))
Should be:
if (!is_edp(intel_dp) || code != SYS_RESTART)
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ pp_div = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp_div = PP_REFERENCE_DIVIDER_MASK;
+
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg , pp_div | 0x1F);
Superfluous space before comma.
+ I915_WRITE(pp_ctrl_reg,
+PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ msleep(intel_dp-panel_power_cycle_delay);
+ }
A blank line before final return statement is customary.
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3344,6 +3378,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
mutex_lock(dev-mode_config.mutex);
edp_panel_vdd_off_sync(intel_dp);
mutex_unlock(dev-mode_config.mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -3782,6 +3820,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call =
edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
}
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 328b1a7..ea2cc07 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -510,6 +510,8 @@ struct intel_dp {
unsigned long last_power_on;
unsigned long last_backlight_off;
bool psr_setup_done;
+ struct notifier_block edp_notifier;
Use only one space between type and name.
With all of the above fixed it's
Reviewed-by: Jani Nikula jani.nik...@intel.com
+
bool use_tps3;
struct intel_connector *attached_connector;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot.
On Sat, 17 May 2014, clinton.a.tay...@intel.com wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its T12
power down duration during warm reboots. This change forces a delay for warm
reboots to the T12 panel timing as defined in the VBT table for the connected
panel.
Wrap to about 72 cols.
Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 43
++
drivers/gpu/drm/i915/intel_drv.h |1 +
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5ca68aa9..023efda 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -302,6 +304,39 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to gaurantee T12 timing */
guarantee
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+ edp_notifier);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp))
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ if (code == SYS_RESTART) {
Check for code != SYS_RESTART along with !is_edp above and bail out
early.
+ pp = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
Elsewhere pp is used for the control reg. Please use pp_div instead.
You use pp_div_reg inconsistently; the above doesn't have it. (I think
you could drop the temp vars for regs here completely, but up to you.)
+ pp = PP_REFERENCE_DIVIDER_MASK;
+ /* 0x1F write to PP_DIV_REG sets max cycle delay */
+ I915_WRITE(pp_div_reg , pp | 0x1F);
+ I915_WRITE(pp_ctrl_reg,
+PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ /* VBT entries are in tenths of uS convert to mS */
Use s for seconds; S is Siemens. But really, drop the comment
altogether because...
+ msleep(dev_priv-vbt.edp_pps.t11_t12 / 10);
I think you should use msleep(intel_dp-panel_power_cycle_delay)
instead. The units are already taken care of, and it has fallbacks for
when VBT is bogus or zero.
+ }
+ }
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3344,6 +3379,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
mutex_lock(dev-mode_config.mutex);
edp_panel_vdd_off_sync(intel_dp);
mutex_unlock(dev-mode_config.mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -3782,6 +3821,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call =
edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
}
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 328b1a7..1ea193a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -512,6 +512,7 @@ struct intel_dp {
bool psr_setup_done;
bool use_tps3;
struct intel_connector *attached_connector;
+ struct notifier_block edp_notifier;
This belongs after psr_setup_done field, preferrably add a space after
them to separate edp stuff from the rest. (I think we should have a sub
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot.
On Tue, May 13, 2014 at 11:51:11AM -0700, clinton.a.tay...@intel.com wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its T12
power down duration during warm reboots. This change forces a delay for warm
reboots to the T12 panel timing as defined in the VBT table for the connected
panel.
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 43
++
drivers/gpu/drm/i915/intel_drv.h |1 +
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5ca68aa9..03ac64f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -302,6 +304,39 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to gaurantee T12 timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+ edp_notifier);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp))
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ if (code == SYS_RESTART) {
+ pr_crit(eDP Notifier Handler\n);
+ pp = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp = PP_REFERENCE_DIVIDER_MASK;
+ I915_WRITE(pp_div_reg , pp | 0x1F);
+ I915_WRITE(pp_ctrl_reg,
+PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ /* VBT entries are in tenths of uS convert to mS */
+ msleep(dev_priv-vbt.edp_pps.t11_t12 / 10);
Imo just compute the desired delay before setting up this reboot handler
and pass it as the argument.
But that leaves a bit the question why we don't need that everywhere else
and what exactly this does? I'm a bit confused with the commit message.
-Daniel
+ }
+ }
+ return 0;
+}
+
static bool edp_have_panel_power(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3344,6 +3379,10 @@ void intel_dp_encoder_destroy(struct drm_encoder
*encoder)
mutex_lock(dev-mode_config.mutex);
edp_panel_vdd_off_sync(intel_dp);
mutex_unlock(dev-mode_config.mutex);
+ if (intel_dp-edp_notifier.notifier_call) {
+ unregister_reboot_notifier(intel_dp-edp_notifier);
+ intel_dp-edp_notifier.notifier_call = NULL;
+ }
}
kfree(intel_dig_port);
}
@@ -3782,6 +3821,10 @@ intel_dp_init_connector(struct intel_digital_port
*intel_dig_port,
if (is_edp(intel_dp)) {
intel_dp_init_panel_power_timestamps(intel_dp);
intel_dp_init_panel_power_sequencer(dev, intel_dp, power_seq);
+ if (IS_VALLEYVIEW(dev)) {
+ intel_dp-edp_notifier.notifier_call =
edp_notify_handler;
+ register_reboot_notifier(intel_dp-edp_notifier);
+ }
}
intel_dp_aux_init(intel_dp, intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 328b1a7..1ea193a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -512,6 +512,7 @@ struct intel_dp {
bool psr_setup_done;
bool use_tps3;
struct intel_connector *attached_connector;
+ struct notifier_block edp_notifier;
uint32_t (*get_aux_clock_divider)(struct intel_dp *dp, int index);
/*
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/vlv: T12 eDP panel timing enforcement during reboot.
On Tue, 13 May 2014 22:26:08 +0200
Daniel Vetter dan...@ffwll.ch wrote:
On Tue, May 13, 2014 at 11:51:11AM -0700, clinton.a.tay...@intel.com wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay for
warm reboots to the T12 panel timing as defined in the VBT table for the
connected panel.
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 43
++
drivers/gpu/drm/i915/intel_drv.h |1 +
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 5ca68aa9..03ac64f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -302,6 +304,39 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to gaurantee T12 timing
*/
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+edp_notifier);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp))
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ if (code == SYS_RESTART) {
+ pr_crit(eDP Notifier Handler\n);
+ pp = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp = PP_REFERENCE_DIVIDER_MASK;
+ I915_WRITE(pp_div_reg , pp | 0x1F);
+ I915_WRITE(pp_ctrl_reg,
+ PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ /* VBT entries are in tenths of uS convert to mS */
+ msleep(dev_priv-vbt.edp_pps.t11_t12 / 10);
Imo just compute the desired delay before setting up this reboot handler
and pass it as the argument.
But that leaves a bit the question why we don't need that everywhere else
and what exactly this does? I'm a bit confused with the commit message.
-Daniel
We could potentially do it on other platforms, but it's really only
needed on those that don't do any display stuff in the BIOS (e.g.
Chromebooks). In that case, a warm reboot might immediately jump back
to driver init, and if we haven't full done a PPS, we'll get into
trouble and potentially confuse the panel in the worst case.
Previous BIOS-free systems may have had this problem too, but this was
the first one we got an actual eDP timing spec violation about, so it's
a good first step. It can easily be extended as needed.
Computing the delay ahead of time in eDP init would make it more
platform agnostic, and we could key off a separate flag as to whether
the delay was needed, but the above is pretty simple too, albeit VLV
specific.
The pr_crit() could probably be dropped though, and the magic 0x1f
needs a comment.
Thanks,
--
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] drm/i915/vlv: T12 eDP panel timing enforcement during reboot.
On Tue, May 13, 2014 at 02:53:22PM -0700, Jesse Barnes wrote:
On Tue, 13 May 2014 22:26:08 +0200
Daniel Vetter dan...@ffwll.ch wrote:
On Tue, May 13, 2014 at 11:51:11AM -0700, clinton.a.tay...@intel.com wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 43
++
drivers/gpu/drm/i915/intel_drv.h |1 +
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 5ca68aa9..03ac64f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -302,6 +304,39 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to gaurantee T12
timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned long
code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+ edp_notifier);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp))
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ if (code == SYS_RESTART) {
+ pr_crit(eDP Notifier Handler\n);
+ pp = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp = PP_REFERENCE_DIVIDER_MASK;
+ I915_WRITE(pp_div_reg , pp | 0x1F);
+ I915_WRITE(pp_ctrl_reg,
+PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ /* VBT entries are in tenths of uS convert to mS */
+ msleep(dev_priv-vbt.edp_pps.t11_t12 / 10);
Imo just compute the desired delay before setting up this reboot handler
and pass it as the argument.
But that leaves a bit the question why we don't need that everywhere else
and what exactly this does? I'm a bit confused with the commit message.
-Daniel
We could potentially do it on other platforms, but it's really only
needed on those that don't do any display stuff in the BIOS (e.g.
Chromebooks). In that case, a warm reboot might immediately jump back
to driver init, and if we haven't full done a PPS, we'll get into
trouble and potentially confuse the panel in the worst case.
Previous BIOS-free systems may have had this problem too, but this was
the first one we got an actual eDP timing spec violation about, so it's
a good first step. It can easily be extended as needed.
Computing the delay ahead of time in eDP init would make it more
platform agnostic, and we could key off a separate flag as to whether
the delay was needed, but the above is pretty simple too, albeit VLV
specific.
The pr_crit() could probably be dropped though, and the magic 0x1f
needs a comment.
In that case I think rolling this out everywhere can't hurt. Gives us a
notch more testing and rebooting tends to not be a performance critical
thing really.
Otoh I've thought the hardware ensure that the t12 timing is obeyed under
all cases? Does that get lost in the reset?
-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/vlv: T12 eDP panel timing enforcement during reboot.
On Wed, 14 May 2014 00:32:30 +0200
Daniel Vetter dan...@ffwll.ch wrote:
On Tue, May 13, 2014 at 02:53:22PM -0700, Jesse Barnes wrote:
On Tue, 13 May 2014 22:26:08 +0200
Daniel Vetter dan...@ffwll.ch wrote:
On Tue, May 13, 2014 at 11:51:11AM -0700, clinton.a.tay...@intel.com
wrote:
From: Clint Taylor clinton.a.tay...@intel.com
The panel power sequencer on vlv doesn't appear to accept changes to
its T12 power down duration during warm reboots. This change forces a
delay for warm reboots to the T12 panel timing as defined in the VBT
table for the connected panel.
Signed-off-by: Clint Taylor clinton.a.tay...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 43
++
drivers/gpu/drm/i915/intel_drv.h |1 +
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 5ca68aa9..03ac64f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/export.h
+#include linux/notifier.h
+#include linux/reboot.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_crtc_helper.h
@@ -302,6 +304,39 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
return
VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
}
+/* Reboot notifier handler to shutdown panel power to gaurantee T12
timing */
+static int edp_notify_handler(struct notifier_block *this, unsigned
long code,
+ void *unused)
+{
+ struct intel_dp *intel_dp = container_of(this, typeof(*
intel_dp),
+edp_notifier);
+ struct intel_digital_port *intel_dig_port =
dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port-base.base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ u32 pp;
+ u32 pp_ctrl_reg, pp_div_reg;
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ if (!is_edp(intel_dp))
+ return 0;
+
+ if (IS_VALLEYVIEW(dev)) {
+ pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+ pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
+ if (code == SYS_RESTART) {
+ pr_crit(eDP Notifier Handler\n);
+ pp = I915_READ(VLV_PIPE_PP_DIVISOR(pipe));
+ pp = PP_REFERENCE_DIVIDER_MASK;
+ I915_WRITE(pp_div_reg , pp | 0x1F);
+ I915_WRITE(pp_ctrl_reg,
+ PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+ /* VBT entries are in tenths of uS convert to
mS */
+ msleep(dev_priv-vbt.edp_pps.t11_t12 / 10);
Imo just compute the desired delay before setting up this reboot handler
and pass it as the argument.
But that leaves a bit the question why we don't need that everywhere else
and what exactly this does? I'm a bit confused with the commit message.
-Daniel
We could potentially do it on other platforms, but it's really only
needed on those that don't do any display stuff in the BIOS (e.g.
Chromebooks). In that case, a warm reboot might immediately jump back
to driver init, and if we haven't full done a PPS, we'll get into
trouble and potentially confuse the panel in the worst case.
Previous BIOS-free systems may have had this problem too, but this was
the first one we got an actual eDP timing spec violation about, so it's
a good first step. It can easily be extended as needed.
Computing the delay ahead of time in eDP init would make it more
platform agnostic, and we could key off a separate flag as to whether
the delay was needed, but the above is pretty simple too, albeit VLV
specific.
The pr_crit() could probably be dropped though, and the magic 0x1f
needs a comment.
In that case I think rolling this out everywhere can't hurt. Gives us a
notch more testing and rebooting tends to not be a performance critical
thing really.
Otoh I've thought the hardware ensure that the t12 timing is obeyed under
all cases? Does that get lost in the reset?
Yeah on reset the PPS will get reset too, so the off-on delay may not
be honored.
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
