[PATCH] drm: bridge: Detach all bridges in a chain at encoder cleanup time

[Laurent Pinchart]

Instead of detaching only the bridge directly connected to the encoder,
detach all bridges in the chain.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/drm_encoder.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Hi Daniel,

As requested, here's a patch that makes bridge detach recursive. It's based on
top of "[PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder
cleanup time".

diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 9c1f99646e0d..33d3e6ec83be 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -164,8 +164,16 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
 * the indices on the drm_encoder after us in the encoder_list.
 */

-   if (encoder->bridge)
-   drm_bridge_detach(encoder->bridge);
+   if (encoder->bridge) {
+   struct drm_bridge *bridge = encoder->bridge;
+   struct drm_bridge *next;
+
+   while (bridge) {
+   next = bridge->next;
+   drm_bridge_detach(bridge);
+   bridge = next;
+   }
+   }

drm_modeset_lock_all(dev);
drm_mode_object_unregister(dev, >base);
-- 
Regards,

Laurent Pinchart

[PATCH v2 2/3] drm: Pull together probe + setup for drm_fb_helper

[Daniel Vetter]

On Tue, Nov 29, 2016 at 11:26 PM, Chris Wilson  
wrote:
> On Tue, Nov 29, 2016 at 09:29:06PM +0100, Daniel Vetter wrote:
>> On Tue, Nov 29, 2016 at 12:02:16PM +, Chris Wilson wrote:
>> > drm_fb_helper_probe_connector_modes() is always called before
>> > drm_setup_crtcs(), so just move the call into drm_setup_crtcs for a
>> > small bit of code compaction.
>> >
>> > Signed-off-by: Chris Wilson 
>> > Reviewed-by: Daniel Vetter 
>>
>> rb not entirely correct, since it's missing the crucial note I asked for
>> to understand things:
>>
>> "Note that register_framebuffer will do a modeset (when fbcon is enabled)
>> and hence must be moved out of the critical section. A follow-up patch
>> will add new locking for the fb list, hence move all the related
>> registration code together."
>
> Which is in patch 1 where the move was required.

Oh dear did I screw up :( My apologies for the mess I caused, should
have checked things properly ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/drm_bridge: adjust bridge module's refcount

[Jyri Sarha]

On 11/29/16 23:12, Jyri Sarha wrote:
> @@ -114,6 +118,9 @@ int drm_bridge_attach(struct drm_device *dev, struct 
> drm_bridge *bridge)
>   if (bridge->dev)
>   return -EBUSY;
>  
> + if (!try_module_get(bridge->module))
> + return -ENOENT;
> +
>   bridge->dev = dev;
>  
>   if (bridge->funcs->attach)
This sill needs:

-   if (bridge->funcs->attach)
-   return bridge->funcs->attach(bridge);
+   if (bridge->funcs->attach) {
+   int ret = bridge->funcs->attach(bridge);
+
+   if (ret) {
+   module_put(bridge->module);
+   return ret;
+   }
+   }

> @@ -144,6 +151,8 @@ void drm_bridge_detach(struct drm_bridge *bridge)
>   if (bridge->funcs->detach)
>   bridge->funcs->detach(bridge);
>  
> + module_put(bridge->module);
> +
>   bridge->dev = NULL;
>  }
>  EXPORT_SYMBOL(drm_bridge_detach);

[PATCH 3/3] drm/i915: Implement Link Rate fallback on Link training failure

[Manasi Navare]

If link training at a link rate optimal for a particular
mode fails during modeset's atomic commit phase, then we
let the modeset complete and then retry. We save the link rate
value at which link training failed, update the link status property
to "BAD" and use a lower link rate to prune the modes. It will redo
the modeset on the current mode at lower link rate or if the current
mode gets pruned due to lower link constraints then, it will send a
hotplug uevent for userspace to handle it.

This is also required to pass DP CTS tests 4.3.1.3, 4.3.1.4,
4.3.1.6.

v9:
* Use the trimmed max values of link rate/lane count based on
link train fallback (Daniel Vetter)
v8:
* Set link_status to BAD first and then call mode_valid (Jani Nikula)
v7:
Remove the redundant variable in previous patch itself
v6:
* Obtain link rate index from fallback_link_rate using
the helper intel_dp_link_rate_index (Jani Nikula)
* Include fallback within intel_dp_start_link_train (Jani Nikula)
v5:
* Move set link status to drm core (Daniel Vetter, Jani Nikula)
v4:
* Add fallback support for non DDI platforms too
* Set connector->link status inside set_link_status function
(Jani Nikula)
v3:
* Set link status property to BAd unconditionally (Jani Nikula)
* Dont use two separate variables link_train_failed and link_status
to indicate same thing (Jani Nikula)
v2:
* Squashed a few patches (Jani Nikula)

Acked-by: Tony Cheng 
Acked-by: Harry Wentland 
Cc: Jani Nikula 
Cc: Daniel Vetter 
Cc: Ville Syrjala 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/i915/intel_dp.c   | 44 +++
 drivers/gpu/drm/i915/intel_dp_link_training.c | 25 +--
 drivers/gpu/drm/i915/intel_drv.h  |  3 ++
 3 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bc1268c..a50b6cd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4435,6 +4435,8 @@ static bool intel_digital_port_connected(struct 
drm_i915_private *dev_priv,
intel_dp->compliance_test_active = 0;
intel_dp->compliance_test_type = 0;
intel_dp->compliance_test_data = 0;
+   intel_dp->fallback_link_rate = 0;
+   intel_dp->fallback_lane_count = 0;

if (intel_dp->is_mst) {
DRM_DEBUG_KMS("MST device may have disappeared %d vs 
%d\n",
@@ -4526,6 +4528,13 @@ static bool intel_digital_port_connected(struct 
drm_i915_private *dev_priv,
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
  connector->base.id, connector->name);

+   /* If this is a retry due to link training failure
+* then do no do a full detect
+*/
+   if (status == connector_status_connected &&
+   intel_dp->fallback_lane_count)
+   return status;
+
/* If full detect is not performed yet, do a full detect */
if (!intel_dp->detect_done)
status = intel_dp_long_pulse(intel_dp->attached_connector);
@@ -5690,6 +5699,37 @@ static bool intel_edp_init_connector(struct intel_dp 
*intel_dp,
return false;
 }

+static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
+{
+   struct intel_connector *intel_connector;
+   struct drm_connector *connector;
+   struct drm_display_mode *mode;
+   bool verbose_prune = true;
+
+   intel_connector = container_of(work, typeof(*intel_connector),
+  modeset_retry_work);
+   connector = _connector->base;
+   DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
+ connector->name);
+
+   /* Grab the locks before changing connector property*/
+   mutex_lock(>dev->mode_config.mutex);
+   /* Set connector link status to BAD and send a Uevent to notify
+* userspace to do a modeset.
+*/
+   drm_mode_connector_set_link_status_property(connector,
+   DRM_MODE_LINK_STATUS_BAD);
+   list_for_each_entry(mode, >modes, head) {
+   mode->status = intel_dp_mode_valid(connector,
+  mode);
+   }
+   drm_mode_prune_invalid(connector->dev, >modes,
+  verbose_prune);
+   mutex_unlock(>dev->mode_config.mutex);
+   /* Send Hotplug uevent so userspace can reprobe */
+   drm_kms_helper_hotplug_event(connector->dev);
+}
+
 bool
 intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
struct intel_connector *intel_connector)
@@ -5702,6 +5742,10 @@ static bool intel_edp_init_connector(struct intel_dp 
*intel_dp,
enum port port = intel_dig_port->port;
int type;

+   /* Initialize the work for modeset in case of link train failure */
+   INIT_WORK(_connector->modeset_retry_work,
+ intel_dp_modeset_retry_work_fn);
+
 

[PATCH 2/3] drm/i915: Find fallback link rate/lane count

[Manasi Navare]

If link training fails, then we need to fallback to lower
link rate first and if link training fails at RBR, then
fallback to lower lane count.
This function finds the next lower link rate/lane count
value after link training failure and limits the max
link_rate and lane_count values to these fallback values.

v6:
* Cap the max link rate and lane count to the max
values obtained during fallback link training (Daniel Vetter)
v5:
* Start the fallback at the lane count value passed not
the max lane count (Jani Nikula)
v4:
* Remove the redundant variable link_train_failed
v3:
* Remove fallback_link_rate_index variable, just obtain
that using the helper intel_dp_link_rate_index (Jani Nikula)
v2:
Squash the patch that returns the link rate index (Jani Nikula)

Acked-by: Tony Cheng 
Acked-by: Harry Wentland 
Cc: Ville Syrjala 
Cc: Jani Nikula 
Cc: Daniel Vetter 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/i915/intel_dp.c  | 59 ++--
 drivers/gpu/drm/i915/intel_drv.h |  4 +++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8026a83..bc1268c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -153,12 +153,18 @@ static void vlv_steal_power_sequencer(struct drm_device 
*dev,
 static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
 {
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-   u8 source_max, sink_max;
+   u8 temp, source_max, sink_max;

source_max = intel_dig_port->max_lanes;
sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
+   temp = min(source_max, sink_max);
+   /* Limit the max value to max fallback value during link train
+* failure handling.
+*/
+   if (intel_dp->fallback_lane_count)
+   return min(temp, intel_dp->fallback_lane_count);

-   return min(source_max, sink_max);
+   return temp;
 }

 /*
@@ -281,6 +287,15 @@ static int intel_dp_common_rates(struct intel_dp *intel_dp,
int source_len, sink_len;

sink_len = intel_dp_sink_rates(intel_dp, _rates);
+   /* Limit the max sink rate to the max fallback link rate
+* found during link training failure handling.
+*/
+   if (intel_dp->fallback_link_rate) {
+   int len = sink_len - 1;
+   while (len > 0 && sink_rates[len] > 
intel_dp->fallback_link_rate)
+   len--;
+   sink_len = len + 1;
+   }
source_len = intel_dp_source_rates(intel_dp, _rates);

return intersect_rates(source_rates, source_len,
@@ -288,6 +303,46 @@ static int intel_dp_common_rates(struct intel_dp *intel_dp,
   common_rates);
 }

+static int intel_dp_link_rate_index(struct intel_dp *intel_dp,
+   int *common_rates, int link_rate)
+{
+   int common_len;
+   int index;
+
+   common_len = intel_dp_common_rates(intel_dp, common_rates);
+   for (index = 0; index < common_len; index++) {
+   if (link_rate == common_rates[common_len - index - 1])
+   return common_len - index - 1;
+   }
+
+   return -1;
+}
+
+int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
+   int link_rate, uint8_t lane_count)
+{
+   int common_rates[DP_MAX_SUPPORTED_RATES] = {};
+   int common_len;
+   int link_rate_index = -1;
+
+   common_len = intel_dp_common_rates(intel_dp, common_rates);
+   link_rate_index = intel_dp_link_rate_index(intel_dp,
+  common_rates,
+  link_rate);
+   if (link_rate_index > 0) {
+   intel_dp->fallback_link_rate = common_rates[link_rate_index - 
1];
+   intel_dp->fallback_lane_count = lane_count;
+   } else if (lane_count > 1) {
+   intel_dp->fallback_link_rate = 0;
+   intel_dp->fallback_lane_count = lane_count >> 1;
+   } else {
+   DRM_ERROR("Link Training Unsuccessful\n");
+   return -1;
+   }
+
+   return 0;
+}
+
 static enum drm_mode_status
 intel_dp_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9bbe5c5..2da3b40 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -891,6 +891,8 @@ struct intel_dp {
uint32_t DP;
int link_rate;
uint8_t lane_count;
+   int fallback_link_rate;
+   uint8_t fallback_lane_count;
uint8_t sink_count;
bool link_mst;
bool has_audio;
@@ -1393,6 +1395,8 @@ bool intel_dp_init_connector(struct intel_digital_port 
*intel_dig_port,
 void intel_dp_set_link_params(struct intel_dp *intel_dp,
   

[PATCH 1/3] drm: Add a new connector atomic property for link status

[Manasi Navare]

At the time userspace does setcrtc, we've already promised the mode
would work. The promise is based on the theoretical capabilities of
the link, but it's possible we can't reach this in practice. The DP
spec describes how the link should be reduced, but we can't reduce
the link below the requirements of the mode. Black screen follows.

One idea would be to have setcrtc return a failure. However, it
already should not fail as the atomic checks have passed. It would
also conflict with the idea of making setcrtc asynchronous in the
future, returning before the actual mode setting and link training.

Another idea is to train the link "upfront" at hotplug time, before
pruning the mode list, so that we can do the pruning based on
practical not theoretical capabilities. However, the changes for link
training are pretty drastic, all for the sake of error handling and
DP compliance, when the most common happy day scenario is the current
approach of link training at mode setting time, using the optimal
parameters for the mode. It is also not certain all hardware could do
this without the pipe on; not even all our hardware can do this. Some
of this can be solved, but not trivially.

Both of the above ideas also fail to address link degradation *during*
operation.

The solution is to add a new "link-status" connector property in order
to address link training failure in a way that:
a) changes the current happy day scenario as little as possible, to
avoid regressions, b) can be implemented the same way by all drm
drivers, c) is still opt-in for the drivers and userspace, and opting
out doesn't regress the user experience, d) doesn't prevent drivers
from implementing better or alternate approaches, possibly without
userspace involvement. And, of course, handles all the issues presented.
In the usual happy day scenario, this is always "good". If something
fails during or after a mode set, the kernel driver can set the link
status to "bad" and issue a hotplug uevent for userspace to have it
re-check the valid modes through GET_CONNECTOR IOCTL, and try modeset
again. If the theoretical capabilities of the link can't be reached,
the mode list is trimmed based on that.

v4:
* Add comments in kernel-doc format (Daniel Vetter)
* Update the kernel-doc for link-status (Sean Paul)
v3:
* Fixed a build error (Jani Saarinen)
v2:
* Removed connector->link_status (Daniel Vetter)
* Set connector->state->link_status in 
drm_mode_connector_set_link_status_property
(Daniel Vetter)
* Set the connector_changed flag to true if connector->state->link_status 
changed.
* Reset link_status to GOOD in update_output_state (Daniel Vetter)
* Never allow userspace to set link status from Good To Bad (Daniel Vetter)

Acked-by: Tony Cheng 
Acked-by: Harry Wentland 
Cc: Jani Nikula 
Cc: Daniel Vetter 
Cc: Ville Syrjala 
Cc: Chris Wilson 
Cc: Sean Paul 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/drm_atomic.c| 10 +++
 drivers/gpu/drm/drm_atomic_helper.c |  8 ++
 drivers/gpu/drm/drm_connector.c | 54 -
 include/drm/drm_connector.h | 19 +
 include/drm/drm_mode_config.h   |  5 
 include/uapi/drm/drm_mode.h |  4 +++
 6 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 89737e4..990f013 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1087,6 +1087,14 @@ int drm_atomic_connector_set_property(struct 
drm_connector *connector,
 * now?) atomic writes to DPMS property:
 */
return -EINVAL;
+   } else if (property == config->link_status_property) {
+   /* Never downgrade from GOOD to BAD on userspace's request here,
+* only hw issues can do that.
+*/
+   if (state->link_status == DRM_LINK_STATUS_GOOD)
+   return 0;
+   state->link_status = val;
+   return 0;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -1135,6 +1143,8 @@ static void drm_atomic_connector_print_state(struct 
drm_printer *p,
*val = (state->crtc) ? state->crtc->base.id : 0;
} else if (property == config->dpms_property) {
*val = connector->dpms;
+   } else if (property == config->link_status_property) {
+   *val = state->link_status;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 494680c..962ed66 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -519,6 +519,13 @@ static int 

[PATCH] drm: Make the connector .detect() callback optional

[Jyri Sarha]

On 11/29/16 22:56, Laurent Pinchart wrote:
> Many drivers (21 to be exact) create connectors that are always
> connected (for instance to an LVDS or DSI panel). Instead of forcing
> them to implement a dummy .detect() handler, make the callback optional
> and consider the connector as always connected in that case.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c |  7 ---
>  drivers/gpu/drm/arc/arcpgu_sim.c |  7 ---
>  drivers/gpu/drm/ast/ast_mode.c   |  7 ---
>  drivers/gpu/drm/bochs/bochs_kms.c|  7 ---
>  drivers/gpu/drm/bridge/nxp-ptn3460.c |  7 ---
>  drivers/gpu/drm/bridge/parade-ps8622.c   |  7 ---
>  drivers/gpu/drm/bridge/tc358767.c|  7 ---
>  drivers/gpu/drm/cirrus/cirrus_mode.c |  7 ---
>  drivers/gpu/drm/drm_probe_helper.c   | 14 +++---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c|  7 ---
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c  | 14 --
>  drivers/gpu/drm/gma500/psb_intel_lvds.c  | 14 --
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c |  7 ---
>  drivers/gpu/drm/i915/intel_dsi.c |  7 ---
>  drivers/gpu/drm/imx/imx-ldb.c|  7 ---
>  drivers/gpu/drm/imx/imx-tve.c|  7 ---
>  drivers/gpu/drm/imx/parallel-display.c   |  7 ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c   |  7 ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c   |  7 ---
>  drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c|  7 ---
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c   |  7 ---
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c|  7 ---
>  drivers/gpu/drm/sti/sti_hda.c|  7 ---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c|  7 ---
>  drivers/gpu/drm/sun4i/sun4i_tv.c |  7 ---
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c|  8 
>  include/drm/drm_connector.h  |  3 +++
>  27 files changed, 14 insertions(+), 193 deletions(-)

Acked-by: Jyri Sarha 

... for tilcdc part.

[GIT PULL] exynos-drm-fixes

[Inki Dae]

2016-11-25 8:59 GMT+09:00 Dave Airlie :
>>No critial patch but it make sure to unmap the region
>>if HDMI probing failed, and it includes two trivial fixups.
>>
>
> I've cherry-picked the hdmi fix, but I think the other two patches
> should go in -next
> at this stage,

Thanks,
Inki Dae

>
> Thanks,
> Dave.
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/drm_bridge: adjust bridge module's refcount

[Jyri Sarha]

Store the module that provides the bridge and adjust its refcount
accordingly. The bridge module unload should not be allowed while the
bridge is attached.

Signed-off-by: Jyri Sarha 
---
 drivers/gpu/drm/drm_bridge.c | 15 ---
 include/drm/drm_bridge.h | 11 ++-
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 0ee052b..36d427b 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -61,22 +61,26 @@
 static LIST_HEAD(bridge_list);

 /**
- * drm_bridge_add - add the given bridge to the global bridge list
+ * __drm_bridge_add - add the given bridge to the global bridge list
  *
  * @bridge: bridge control structure
+ * @module: module that provides this bridge
  *
  * RETURNS:
  * Unconditionally returns Zero.
  */
-int drm_bridge_add(struct drm_bridge *bridge)
+int __drm_bridge_add(struct drm_bridge *bridge, struct module *module)
 {
+#ifdef MODULE
+   bridge->module = module;
+#endif
mutex_lock(_lock);
list_add_tail(>list, _list);
mutex_unlock(_lock);

return 0;
 }
-EXPORT_SYMBOL(drm_bridge_add);
+EXPORT_SYMBOL(__drm_bridge_add);

 /**
  * drm_bridge_remove - remove the given bridge from the global bridge list
@@ -114,6 +118,9 @@ int drm_bridge_attach(struct drm_device *dev, struct 
drm_bridge *bridge)
if (bridge->dev)
return -EBUSY;

+   if (!try_module_get(bridge->module))
+   return -ENOENT;
+
bridge->dev = dev;

if (bridge->funcs->attach)
@@ -144,6 +151,8 @@ void drm_bridge_detach(struct drm_bridge *bridge)
if (bridge->funcs->detach)
bridge->funcs->detach(bridge);

+   module_put(bridge->module);
+
bridge->dev = NULL;
 }
 EXPORT_SYMBOL(drm_bridge_detach);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 530a1d6..d60d5d2 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -25,6 +25,7 @@

 #include 
 #include 
+#include 
 #include 
 #include 

@@ -192,13 +193,21 @@ struct drm_bridge {
 #ifdef CONFIG_OF
struct device_node *of_node;
 #endif
+#ifdef MODULE
+   struct module *module;
+#endif
struct list_head list;

const struct drm_bridge_funcs *funcs;
void *driver_private;
 };

-int drm_bridge_add(struct drm_bridge *bridge);
+int __drm_bridge_add(struct drm_bridge *bridge, struct module *module);
+#ifdef MODULE
+#define drm_bridge_add(bridge) __drm_bridge_add(bridge, THIS_MODULE)
+#else
+#define drm_bridge_add(bridge) __drm_bridge_add(bridge, NULL)
+#endif
 void drm_bridge_remove(struct drm_bridge *bridge);
 struct drm_bridge *of_drm_find_bridge(struct device_node *np);
 int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge);
-- 
1.9.1

[PATCH] drm: Make the connector .detect() callback optional

[Laurent Pinchart]

Hi Sean,

On Tuesday 29 Nov 2016 16:07:02 Sean Paul wrote:
> On Tue, Nov 29, 2016 at 3:56 PM, Laurent Pinchart wrote:
> > Many drivers (21 to be exact) create connectors that are always
> > connected (for instance to an LVDS or DSI panel). Instead of forcing
> > them to implement a dummy .detect() handler, make the callback optional
> > and consider the connector as always connected in that case.
> 
> I wonder if we should be a little bit smarter about this and default
> connected only for built-in types (LVDS, edp, DSI), and return unknown
> for others?

I've deliberately decided not to change the behaviour of any driver in this 
patch to ease review and merge. We can change (a.k.a. improve :-)) the logic 
on top of this. Patches are welcome ;-)

> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/gpu/drm/amd/amdgpu/dce_virtual.c |  7 ---
> >  drivers/gpu/drm/arc/arcpgu_sim.c |  7 ---
> >  drivers/gpu/drm/ast/ast_mode.c   |  7 ---
> >  drivers/gpu/drm/bochs/bochs_kms.c|  7 ---
> >  drivers/gpu/drm/bridge/nxp-ptn3460.c |  7 ---
> >  drivers/gpu/drm/bridge/parade-ps8622.c   |  7 ---
> >  drivers/gpu/drm/bridge/tc358767.c|  7 ---
> >  drivers/gpu/drm/cirrus/cirrus_mode.c |  7 ---
> >  drivers/gpu/drm/drm_probe_helper.c   | 14 +++---
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c|  7 ---
> >  drivers/gpu/drm/gma500/cdv_intel_lvds.c  | 14 --
> >  drivers/gpu/drm/gma500/psb_intel_lvds.c  | 14 --
> >  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c |  7 ---
> >  drivers/gpu/drm/i915/intel_dsi.c |  7 ---
> >  drivers/gpu/drm/imx/imx-ldb.c|  7 ---
> >  drivers/gpu/drm/imx/imx-tve.c|  7 ---
> >  drivers/gpu/drm/imx/parallel-display.c   |  7 ---
> >  drivers/gpu/drm/mediatek/mtk_dsi.c   |  7 ---
> >  drivers/gpu/drm/mgag200/mgag200_mode.c   |  7 ---
> >  drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c|  7 ---
> >  drivers/gpu/drm/rockchip/dw-mipi-dsi.c   |  7 ---
> >  drivers/gpu/drm/shmobile/shmob_drm_crtc.c|  7 ---
> >  drivers/gpu/drm/sti/sti_hda.c|  7 ---
> >  drivers/gpu/drm/sun4i/sun4i_rgb.c|  7 ---
> >  drivers/gpu/drm/sun4i/sun4i_tv.c |  7 ---
> >  drivers/gpu/drm/tilcdc/tilcdc_panel.c|  8 
> >  include/drm/drm_connector.h  |  3 +++
> >  27 files changed, 14 insertions(+), 193 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v5 3/4] drm/bridge: Add ti-tfp410 DVI transmitter driver

[Jyri Sarha]

On 11/29/16 22:26, Laurent Pinchart wrote:
> Hi Jyri,
> 
> On Tuesday 29 Nov 2016 22:18:25 Jyri Sarha wrote:
>> > The module unload should not be allowed while the bridge is attached. So
>> > still need to add these:
>> > 
>> > On 11/25/16 11:02, Jyri Sarha wrote:
>>> > > +
>>> > > +static int tfp410_attach(struct drm_bridge *bridge)
>>> > > +{
>>> > > +   struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
>>> > > +   int ret;
>>> > > +
>> > 
>> > +   if (!try_module_get(THIS_MODULE)) {
>> > +   dev_err(dvi->dev, "Module unloading\n");
>> > +   return -ENODEV;
>> > +   }
>> > +
> Shouldn't this be done in core code ?
> 

Hmmm, probably. I'll send a patch shortly.

Cheers,
Jyri

[PATCH 2/3] drm/dp/mst: Calculate total link bandwidth instead of hardcoding it

[Ville Syrjälä]

On Thu, Nov 17, 2016 at 06:03:47PM -0800, Dhinakaran Pandiyan wrote:
> The total or the nominal link bandwidth, which we save in terms of PBN, is
> a factor of link rate and lane count. But, currently we hardcode it to
> 2560 PBN. This results in incorrect computation of total slots.
> 
> E.g, 2 lane HBR2 configuration and 4k at 60Hz, 24bpp mode
>   nominal link bw = 1080 MBps = 1280PBN = 64 slots
>   required bw 533.25 MHz*3 = 1599.75 MBps or 1896 PBN
>  with +0.6% margin = 1907.376 PBN = 96 slots
>   This is greater than the max. possible value of 64 slots. But, we
>   incorrectly compute available slots as 2560 PBN = 128 slots and don't
>   return error.
> 
> So, let's fix this by calculating the total link bandwidth as
> link bw (PBN) = BW per time slot(PBN) * max. time slots , where max. time
> slots is 64
> 
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 04e4571..26dfd99 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2038,9 +2038,8 @@ int drm_dp_mst_topology_mgr_set_mst(struct 
> drm_dp_mst_topology_mgr *mgr, bool ms
>   ret = -EINVAL;
>   goto out_unlock;
>   }
> -
> - mgr->total_pbn = 2560;
> - mgr->total_slots = DIV_ROUND_UP(mgr->total_pbn, mgr->pbn_div);
> + mgr->total_pbn = 64 * mgr->pbn_div;

Do we actually have a use in mind for total_pbn? It seems unused now.

> + mgr->total_slots = 64;

Same for total_slots.

>   mgr->avail_slots = mgr->total_slots;

So avail_slots is all that's used. And shouldn't it actually start
out at 63 instead of 64 on account of the MTP header always taking
up one slot?

>  
>   /* add initial branch device at LCT 1 */
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel OTC

[PATCH v6 3/5] ARM: dts: sun8i-h3: add HDMI video nodes

[Maxime Ripard]

On Fri, Nov 25, 2016 at 11:22:13AM +0100, Jean-Francois Moine wrote:
> On Fri, 25 Nov 2016 17:41:51 +0800
> Icenowy Zheng  wrote:
> 
> > After removing CLK_PLL_DE's assigned-clock, the kernel passes compilation.
> 
> The 'pll-de' and 'de' must have a fixed rate. Otherwise, if you do not
> use the legacy u-boot, I don't know which can be the rate of the DE.
> 
> > However, it cannot recognize any HDMI screen...
> > 
> > (My board is Orange Pi One, and I manually added status="okay"; to , 
> > , )
> > 
> > [   16.507802] sun8i-de2 100.de-controller: bound 
> > 1c0c000.lcd-controller (ops de2_lcd_ops [sun8i_de2_drm])
> > [   16.675948] sun8i-de2 100.de-controller: bound 1ee.hdmi (ops 
> > de2_hdmi_fini [sun8i_de2_hdmi])
> > [   16.685120] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> > [   16.695876] [drm] No driver support for vblank timestamp query.
> > [   16.701862] sun8i-de2 100.de-controller: No connectors reported 
> > connected with modes
> > [   16.713061] [drm] Cannot find any crtc or sizes - going 1024x768
> > [   16.734214] Console: switching to colour frame buffer device 128x48
> > [   16.751022] sun8i-de2 100.de-controller: fb0:  frame buffer device
> 
> I put a 'pr_warn' message is case the EDID cannot be read.
> Have you this message?
> 
> Anyway, there is a problem with the EDID:
> - my Orange Pi 2 (H3) randomly fails to read it. But this succeeds after
>   rebooting once or twice.
> - my Banana Pi M2+ (H3) reads it correctly each time.
> - my Banana Pi M3 (A83T) can never read it.
> 
> BTW, on first tries, I was forcing a CEA mode thru the kernel command
> line. This was working with the OPi2 and BPiM3, but there was no sound.
> In the last version, I use a EDID in edid_firmware for having sound
> with the BPiM3. This works fine.
> But, forcing a CEA mode is no more possible, so, when the OPi2 cannot
> read the EDID, the system switches to a VGA mode (default 1024x768)
> which is not supported. It seems that this is your case.
> 
> So, in brief, if your board cannot read the EDID, put a EDID somewhere
> and its path in /sys/module/drm_kms_helper/parameters/edid_firmware.
> There will be no console, but X11 will work correctly.

This is one of the things that are usually very helpful to put in a
cover letter. This is obviously also a blocker for the merge of the
driver, and should be dealt with.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/a265faef/attachment.sig>

[PATCH v3 06/13] drm: bridge: Add LVDS encoder driver

[Laurent Pinchart]

Hi Daniel,

On Tuesday 29 Nov 2016 10:54:09 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:04:36AM +0200, Laurent Pinchart wrote:
> > The LVDS encoder driver is a DRM bridge driver that supports the
> > parallel to LVDS encoders that don't require any configuration. The
> > driver thus doesn't interact with the device, but creates an LVDS
> > connector for the panel and exposes its size and timing based on
> > information retrieved from DT.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> Since it's 100% dummy, why put LVDS into the name? This little thing here
> could be our generic "wrap drm_panel and attach it to a chain" helper. So
> what about calling this _The_ drm_panel_bridge, and also linking it into
> docs to feature it a bit more prominently.

I'm not opposed to that, except that this driver should not be considered as 
just a helper to link a panel. It should only be used to support a real 
hardware bridge that requires no control.

> I came up with this because I spotted some refactoring belows for building
> this helper, until I realized that this driver _is_ the helper I think we
> want ;-) Only thing missing is an exported function to instantiate a
> bridge with just a drm_panel as the parameter. And putting it into the
> drm_kms_helper.ko module.

What would that be used for ? The bridge should be instantiated by this bridge 
driver, bound to a bridge device instantiated from DT (or the platform 
firmware of your choice).

> > +static enum drm_connector_status
> > +lvds_connector_detect(struct drm_connector *connector, bool force)
> > +{
> > +   return connector_status_connected;
> > +}
> 
> We have piles of this exact dummy callback all over, maybe make it the
> default and rip them all out?

Done, "[PATCH] drm: Make the connector .detect() callback optional".

-- 
Regards,

Laurent Pinchart

[PATCH] drm: Make the connector .detect() callback optional

[Laurent Pinchart]

Many drivers (21 to be exact) create connectors that are always
connected (for instance to an LVDS or DSI panel). Instead of forcing
them to implement a dummy .detect() handler, make the callback optional
and consider the connector as always connected in that case.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c |  7 ---
 drivers/gpu/drm/arc/arcpgu_sim.c |  7 ---
 drivers/gpu/drm/ast/ast_mode.c   |  7 ---
 drivers/gpu/drm/bochs/bochs_kms.c|  7 ---
 drivers/gpu/drm/bridge/nxp-ptn3460.c |  7 ---
 drivers/gpu/drm/bridge/parade-ps8622.c   |  7 ---
 drivers/gpu/drm/bridge/tc358767.c|  7 ---
 drivers/gpu/drm/cirrus/cirrus_mode.c |  7 ---
 drivers/gpu/drm/drm_probe_helper.c   | 14 +++---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c|  7 ---
 drivers/gpu/drm/gma500/cdv_intel_lvds.c  | 14 --
 drivers/gpu/drm/gma500/psb_intel_lvds.c  | 14 --
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c |  7 ---
 drivers/gpu/drm/i915/intel_dsi.c |  7 ---
 drivers/gpu/drm/imx/imx-ldb.c|  7 ---
 drivers/gpu/drm/imx/imx-tve.c|  7 ---
 drivers/gpu/drm/imx/parallel-display.c   |  7 ---
 drivers/gpu/drm/mediatek/mtk_dsi.c   |  7 ---
 drivers/gpu/drm/mgag200/mgag200_mode.c   |  7 ---
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c|  7 ---
 drivers/gpu/drm/rockchip/dw-mipi-dsi.c   |  7 ---
 drivers/gpu/drm/shmobile/shmob_drm_crtc.c|  7 ---
 drivers/gpu/drm/sti/sti_hda.c|  7 ---
 drivers/gpu/drm/sun4i/sun4i_rgb.c|  7 ---
 drivers/gpu/drm/sun4i/sun4i_tv.c |  7 ---
 drivers/gpu/drm/tilcdc/tilcdc_panel.c|  8 
 include/drm/drm_connector.h  |  3 +++
 27 files changed, 14 insertions(+), 193 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 81cbf0b05dff..1d93e123532d 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -429,12 +429,6 @@ dce_virtual_dpms(struct drm_connector *connector, int mode)
return 0;
 }

-static enum drm_connector_status
-dce_virtual_detect(struct drm_connector *connector, bool force)
-{
-   return connector_status_connected;
-}
-
 static int
 dce_virtual_set_property(struct drm_connector *connector,
 struct drm_property *property,
@@ -463,7 +457,6 @@ static const struct drm_connector_helper_funcs 
dce_virtual_connector_helper_func

 static const struct drm_connector_funcs dce_virtual_connector_funcs = {
.dpms = dce_virtual_dpms,
-   .detect = dce_virtual_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.set_property = dce_virtual_set_property,
.destroy = dce_virtual_destroy,
diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c b/drivers/gpu/drm/arc/arcpgu_sim.c
index 2bf06d71556a..bca3a678c955 100644
--- a/drivers/gpu/drm/arc/arcpgu_sim.c
+++ b/drivers/gpu/drm/arc/arcpgu_sim.c
@@ -41,12 +41,6 @@ static int arcpgu_drm_connector_get_modes(struct 
drm_connector *connector)
return count;
 }

-static enum drm_connector_status
-arcpgu_drm_connector_detect(struct drm_connector *connector, bool force)
-{
-   return connector_status_connected;
-}
-
 static void arcpgu_drm_connector_destroy(struct drm_connector *connector)
 {
drm_connector_unregister(connector);
@@ -61,7 +55,6 @@ arcpgu_drm_connector_helper_funcs = {
 static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
.dpms = drm_helper_connector_dpms,
.reset = drm_atomic_helper_connector_reset,
-   .detect = arcpgu_drm_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = arcpgu_drm_connector_destroy,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 5957c3e659fe..e26c98f51eb4 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -839,12 +839,6 @@ static void ast_connector_destroy(struct drm_connector 
*connector)
kfree(connector);
 }

-static enum drm_connector_status
-ast_connector_detect(struct drm_connector *connector, bool force)
-{
-   return connector_status_connected;
-}
-
 static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
.mode_valid = ast_mode_valid,
.get_modes = ast_get_modes,
@@ -853,7 +847,6 @@ static const struct drm_connector_helper_funcs 
ast_connector_helper_funcs = {

 static const struct drm_connector_funcs ast_connector_funcs = {
.dpms = 

[PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Maxime Ripard]

On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> This patchset series adds HDMI video support to the Allwinner
> sun8i SoCs which include the display engine 2 (DE2).
> The driver contains the code for the A83T and H3 SoCs, and
> some H3 boards, but it could be used/extended for other SoCs
> (A64, H2, H5) and boards (Banana PIs, Orange PIs).

Honestly, I'm getting a bit worried by the fact that you ignore
reviews.

On the important reviews that you got that are to be seen as major
issues that block the inclusion, we have:
  - The fact that the HDMI driver is actually just a designware IP,
and while you should use the driver that already exists, you just
duplicated all that code.

  - The fact that you ignored Rob (v6) and I (v5) comment on using OF
graph to model the connection between the display engine and the
TCON. Something that Laurent also pointed out in this version.

  - The fact that you ignored that you needed an HDMI connector node
as a child of the HDMI controller. This has been reported by Rob
(v6) and yet again in this version by Laurent.

  - And finally the fact that we can't have several display engine in
parallel, if needs be. This has happened in the past already on
Allwinner SoCs, so it's definitely something we should consider in
the DT bindings, since we can't break them.

Until those are fixed, I cannot see how this driver can be merged,
unfortunately.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/3fff35b9/attachment.sig>

[linux-sunxi] Re: [PATCH v7 1/8] drm: sun8i: Add a basic DRM driver for Allwinner DE2

[Icenowy Zheng]

29.11.2016, 22:30, "Daniel Vetter" :
> On Mon, Nov 28, 2016 at 03:23:54PM +0100, Jean-Francois Moine wrote:
>>  Allwinner's recent SoCs, as A64, A83T and H3, contain a new display
>>  engine, DE2.
>>  This patch adds a DRM video driver for this device.
>>
>>  Signed-off-by: Jean-Francois Moine 
>
> Scrolled around a bit, seemed all reasonable.
>
> Acked-by: Daniel Vetter 
>
> Not sure a new driver for each chip is reasonable, experience says that
> long-term you want to share quite a pile of code between different hw
> platforms from the same vendor. But that's entirely up to you.
> -Daniel

The Display Engine on A83T and newer SoCs is quite new, different from
the ones on A33 and older.

It's called "DE 2.0" in the user manual of the SoCs.

>
>>  ---
>>   drivers/gpu/drm/Kconfig | 2 +
>>   drivers/gpu/drm/Makefile | 1 +
>>   drivers/gpu/drm/sun8i/Kconfig | 19 +
>>   drivers/gpu/drm/sun8i/Makefile | 7 +
>>   drivers/gpu/drm/sun8i/de2_crtc.c | 449 +++
>>   drivers/gpu/drm/sun8i/de2_crtc.h | 50 +++
>>   drivers/gpu/drm/sun8i/de2_drv.c | 317 
>>   drivers/gpu/drm/sun8i/de2_drv.h | 48 +++
>>   drivers/gpu/drm/sun8i/de2_plane.c | 734 
>> ++
>>   9 files changed, 1627 insertions(+)
>>   create mode 100644 drivers/gpu/drm/sun8i/Kconfig
>>   create mode 100644 drivers/gpu/drm/sun8i/Makefile
>>   create mode 100644 drivers/gpu/drm/sun8i/de2_crtc.c
>>   create mode 100644 drivers/gpu/drm/sun8i/de2_crtc.h
>>   create mode 100644 drivers/gpu/drm/sun8i/de2_drv.c
>>   create mode 100644 drivers/gpu/drm/sun8i/de2_drv.h
>>   create mode 100644 drivers/gpu/drm/sun8i/de2_plane.c
>>
>>  diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>  index 95fc041..bb1bfbc 100644
>>  --- a/drivers/gpu/drm/Kconfig
>>  +++ b/drivers/gpu/drm/Kconfig
>>  @@ -202,6 +202,8 @@ source "drivers/gpu/drm/shmobile/Kconfig"
>>
>>   source "drivers/gpu/drm/sun4i/Kconfig"
>>
>>  +source "drivers/gpu/drm/sun8i/Kconfig"
>>  +
>>   source "drivers/gpu/drm/omapdrm/Kconfig"
>>
>>   source "drivers/gpu/drm/tilcdc/Kconfig"
>>  diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>>  index 883f3e7..3e1eaa0 100644
>>  --- a/drivers/gpu/drm/Makefile
>>  +++ b/drivers/gpu/drm/Makefile
>>  @@ -72,6 +72,7 @@ obj-$(CONFIG_DRM_RCAR_DU) += rcar-du/
>>   obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/
>>   obj-y += omapdrm/
>>   obj-$(CONFIG_DRM_SUN4I) += sun4i/
>>  +obj-$(CONFIG_DRM_SUN8I) += sun8i/
>>   obj-y += tilcdc/
>>   obj-$(CONFIG_DRM_QXL) += qxl/
>>   obj-$(CONFIG_DRM_BOCHS) += bochs/
>>  diff --git a/drivers/gpu/drm/sun8i/Kconfig b/drivers/gpu/drm/sun8i/Kconfig
>>  new file mode 100644
>>  index 000..6940895
>>  --- /dev/null
>>  +++ b/drivers/gpu/drm/sun8i/Kconfig
>>  @@ -0,0 +1,19 @@
>>  +#
>>  +# Allwinner DE2 Video configuration
>>  +#
>>  +
>>  +config DRM_SUN8I
>>  + bool
>>  +
>>  +config DRM_SUN8I_DE2
>>  + tristate "Support for Allwinner Video with DE2 interface"
>>  + depends on DRM && OF
>>  + depends on ARCH_SUNXI || COMPILE_TEST
>>  + select DRM_GEM_CMA_HELPER
>>  + select DRM_KMS_CMA_HELPER
>>  + select DRM_KMS_HELPER
>>  + select DRM_SUN8I
>>  + help
>>  + Choose this option if your Allwinner chipset has the DE2 interface
>>  + as the A64, A83T and H3. If M is selected the module will be called
>>  + sun8i-de2-drm.
>>  diff --git a/drivers/gpu/drm/sun8i/Makefile 
>> b/drivers/gpu/drm/sun8i/Makefile
>>  new file mode 100644
>>  index 000..f107919
>>  --- /dev/null
>>  +++ b/drivers/gpu/drm/sun8i/Makefile
>>  @@ -0,0 +1,7 @@
>>  +#
>>  +# Makefile for Allwinner's sun8i DRM device driver
>>  +#
>>  +
>>  +sun8i-de2-drm-objs := de2_drv.o de2_crtc.o de2_plane.o
>>  +
>>  +obj-$(CONFIG_DRM_SUN8I_DE2) += sun8i-de2-drm.o
>>  diff --git a/drivers/gpu/drm/sun8i/de2_crtc.c 
>> b/drivers/gpu/drm/sun8i/de2_crtc.c
>>  new file mode 100644
>>  index 000..4e94ccc
>>  --- /dev/null
>>  +++ b/drivers/gpu/drm/sun8i/de2_crtc.c
>>  @@ -0,0 +1,449 @@
>>  +/*
>>  + * Allwinner DRM driver - DE2 CRTC
>>  + *
>>  + * Copyright (C) 2016 Jean-Francois Moine 
>>  + *
>>  + * This program is free software; you can redistribute it and/or
>>  + * modify it under the terms of the GNU General Public License as
>>  + * published by the Free Software Foundation; either version 2 of
>>  + * the License, or (at your option) any later version.
>>  + */
>>  +
>>  +#include 
>>  +#include 
>>  +#include 
>>  +#include 
>>  +#include 
>>  +#include 
>>  +
>>  +#include "de2_drv.h"
>>  +#include "de2_crtc.h"
>>  +
>>  +/* I/O map */
>>  +
>>  +#define TCON_GCTL_REG 0x00
>>  +#define TCON_GCTL_TCON_ENABLE BIT(31)
>>  +#define TCON_GINT0_REG 0x04
>>  +#define TCON_GINT0_TCON1_Vb_Int_En BIT(30)
>>  +#define TCON_GINT0_TCON1_Vb_Int_Flag BIT(14)
>>  +#define TCON_GINT0_TCON1_Vb_Line_Int_Flag BIT(12)
>>  +#define TCON0_CTL_REG 0x40
>>  +#define 

[Bug 93341] Semi-random GPU lockups on radeonsi with a RadeonHD 7770 (when playing videos, running OpenGL games, WebGL apps, or after extended periods of time)

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=93341

--- Comment #16 from Jean-François Fortin Tam  ---
Created attachment 128278
  --> https://bugs.freedesktop.org/attachment.cgi?id=128278=edit
journalctl output at the time of a deadlock on F25

> Do you get the same dmesg errors?
> I have Wayland locking up randomly,
> but dmesg stays clean and I can ssh into the machine and reboot.

Pretty much yeah. Attached is the crash I have experienced just now, and the
computer wasn't doing anything other than sitting around on the desktop and
playing music from Rhythmbox... and you can see the usual:


/usr/libexec/gdm-x-session[18145]: radeon: Failed to deallocate virtual address
for buffer:
/usr/libexec/gdm-x-session[18145]: radeon:size  : 20480 bytes
kernel: radeon :02:00.0: ring 3 stalled for more than 10083msec
kernel: radeon :02:00.0: GPU lockup (current fence id 0x002d46ee
last fence id 0x002d4710 on ring 3)
kernel: radeon :02:00.0: failed to get a new IB (-35)
kernel: [drm:radeon_gem_va_ioctl [radeon]] *ERROR* Couldn't update BO_VA (-35)
kernel: radeon :02:00.0: failed to get a new IB (-35)
kernel: radeon :02:00.0: failed to get a new IB (-35)
kernel: [drm:radeon_cs_ioctl [radeon]] *ERROR* Failed to get ib !
kernel: [drm:radeon_gem_va_ioctl [radeon]] *ERROR* Couldn't update BO_VA (-35)
/usr/libexec/gdm-x-session[18145]: radeon:va: 0x1f836000
/usr/libexec/gdm-x-session[18145]: radeon: Failed to deallocate virtual address
for buffer:
/usr/libexec/gdm-x-session[18145]: radeon:size  : 45056 bytes
/usr/libexec/gdm-x-session[18145]: radeon:va: 0x1f4b

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/a1abd358/attachment.html>

[PATCH v5 4/4] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers

[Laurent Pinchart]

Hi Jyri,

On Tuesday 29 Nov 2016 22:18:56 Jyri Sarha wrote:
> On 11/25/16 11:02, Jyri Sarha wrote:
> > -void tilcdc_remove_external_encoders(struct drm_device *dev)
> > +void tilcdc_remove_external_device(struct drm_device *dev)
> >  {
> > struct tilcdc_drm_private *priv = dev->dev_private;
> > -   int i;
> > 
> > /* Restore the original helper functions, if any. */
> > -   for (i = 0; i < priv->num_connectors; i++)
> > -   if (IS_ERR(priv->connector_funcs[i]))
> > -   drm_connector_helper_add(priv->connectors[i], NULL);
> > -   else if (priv->connector_funcs[i])
> > -   drm_connector_helper_add(priv->connectors[i],
> > -priv->connector_funcs[i]);
> > +   if (IS_ERR(priv->connector_funcs))
> > +   drm_connector_helper_add(priv->external_connector, NULL);
> > +   else if (priv->connector_funcs)
> > +   drm_connector_helper_add(priv->external_connector,
> > +priv->connector_funcs);
> 
> I still need to add:
> +
> +   if (priv->external_encoder && priv->external_encoder->bridge)
> +   drm_bridge_detach(priv->external_encoder->bridge);

"drm: bridge: Detach bridge from encoder at encoder cleanup time" :-)

> > +}
> 
> ... and reorder the tilcdc_fini() a bit. I need to put
> tilcdc_remove_external_device() before drm_mode_config_cleanup()
> for drm_bridge_detach() to be called before whole mode config is teared
> apart.
> 
> There is also a bug in tearing down drm debugfs that had to be fixed
> before I got module unload and reload to work properly. I'll send a
> patch shortly.

-- 
Regards,

Laurent Pinchart

[PATCH v5 3/4] drm/bridge: Add ti-tfp410 DVI transmitter driver

[Laurent Pinchart]

Hi Jyri,

On Tuesday 29 Nov 2016 22:18:25 Jyri Sarha wrote:
> The module unload should not be allowed while the bridge is attached. So
> still need to add these:
> 
> On 11/25/16 11:02, Jyri Sarha wrote:
> > +
> > +static int tfp410_attach(struct drm_bridge *bridge)
> > +{
> > +   struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
> > +   int ret;
> > +
> 
> +   if (!try_module_get(THIS_MODULE)) {
> +   dev_err(dvi->dev, "Module unloading\n");
> +   return -ENODEV;
> +   }
> +

Shouldn't this be done in core code ?

> > +   if (!bridge->encoder) {
> > +   dev_err(dvi->dev, "Missing encoder\n");
> > +   return -ENODEV;
> > +   }
> > +
> > +   drm_connector_helper_add(>connector,
> > +_con_helper_funcs);
> > +   ret = drm_connector_init(bridge->dev, >connector,
> > +_con_funcs, DRM_MODE_CONNECTOR_HDMIA);
> > +   if (ret) {
> > +   dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   drm_mode_connector_attach_encoder(>connector,
> > + bridge->encoder);
> > +
> > +   return 0;
> > +}
> 
> +static void tfp410_detach(struct drm_bridge *bridge)
> +{
> +   module_put(THIS_MODULE);
> +}
> +
> 
> > +
> > +static const struct drm_bridge_funcs tfp410_bridge_funcs = {
> > +   .attach = tfp410_attach,
> 
> +   .detach = tfp410_detach,
> 
> > +};
> > +

-- 
Regards,

Laurent Pinchart

[PATCH v2 2/3] drm: Pull together probe + setup for drm_fb_helper

[Chris Wilson]

On Tue, Nov 29, 2016 at 09:29:06PM +0100, Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 12:02:16PM +, Chris Wilson wrote:
> > drm_fb_helper_probe_connector_modes() is always called before
> > drm_setup_crtcs(), so just move the call into drm_setup_crtcs for a
> > small bit of code compaction.
> > 
> > Signed-off-by: Chris Wilson 
> > Reviewed-by: Daniel Vetter 
> 
> rb not entirely correct, since it's missing the crucial note I asked for
> to understand things:
> 
> "Note that register_framebuffer will do a modeset (when fbcon is enabled)
> and hence must be moved out of the critical section. A follow-up patch
> will add new locking for the fb list, hence move all the related
> registration code together."

Which is in patch 1 where the move was required.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm/atomic: debugfs atomic state should be removed too

[Jyri Sarha]

If atomic state file is not removed from debugfs it will prevent
atomic driver modules from reloading after unload when debugfs is
enabled.

Fixes commit 6559c901cb484 ("drm/atomic: add debugfs file to dump out
atomic state")

Signed-off-by: Jyri Sarha 
---
 drivers/gpu/drm/drm_atomic.c  | 6 ++
 drivers/gpu/drm/drm_debugfs.c | 2 ++
 include/drm/drm_atomic.h  | 1 +
 3 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index b476ec5..0db642a 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1686,6 +1686,12 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
ARRAY_SIZE(drm_atomic_debugfs_list),
minor->debugfs_root, minor);
 }
+
+void drm_atomic_debugfs_fini(struct drm_minor *minor)
+{
+   drm_debugfs_remove_files(drm_atomic_debugfs_list,
+   ARRAY_SIZE(drm_atomic_debugfs_list), minor);
+}
 #endif

 /*
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 206a4fe..68be42c 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -237,6 +237,8 @@ int drm_debugfs_cleanup(struct drm_minor *minor)

drm_debugfs_remove_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES, minor);

+   drm_atomic_debugfs_fini(minor);
+
debugfs_remove(minor->debugfs_root);
minor->debugfs_root = NULL;

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index c0eaec7..c8cec34 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -372,6 +372,7 @@ void drm_atomic_set_fence_for_plane(struct drm_plane_state 
*plane_state,
 #ifdef CONFIG_DEBUG_FS
 struct drm_minor;
 int drm_atomic_debugfs_init(struct drm_minor *minor);
+void drm_atomic_debugfs_fini(struct drm_minor *minor);
 #endif

 #define for_each_connector_in_state(__state, connector, connector_state, __i) \
-- 
1.9.1

[PATCH v5 4/4] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers

[Jyri Sarha]

On 11/25/16 11:02, Jyri Sarha wrote:
> -void tilcdc_remove_external_encoders(struct drm_device *dev)
> +void tilcdc_remove_external_device(struct drm_device *dev)
>  {
>   struct tilcdc_drm_private *priv = dev->dev_private;
> - int i;
>  
>   /* Restore the original helper functions, if any. */
> - for (i = 0; i < priv->num_connectors; i++)
> - if (IS_ERR(priv->connector_funcs[i]))
> - drm_connector_helper_add(priv->connectors[i], NULL);
> - else if (priv->connector_funcs[i])
> - drm_connector_helper_add(priv->connectors[i],
> -  priv->connector_funcs[i]);
> + if (IS_ERR(priv->connector_funcs))
> + drm_connector_helper_add(priv->external_connector, NULL);
> + else if (priv->connector_funcs)
> + drm_connector_helper_add(priv->external_connector,
> +  priv->connector_funcs);


I still need to add:
+
+   if (priv->external_encoder && priv->external_encoder->bridge)
+   drm_bridge_detach(priv->external_encoder->bridge);

> +}

... and reorder the tilcdc_fini() a bit. I need to put
tilcdc_remove_external_device() before drm_mode_config_cleanup()
for drm_bridge_detach() to be called before whole mode config is teared
apart.

There is also a bug in tearing down drm debugfs that had to be fixed
before I got module unload and reload to work properly. I'll send a
patch shortly.

[PATCH v5 3/4] drm/bridge: Add ti-tfp410 DVI transmitter driver

[Jyri Sarha]

The module unload should not be allowed while the bridge is attached. So
still need to add these:

On 11/25/16 11:02, Jyri Sarha wrote:
> +
> +static int tfp410_attach(struct drm_bridge *bridge)
> +{
> + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
> + int ret;
> +


+   if (!try_module_get(THIS_MODULE)) {
+   dev_err(dvi->dev, "Module unloading\n");
+   return -ENODEV;
+   }
+


> + if (!bridge->encoder) {
> + dev_err(dvi->dev, "Missing encoder\n");
> + return -ENODEV;
> + }
> +
> + drm_connector_helper_add(>connector,
> +  _con_helper_funcs);
> + ret = drm_connector_init(bridge->dev, >connector,
> +  _con_funcs, DRM_MODE_CONNECTOR_HDMIA);
> + if (ret) {
> + dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret);
> + return ret;
> + }
> +
> + drm_mode_connector_attach_encoder(>connector,
> +   bridge->encoder);
> +
> + return 0;
> +}

+static void tfp410_detach(struct drm_bridge *bridge)
+{
+   module_put(THIS_MODULE);
+}
+

> +
> +static const struct drm_bridge_funcs tfp410_bridge_funcs = {
> + .attach = tfp410_attach,

+   .detach = tfp410_detach,

> +};
> +

[Bug 98907] commit bf75ef3 causes Xorg lock-up

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98907

--- Comment #1 from Hleb Valoshka <375gnu at gmail.com> ---
My X.org is 1.19.0, my kernel and X11 drivers tested are: 4.9-rc4 +
modesetting/amdgpu 1.2.0, 4.9-rc7 + amdgpu 1.2.0. My videocard is HD7750. Dmesg
and Xorg.0.log are clean.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/0bd3f966/attachment.html>

[PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI

[Laurent Pinchart]

Hi Jean-François,

On Tuesday 29 Nov 2016 21:04:55 Jean-Francois Moine wrote:
> On Tue, 29 Nov 2016 21:33 +0200 Laurent Pinchart wrote:
> >>> You need a third port for the HDMI encoder output, connected to an
> >>> HDMI connector DT node.
> >> 
> >> I don't see what you mean. The HDMI device is both the encoder
> >> and connector (as the TDA998x):
> >
> > The driver might create both an encoder and a connector, but I very much
> > doubt that the "allwinner,sun8i-a83t-hdmi" hardware contains a connector,
> > unless the SoC package has an HDMI connector coming out of it :-)
> > 
> >> plane -> DE2 mixer ---> TCON -> HDMI -> display device
> >> - plane --- CRTC -   - encoder  \
> >>connector -- (HDMI cable)
> >>  audio-controller -   - audio-codec /
> 
> The schema is the same as the Dove Cubox: the TDA998x is just a chip
> with some wires going out and the physical connector is supposed to be
> at the end of the wires.

I've missed the Dove Cubox DT bindings when they were submitted. Fortunately 
(or unfortunately for you, depending on how you look at it ;-)) I've paid more 
attention this time.

> Here, the HDMI pins of the SoC go to a pure hardware chip and then to
> the physical connector. Which software entity do you want to add?

I don't want to add a software entity, I just want to model the connector in 
DT as it's present in the system. Even though that's more common for other bus 
types than HDMI (LVDS for instance) it wouldn't be inconceivable to connect 
the HDMI signals to an on-board chim instead of an HDMI connector, so the HDMI 
encoder output should be modelled by a port and connected to a connector DT 
node in this case.

-- 
Regards,

Laurent Pinchart

[Bug 98907] commit bf75ef3 causes Xorg lock-up

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98907

Bug ID: 98907
   Summary: commit bf75ef3 causes Xorg lock-up
   Product: Mesa
   Version: git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: critical
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: 375gnu at gmail.com
QA Contact: dri-devel at lists.freedesktop.org

I had Mesa snapshot from 2016/11/09 running fine but with one from 2016/11/25 I
have Xorg lock-up.

 5248 tty7 Dsl+   0:06 /usr/lib/xorg/Xorg -nolisten tcp -auth
/var/run/slim.auth vt07

All other processes run fine, I can connect pc using ssh, but Xorg does not
react to keyboard (C+A+F1, C+A+Bs), so the only way to restore system is
REISUB, even after reboot command the system doesn't reboot, the screen is
blanked so I don't see what's going with it.

I tested this issue only with Black Mesa game from steam.

I can provide the last 20 lines of strace -p <Xorg's pid>, but they look
useless.

After git bisect and patch -R I've found a commit causing this issue:

bf75ef3f9201e11bb08a4d03dab20d5ff86f1ebc

Author: Marek Olšák   2016-11-15 23:15:55
Committer: Marek Olšák   2016-11-21 23:44:35
Parent: ef6c84b301ce15022d4907dfb0db5764e31e68f5 (radeonsi: eliminate VS
outputs that aren't used by PS at run
time)
Child:  c5a654786b5b68c5c215e3bd1bc83b02d7e2a0e9 (swr: [rasterizer memory]
minify original sizes for block for
mats)
Branches: debian, master, remotes/origin/master
Follows: 13.0-branchpoint
Precedes: 

radeonsi: remove all varyings for depth-only rendering or rasterization off

Tested-by: Edmondo Tommasina 
Reviewed-by: Nicolai Hähnle 

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/d25c6251/attachment.html>

[PATCH] drm: Make the connector .detect() callback optional

[Maxime Ripard]

On Tue, Nov 29, 2016 at 10:56:30PM +0200, Laurent Pinchart wrote:
> Many drivers (21 to be exact) create connectors that are always
> connected (for instance to an LVDS or DSI panel). Instead of forcing
> them to implement a dummy .detect() handler, make the callback optional
> and consider the connector as always connected in that case.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c |  7 ---
>  drivers/gpu/drm/arc/arcpgu_sim.c |  7 ---
>  drivers/gpu/drm/ast/ast_mode.c   |  7 ---
>  drivers/gpu/drm/bochs/bochs_kms.c|  7 ---
>  drivers/gpu/drm/bridge/nxp-ptn3460.c |  7 ---
>  drivers/gpu/drm/bridge/parade-ps8622.c   |  7 ---
>  drivers/gpu/drm/bridge/tc358767.c|  7 ---
>  drivers/gpu/drm/cirrus/cirrus_mode.c |  7 ---
>  drivers/gpu/drm/drm_probe_helper.c   | 14 +++---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c|  7 ---
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c  | 14 --
>  drivers/gpu/drm/gma500/psb_intel_lvds.c  | 14 --
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c |  7 ---
>  drivers/gpu/drm/i915/intel_dsi.c |  7 ---
>  drivers/gpu/drm/imx/imx-ldb.c|  7 ---
>  drivers/gpu/drm/imx/imx-tve.c|  7 ---
>  drivers/gpu/drm/imx/parallel-display.c   |  7 ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c   |  7 ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c   |  7 ---
>  drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c|  7 ---
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c   |  7 ---
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c|  7 ---
>  drivers/gpu/drm/sti/sti_hda.c|  7 ---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c|  7 ---
>  drivers/gpu/drm/sun4i/sun4i_tv.c |  7 ---
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c|  8 
>  include/drm/drm_connector.h  |  3 +++
>  27 files changed, 14 insertions(+), 193 deletions(-)

For the sunxi part,
Acked-by: Maxime Ripard 

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/87359257/attachment.sig>

[Bug 97137] R7 M370 hangs with radeon.dpm=1 when running demanding graphical programs with DRI PRIME

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=97137

Mauro Santos  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|NEW |RESOLVED

--- Comment #8 from Mauro Santos  ---
This is now working for me.

The fix was introduced sometime between stable kernel 4.8.6 and 4.8.7 and is
still working fine with 4.8.11.

I haven't tried to find exactly which commit fixed things but given that it's
working now I'll let it rest. If later there is a regression I now have a good
starting point for a bisect and new bug report.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/9bfebd4a/attachment.html>

[drm-intel:drm-intel-next-queued 1/3] drivers/gpu/drm/i915/./i915_trace.h:409:24: error: 'struct i915_address_space' has no member named 'dev'

[kbuild test robot]

~
   drivers/gpu/drm/i915/./i915_trace.h: In function 'perf_trace_i915_ppgtt':
   drivers/gpu/drm/i915/./i915_trace.h:714:21: error: 'struct 
i915_address_space' has no member named 'dev'
   __entry->dev = vm->dev->primary->index;
^
   include/trace/perf.h:65:4: note: in definition of macro 'DECLARE_EVENT_CLASS'
 { assign; }   \
   ^~
   drivers/gpu/drm/i915/./i915_trace.h:712:2: note: in expansion of macro 
'TP_fast_assign'
 TP_fast_assign(
 ^~

vim +409 drivers/gpu/drm/i915/./i915_trace.h

1c5d22f7 Chris Wilson 2009-08-25  390  
903cf20c Li Zefan 2010-03-11  391  DEFINE_EVENT(i915_gem_object, 
i915_gem_object_destroy,
05394f39 Chris Wilson 2010-11-08  392   TP_PROTO(struct drm_i915_gem_object 
*obj),
903cf20c Li Zefan 2010-03-11  393   TP_ARGS(obj)
1c5d22f7 Chris Wilson 2009-08-25  394  );
1c5d22f7 Chris Wilson 2009-08-25  395  
db53a302 Chris Wilson 2011-02-03 @396  TRACE_EVENT(i915_gem_evict,
e522ac23 Chris Wilson 2016-08-04  397   TP_PROTO(struct i915_address_space 
*vm, u32 size, u32 align, unsigned int flags),
e522ac23 Chris Wilson 2016-08-04  398   TP_ARGS(vm, size, align, flags),
1c5d22f7 Chris Wilson 2009-08-25  399  
db53a302 Chris Wilson 2011-02-03  400   TP_STRUCT__entry(
db53a302 Chris Wilson 2011-02-03  401__field(u32, dev)
e522ac23 Chris Wilson 2016-08-04  402__field(struct 
i915_address_space *, vm)
db53a302 Chris Wilson 2011-02-03  403__field(u32, size)
db53a302 Chris Wilson 2011-02-03  404__field(u32, align)
e522ac23 Chris Wilson 2016-08-04  405__field(unsigned 
int, flags)
db53a302 Chris Wilson 2011-02-03  406   ),
1c5d22f7 Chris Wilson 2009-08-25  407  
db53a302 Chris Wilson 2011-02-03 @408   TP_fast_assign(
e522ac23 Chris Wilson 2016-08-04 @409  __entry->dev = 
vm->dev->primary->index;
e522ac23 Chris Wilson 2016-08-04  410  __entry->vm = vm;
db53a302 Chris Wilson 2011-02-03  411  __entry->size = size;
db53a302 Chris Wilson 2011-02-03  412  __entry->align = 
align;

:: The code at line 409 was first introduced by commit
:: e522ac2324f384e1fafd1a4ae6ebf38095dc6695 drm/i915: Remove surplus 
drm_device parameter to i915_gem_evict_something()

:: TO: Chris Wilson 
:: CC: Chris Wilson 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
-- next part --
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 25157 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/366f926a/attachment-0001.gz>

[PATCH] drm/atomic: debugfs atomic state should be removed too

[Daniel Vetter]

On Tue, Nov 29, 2016 at 10:19:10PM +0200, Jyri Sarha wrote:
> If atomic state file is not removed from debugfs it will prevent
> atomic driver modules from reloading after unload when debugfs is
> enabled.
> 
> Fixes commit 6559c901cb484 ("drm/atomic: add debugfs file to dump out
> atomic state")
> 
> Signed-off-by: Jyri Sarha 

We have one already:

commit 8c0b55e22aff84cb6938a993d86c3ce02006236e
Author: Liviu Dudau 
Date:   Thu Nov 17 11:41:29 2016 +

drm/atomic: cleanup debugfs entries on un-registering the driver.

Cheers, Daniel

> ---
>  drivers/gpu/drm/drm_atomic.c  | 6 ++
>  drivers/gpu/drm/drm_debugfs.c | 2 ++
>  include/drm/drm_atomic.h  | 1 +
>  3 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index b476ec5..0db642a 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1686,6 +1686,12 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
>   ARRAY_SIZE(drm_atomic_debugfs_list),
>   minor->debugfs_root, minor);
>  }
> +
> +void drm_atomic_debugfs_fini(struct drm_minor *minor)
> +{
> + drm_debugfs_remove_files(drm_atomic_debugfs_list,
> + ARRAY_SIZE(drm_atomic_debugfs_list), minor);
> +}
>  #endif
>  
>  /*
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 206a4fe..68be42c 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -237,6 +237,8 @@ int drm_debugfs_cleanup(struct drm_minor *minor)
>  
>   drm_debugfs_remove_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES, minor);
>  
> + drm_atomic_debugfs_fini(minor);
> +
>   debugfs_remove(minor->debugfs_root);
>   minor->debugfs_root = NULL;
>  
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index c0eaec7..c8cec34 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -372,6 +372,7 @@ void drm_atomic_set_fence_for_plane(struct 
> drm_plane_state *plane_state,
>  #ifdef CONFIG_DEBUG_FS
>  struct drm_minor;
>  int drm_atomic_debugfs_init(struct drm_minor *minor);
> +void drm_atomic_debugfs_fini(struct drm_minor *minor);
>  #endif
>  
>  #define for_each_connector_in_state(__state, connector, connector_state, 
> __i) \
> -- 
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v3 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings

[Laurent Pinchart]

Hi Mark,

On Tuesday 29 Nov 2016 11:01:25 Mark Brown wrote:
> On Tue, Nov 29, 2016 at 11:11:03AM +0200, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 13:41:33 Archit Taneja wrote:
> >> I thought we couldn't add mandatory properties once the device is
> >> already present in DT for one or more platforms.
> > 
> > You can, as long as you treat them as optional in the driver to retain
> > backward compatibility. The DT bindings should document the properties
> > expected from a new platform (older versions of the bindings will always
> > be available in the git history).
> 
> The device probably never worked without power...  note that the kernel
> will substitute in dummy regulators for anything that isn't explicitly
> mapped so it won't actually break anything.
> 
> >> Say, if we do make it mandatory for future additions, we would need to
> >> have DT property for the supplies for the new platforms. If the
> >> regulators on these boards are fixed supplies, they would be need to be
> >> modeled using "regulator-fixed", possibly without any input supply. Is
> >> that what you're suggesting?
> > 
> > That's the idea, yes. Clock maintainers have a similar opinion regarding
> > the clock bindings, where a clock that is not optional at the hardware
> > level should be specified in DT even if it's always present.
> > 
> > Mark, any opinion ?
> 
> It's best practice to always describe the power.  The kernel will cope
> if people don't but it's not unknown for drivers to discover a reason
> for wanting information about their power and hard to retrofit that if
> it's not been in there from the get go.

Sounds good to me, thanks.

> Please note that if you're going to CC me into a graphics thread there's
> a good chance I will miss it, I get copied on quite a lot of graphics
> related mail that's not really relevant so I often skip it.  Changing
> the subject line would help with that.

I try to add a (CC'ing xxx) at the beginning of the e-mail to draw attention 
when a question is targetted at a particular person. If that's not enough I 
can change the subject, but might forget to do so from time to time. Or you 
could whitelist me, unless I'm already blacklisted ;-)

-- 
Regards,

Laurent Pinchart

[PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI

[Laurent Pinchart]

Hi Jean-François,

On Tuesday 29 Nov 2016 20:27:51 Jean-Francois Moine wrote:
> On Tue, 29 Nov 2016 20:46:22 +0200 Laurent Pinchart wrote:
> [snip]
> 
> >> +Example:
> >> +
> >> +  hdmi: hdmi at 01ee {
> >> +  compatible = "allwinner,sun8i-a83t-hdmi";
> >> +  reg = <0x01ee 0x2>;
> >> +  clocks = < CLK_BUS_HDMI>, < CLK_HDMI>,
> >> +   < CLK_HDMI_DDC>;
> >> +  clock-names = "bus", "clock", "ddc-clock";
> >> +  resets = < RST_HDMI0>, < RST_HDMI1>;
> >> +  reset-names = "hdmi0", "hdmi1";
> >> +  pinctrl-names = "default";
> >> +  pinctrl-0 = <_pins_a>;
> >> +  status = "disabled";
> >> +  #address-cells = <1>;
> >> +  #size-cells = <0>;
> >> +  port at 0 { /* video */
> >> +  reg = <0>;
> >> +  hdmi_tcon1: endpoint {
> >> +  remote-endpoint = <_hdmi>;
> >> +  };
> >> +  };
> >> +  port at 1 { /* audio */
> >> +  reg = <1>;
> >> +  hdmi_i2s2: endpoint {
> >> +  remote-endpoint = <_hdmi>;
> >> +  };
> >> +  };
> > 
> > You need a third port for the HDMI encoder output, connected to an HDMI
> > connector DT node.
> 
> I don't see what you mean. The HDMI device is both the encoder
> and connector (as the TDA998x):

The driver might create both an encoder and a connector, but I very much doubt 
that the "allwinner,sun8i-a83t-hdmi" hardware contains a connector, unless the 
SoC package has an HDMI connector coming out of it :-)

> plane -> DE2 mixer ---> TCON -> HDMI -> display device
> - plane --- CRTC -   - encoder  \
>connector -- (HDMI cable)
>  audio-controller -   - audio-codec /
> 
> > > + };

-- 
Regards,

Laurent Pinchart

[PATCH v2 2/3] drm: Pull together probe + setup for drm_fb_helper

[Daniel Vetter]

On Tue, Nov 29, 2016 at 12:02:16PM +, Chris Wilson wrote:
> drm_fb_helper_probe_connector_modes() is always called before
> drm_setup_crtcs(), so just move the call into drm_setup_crtcs for a
> small bit of code compaction.
> 
> Signed-off-by: Chris Wilson 
> Reviewed-by: Daniel Vetter 

rb not entirely correct, since it's missing the crucial note I asked for
to understand things:

"Note that register_framebuffer will do a modeset (when fbcon is enabled)
and hence must be moved out of the critical section. A follow-up patch
will add new locking for the fb list, hence move all the related
registration code together."

Sean, since you reviewed all, can you pls add this note to the commit
message when merging?

Thanks, Daniel

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 37 +++--
>  1 file changed, 11 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 380a0ec01033..90da28d2fcf3 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2099,20 +2099,19 @@ static int drm_pick_crtcs(struct drm_fb_helper 
> *fb_helper,
>   return best_score;
>  }
>  
> -static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
> +static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> + u32 width, u32 height)
>  {
>   struct drm_device *dev = fb_helper->dev;
>   struct drm_fb_helper_crtc **crtcs;
>   struct drm_display_mode **modes;
>   struct drm_fb_offset *offsets;
>   bool *enabled;
> - int width, height;
>   int i;
>  
>   DRM_DEBUG_KMS("\n");
> -
> - width = dev->mode_config.max_width;
> - height = dev->mode_config.max_height;
> + if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
> + DRM_DEBUG_KMS("No connectors reported connected with modes\n");
>  
>   /* prevent concurrent modification of connector_count by hotplug */
>   lockdep_assert_held(_helper->dev->mode_config.mutex);
> @@ -2236,23 +2235,15 @@ int drm_fb_helper_initial_config(struct drm_fb_helper 
> *fb_helper, int bpp_sel)
>  {
>   struct drm_device *dev = fb_helper->dev;
>   struct fb_info *info;
> - int count = 0;
>   int ret;
>  
>   if (!drm_fbdev_emulation)
>   return 0;
>  
>   mutex_lock(>mode_config.mutex);
> - count = drm_fb_helper_probe_connector_modes(fb_helper,
> - dev->mode_config.max_width,
> - 
> dev->mode_config.max_height);
> - /*
> -  * we shouldn't end up with no modes here.
> -  */
> - if (count == 0)
> - dev_info(fb_helper->dev->dev, "No connectors reported connected 
> with modes\n");
> -
> - drm_setup_crtcs(fb_helper);
> + drm_setup_crtcs(fb_helper,
> + dev->mode_config.max_width,
> + dev->mode_config.max_height);
>   ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
>   mutex_unlock(>mode_config.mutex);
>   if (ret)
> @@ -2300,28 +2291,22 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
>  int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>  {
>   struct drm_device *dev = fb_helper->dev;
> - u32 max_width, max_height;
>  
>   if (!drm_fbdev_emulation)
>   return 0;
>  
> - mutex_lock(_helper->dev->mode_config.mutex);
> + mutex_lock(>mode_config.mutex);
>   if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
>   fb_helper->delayed_hotplug = true;
> - mutex_unlock(_helper->dev->mode_config.mutex);
> + mutex_unlock(>mode_config.mutex);
>   return 0;
>   }
>   DRM_DEBUG_KMS("\n");
>  
> - max_width = fb_helper->fb->width;
> - max_height = fb_helper->fb->height;
> + drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
>  
> - drm_fb_helper_probe_connector_modes(fb_helper, max_width, max_height);
> - mutex_unlock(_helper->dev->mode_config.mutex);
> + mutex_unlock(>mode_config.mutex);
>  
> - drm_modeset_lock_all(dev);
> - drm_setup_crtcs(fb_helper);
> - drm_modeset_unlock_all(dev);
>   drm_fb_helper_set_par(fb_helper->fbdev);
>  
>   return 0;
> -- 
> 2.10.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v3 09/13] drm: Add encoder_type field to the drm_bridge structure

[Daniel Vetter]

On Tue, Nov 29, 2016 at 07:49:22PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
> 
> On Tuesday 29 Nov 2016 11:27:20 Daniel Vetter wrote:
> > On Tue, Nov 29, 2016 at 11:58:44AM +0200, Laurent Pinchart wrote:
> > > On Tuesday 29 Nov 2016 10:56:53 Daniel Vetter wrote:
> > >> On Tue, Nov 29, 2016 at 11:04:39AM +0200, Laurent Pinchart wrote:
> > >>> The drm_bridge object models on- or off-chip hardware encoders and
> > >>> provide an abstract control API to display drivers. In order to help
> > >>> display drivers creating the right kind of drm_encoder object, expose
> > >>> the type of the hardware encoder associated with each bridge.
> > >>> 
> > >>> Signed-off-by: Laurent Pinchart
> > >>> 
> > >> 
> > >> DRM_MODE_ENCODER_BRIDGE. Problem solved, because in reality no one cares
> > >> one iota about the encoder type.
> > > 
> > > It's exposed to userspace though, are you 100% sure we won't break
> > > anything ?
> >
> > We've added DP, DSI, DPMST and DPI encoder types thus far, no one
> > screamed.
> 
> In that case why don't we go one step further and remove the encoder type 
> completely ? We can't remove the field from the API, but we can hardcode it 
> to 
> a single value.
> 
> There are however drivers that rely on the encoder type (radeon, nouveau, 
> sti, 
> amdgpu, msm and rcar-du, but I'll fix the last one) so we'd need to address 
> that first. If we don't want to remove the encoder_type field from in-kernel 
> structures and let drivers use it, then I don't think DRM_MODE_ENCODER_BRIDGE 
> would be a good option, we should report the real type instead.

If you strongly believe that I will not stop you. This was just a
suggestion to get all your stuff landed with minimal amounts of effort and
across-the-subsystem cleanup needed. I'd do it that ;-)

And if you don't like DRM_MODE_ENCODER_BRIDGE you could also pick
DRM_MODE_ENCODER_NONE, which is what most seem to do today. In the end it
doesn't matter no matter which option you pick. The only difference is in
the amount of effort you need to spend to get it merged ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH] drm: Make the connector .detect() callback optional

[Deucher, Alexander]

> -Original Message-
> From: Laurent Pinchart
> [mailto:laurent.pinchart+renesas at ideasonboard.com]
> Sent: Tuesday, November 29, 2016 3:57 PM
> To: dri-devel at lists.freedesktop.org
> Cc: Deucher, Alexander; Koenig, Christian; Alexey Brodkin; Stefan Agner;
> Alison Wang; Patrik Jakobsson; Xinliang Liu; Rongrong Zou; Daniel Vetter;
> Philipp Zabel; CK Hu; Mark Yao; Benjamin Gaignard; Vincent Abriou; Maxime
> Ripard; Jyri Sarha
> Subject: [PATCH] drm: Make the connector .detect() callback optional
> 
> Many drivers (21 to be exact) create connectors that are always
> connected (for instance to an LVDS or DSI panel). Instead of forcing
> them to implement a dummy .detect() handler, make the callback optional
> and consider the connector as always connected in that case.
> 
> Signed-off-by: Laurent Pinchart
> 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c |  7 ---
>  drivers/gpu/drm/arc/arcpgu_sim.c |  7 ---
>  drivers/gpu/drm/ast/ast_mode.c   |  7 ---
>  drivers/gpu/drm/bochs/bochs_kms.c|  7 ---
>  drivers/gpu/drm/bridge/nxp-ptn3460.c |  7 ---
>  drivers/gpu/drm/bridge/parade-ps8622.c   |  7 ---
>  drivers/gpu/drm/bridge/tc358767.c|  7 ---
>  drivers/gpu/drm/cirrus/cirrus_mode.c |  7 ---
>  drivers/gpu/drm/drm_probe_helper.c   | 14 +++---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c|  7 ---
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c  | 14 --
>  drivers/gpu/drm/gma500/psb_intel_lvds.c  | 14 --
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c |  7 ---
>  drivers/gpu/drm/i915/intel_dsi.c |  7 ---
>  drivers/gpu/drm/imx/imx-ldb.c|  7 ---
>  drivers/gpu/drm/imx/imx-tve.c|  7 ---
>  drivers/gpu/drm/imx/parallel-display.c   |  7 ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c   |  7 ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c   |  7 ---
>  drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c|  7 ---
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c   |  7 ---
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c|  7 ---
>  drivers/gpu/drm/sti/sti_hda.c|  7 ---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c|  7 ---
>  drivers/gpu/drm/sun4i/sun4i_tv.c |  7 ---
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c|  8 
>  include/drm/drm_connector.h  |  3 +++
>  27 files changed, 14 insertions(+), 193 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index 81cbf0b05dff..1d93e123532d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -429,12 +429,6 @@ dce_virtual_dpms(struct drm_connector
> *connector, int mode)
>   return 0;
>  }
> 
> -static enum drm_connector_status
> -dce_virtual_detect(struct drm_connector *connector, bool force)
> -{
> - return connector_status_connected;
> -}
> -
>  static int
>  dce_virtual_set_property(struct drm_connector *connector,
>struct drm_property *property,
> @@ -463,7 +457,6 @@ static const struct drm_connector_helper_funcs
> dce_virtual_connector_helper_func
> 
>  static const struct drm_connector_funcs dce_virtual_connector_funcs = {
>   .dpms = dce_virtual_dpms,
> - .detect = dce_virtual_detect,
>   .fill_modes = drm_helper_probe_single_connector_modes,
>   .set_property = dce_virtual_set_property,
>   .destroy = dce_virtual_destroy,
> diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c
> b/drivers/gpu/drm/arc/arcpgu_sim.c
> index 2bf06d71556a..bca3a678c955 100644
> --- a/drivers/gpu/drm/arc/arcpgu_sim.c
> +++ b/drivers/gpu/drm/arc/arcpgu_sim.c
> @@ -41,12 +41,6 @@ static int arcpgu_drm_connector_get_modes(struct
> drm_connector *connector)
>   return count;
>  }
> 
> -static enum drm_connector_status
> -arcpgu_drm_connector_detect(struct drm_connector *connector, bool
> force)
> -{
> - return connector_status_connected;
> -}
> -
>  static void arcpgu_drm_connector_destroy(struct drm_connector
> *connector)
>  {
>   drm_connector_unregister(connector);
> @@ -61,7 +55,6 @@ arcpgu_drm_connector_helper_funcs = {
>  static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
>   .dpms = drm_helper_connector_dpms,
>   .reset = drm_atomic_helper_connector_reset,
> - .detect = arcpgu_drm_connector_detect,
>   .fill_modes = drm_helper_probe_single_connector_modes,
>   .destroy = arcpgu_drm_connector_destroy,
>   .atomic_duplicate_state =
> drm_atomic_helper_connector_duplicate_state,
> diff --git a/drivers/gpu/drm/ast/ast_mode.c
> b/drivers/gpu/drm/ast/ast_mode.c
> index 

[PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

[Daniel Vetter]

On Tue, Nov 29, 2016 at 08:56:44PM +0200, Laurent Pinchart wrote:
> Hi Archit,
> 
> On Tuesday 29 Nov 2016 16:04:08 Archit Taneja wrote:
> > On 11/29/2016 02:34 PM, Laurent Pinchart wrote:
> > > Most drivers that use bridges forgot to detach them at cleanup time.
> > > Instead of fixing them one by one, detach the bridge in the core
> > > drm_encoder_cleanup() function.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > 
> > > ---
> > > 
> > >  drivers/gpu/drm/drm_encoder.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > > index 5c067719164d..9c1f99646e0d 100644
> > > --- a/drivers/gpu/drm/drm_encoder.c
> > > +++ b/drivers/gpu/drm/drm_encoder.c
> > > @@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
> > >* the indices on the drm_encoder after us in the encoder_list.
> > >*/
> > > + if (encoder->bridge)
> > > + drm_bridge_detach(encoder->bridge);
> > 
> > This would require the kms driver to still detach the remaining
> > n - 1 bridges in a possible chain. We could probably detach all of
> > them here, or maybe leave detaching of all to the kms driver, and just
> > report a warning here.
> 
> I'd prefer detaching them all here, but that's a bit intrusive and should be 
> tested correctly. The patch series is already growing big, could we do that 
> in 
> a separate patch ?

I think you're bridge-for-panels driver is the first one that's actually
getting chained. I guess you do have to fix this here ;-) And if you go
through with making drm_bridge_detach be a purely drm.ko internal thing,
we can make it recursive like all the other functions. Problem solved.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 2/4] ARM64: dts: meson-gx: Add Graphic Controller nodes

[Laurent Pinchart]

Hi Neil,

On Tuesday 29 Nov 2016 21:16:17 Laurent Pinchart wrote:
> On Tuesday 29 Nov 2016 11:47:47 Neil Armstrong wrote:
> > Add Video Processing Unit and CVBS Output nodes, and enable CVBS on
> > selected boards.
> > 
> > Signed-off-by: Neil Armstrong 
> > ---
> > 
> >  arch/arm64/boot/dts/amlogic/meson-gx.dtsi  | 46 +
> >  .../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts|  4 ++
> >  arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi   |  4 ++
> >  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi|  8 
> >  .../boot/dts/amlogic/meson-gxl-nexbox-a95x.dts |  4 ++
> >  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi |  8 
> >  .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts |  4 ++
> >  arch/arm64/boot/dts/amlogic/meson-gxm.dtsi |  8 
> >  8 files changed, 86 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> > b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi index fc033c0..644d5f6 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> > @@ -153,6 +153,27 @@
> > };
> > };
> > 
> > +   venc_cvbs: venc-cvbs {
> > +   compatible = "amlogic,meson-gx-cvbs";
> > +   status = "disabled";
> 
> Still no registers here ?

Or is this the internal video encoder, not the HHI VDAC ? If so, haven't we 
decided when discussing the previous version that there's no need for a DT 
node to model the video encoder as it's part of the VPU ?

> > +
> > +   ports {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   venc_cvbs_in: port at 0 {
> 
> Nitpicking, you don't need a label here as ports are never referenced by
> phandle. Same for the vpu node below.
> 
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +reg = <0>;
> > +
> > +venc_cvbs_in_vpu: endpoint at 0 {
> > +reg = <0>;
> 
> And there's no requirement to number the endpoint if there's a single one of
> them (but it's not forbidden either).
> 
> > +remote-endpoint =
> 
> <_out_venc_cvbs>;
> 
> > +   };
> > +   };
> > +   };
> > +   };
> > +
> > 
> > soc {

Shouldn't the above node be placed inside the soc ?

> > compatible = "simple-bus";
> > #address-cells = <2>;
> > 
> > @@ -356,5 +377,30 @@
> > 
> > status = "disabled";
> > 
> > };
> > 
> > };
> > 
> > +
> > +   vpu: vpu at d010 {
> > +   compatible = "amlogic,meson-gx-vpu";
> > +   reg = <0x0 0xd010 0x0 0x10>,
> > + <0x0 0xc883c000 0x0 0x1000>,
> > + <0x0 0xc8838000 0x0 0x1000>;
> > +   reg-names = "base", "hhi", "dmc";
> > +   interrupts = ;
> > +
> > +   ports {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   vpu_out: port at 1 {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   reg = <1>;
> > +
> > +   vpu_out_venc_cvbs: endpoint at 0 {
> > +   reg = <0>;
> > +   remote-endpoint =
> 
> <_cvbs_in_vpu>;
> 
> > +   };
> > +   };
> > +   };
> > +   };
> > 
> > };
> >  
> >  };
> > 
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> > b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts index
> > 9696820..a55d1cf 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> > @@ -229,3 +229,7 @@
> > 
> > clocks = < CLKID_FCLK_DIV4>;
> > clock-names = "clkin0";
> >  
> >  };
> > 
> > +
> > +_cvbs {
> > +   status = "okay";
> > +};
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> > b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi index 5e5e2de..3c09bd1
> > 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> > @@ -266,3 +266,7 @@
> > 
> > clocks = < CLKID_FCLK_DIV4>;
> > clock-names = "clkin0";
> >  
> >  };
> > 
> > +
> > +_cvbs {
> > +   status = "okay";
> > +};
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> > b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi index ac5ad3b..1a321c8f
> > 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> > +++ 

[PATCH 2/4] ARM64: dts: meson-gx: Add Graphic Controller nodes

[Laurent Pinchart]

Hi Neil,

Thank you for the patch.

On Tuesday 29 Nov 2016 11:47:47 Neil Armstrong wrote:
> Add Video Processing Unit and CVBS Output nodes, and enable CVBS on selected
> boards.
> 
> Signed-off-by: Neil Armstrong 
> ---
>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi  | 46 +++
>  .../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts|  4 ++
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi   |  4 ++
>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi|  8 
>  .../boot/dts/amlogic/meson-gxl-nexbox-a95x.dts |  4 ++
>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi |  8 
>  .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts |  4 ++
>  arch/arm64/boot/dts/amlogic/meson-gxm.dtsi |  8 
>  8 files changed, 86 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi index fc033c0..644d5f6 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> @@ -153,6 +153,27 @@
>   };
>   };
> 
> + venc_cvbs: venc-cvbs {
> + compatible = "amlogic,meson-gx-cvbs";
> + status = "disabled";

Still no registers here ?

> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + venc_cvbs_in: port at 0 {

Nitpicking, you don't need a label here as ports are never referenced by 
phandle. Same for the vpu node below.

> +  #address-cells = <1>;
> +  #size-cells = <0>;
> +  reg = <0>;
> +
> +  venc_cvbs_in_vpu: endpoint at 0 {
> +  reg = <0>;

And there's no requirement to number the endpoint if there's a single one of 
them (but it's not forbidden either).

> +  remote-endpoint = 
<_out_venc_cvbs>;
> + };
> + };
> + };
> + };
> +
>   soc {
>   compatible = "simple-bus";
>   #address-cells = <2>;
> @@ -356,5 +377,30 @@
>   status = "disabled";
>   };
>   };
> +
> + vpu: vpu at d010 {
> + compatible = "amlogic,meson-gx-vpu";
> + reg = <0x0 0xd010 0x0 0x10>,
> +   <0x0 0xc883c000 0x0 0x1000>,
> +   <0x0 0xc8838000 0x0 0x1000>;
> + reg-names = "base", "hhi", "dmc";
> + interrupts = ;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + vpu_out: port at 1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + vpu_out_venc_cvbs: endpoint at 0 {
> + reg = <0>;
> + remote-endpoint = 
<_cvbs_in_vpu>;
> + };
> + };
> + };
> + };
>   };
>  };
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts index
> 9696820..a55d1cf 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> @@ -229,3 +229,7 @@
>   clocks = < CLKID_FCLK_DIV4>;
>   clock-names = "clkin0";
>  };
> +
> +_cvbs {
> + status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi index 5e5e2de..3c09bd1
> 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> @@ -266,3 +266,7 @@
>   clocks = < CLKID_FCLK_DIV4>;
>   clock-names = "clkin0";
>  };
> +
> +_cvbs {
> + status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi index ac5ad3b..1a321c8f
> 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> @@ -506,3 +506,11 @@
>< CLKID_FCLK_DIV2>;
>   clock-names = "core", "clkin0", "clkin1";
>  };
> +
> +_cvbs {
> + compatible = "amlogic,meson-gxbb-cvbs", "amlogic,meson-gx-cvbs";
> +};
> +
> + {
> + compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts index
> e99101a..2a9b46f 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> 

[PATCH V3 3/3] drm: Add new driver for MXSFB controller

[Daniel Vetter]

On Tue, Nov 29, 2016 at 06:27:30PM +0100, Marek Vasut wrote:
> On 11/14/2016 12:47 PM, Daniel Vetter wrote:
> > On Mon, Nov 14, 2016 at 11:10:36AM +0100, Marek Vasut wrote:
> >> Add new driver for the MXSFB controller found in i.MX23/28/6SX .
> >> The MXSFB controller is a simple framebuffer controller with one
> >> parallel LCD output. Unlike the MXSFB fbdev driver that is used
> >> on these systems now, this driver uses the DRM/KMS framework.
> >>
> >> Signed-off-by: Marek Vasut 
> >> Cc: Lucas Stach 
> >> Cc: Fabio Estevam 
> >> Cc: Shawn Guo 
> >> Cc: Daniel Vetter 
> >> --
> >> V2: - Use drm_simple_kms_helper to reduce amount of common code
> >> - Add dedicated OF compatible for i.MX6SX
> >> V3: - Update to latest next/master
> > 
> > Imo looks all pretty. Please wrap up in a pull request as soon as you have
> > acks from dt and all that and then send a pull request to Dave.
> 
> I finally got an ACK from Rob on 2/3 , which Dave do you mean, Airlie ?

Yup. We only have 1 drm maintainer ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 2/6] drm: Add TV connector states to drm_connector_state

[Daniel Vetter]

On Tue, Nov 29, 2016 at 10:41:58AM -0800, Eric Anholt wrote:
> From: Boris Brezillon 
> 
> Some generic TV connector properties are exposed in drm_mode_config, but
> they are currently handled independently in each DRM encoder driver.
> 
> Extend the drm_connector_state to store TV related states, and modify the
> drm_atomic_connector_{set,get}_property() helpers to fill the connector
> state accordingly.
> 
> Each driver is then responsible for checking and applying the new config
> in its ->atomic_mode_{check,set}() operations.
> 
> Signed-off-by: Boris Brezillon 
> Signed-off-by: Eric Anholt 
> ---
>  drivers/gpu/drm/drm_atomic.c | 50 
> 
>  include/drm/drm_connector.h  | 32 
>  2 files changed, 82 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 23739609427d..02b0668f51e1 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -986,12 +986,38 @@ int drm_atomic_connector_set_property(struct 
> drm_connector *connector,
>* now?) atomic writes to DPMS property:
>*/
>   return -EINVAL;
> + } else if (property == config->tv_select_subconnector_property) {
> + state->tv.subconnector = val;
> + } else if (property == config->tv_left_margin_property) {
> + state->tv.margins.left = val;
> + } else if (property == config->tv_right_margin_property) {
> + state->tv.margins.right = val;
> + } else if (property == config->tv_top_margin_property) {
> + state->tv.margins.bottom = val;

s/bottom/top/

> + } else if (property == config->tv_bottom_margin_property) {
> + state->tv.margins.right = val;

s/right/bottom/

> + } else if (property == config->tv_mode_property) {
> + state->tv.mode = val;
> + } else if (property == config->tv_brightness_property) {
> + state->tv.brightness = val;
> + } else if (property == config->tv_contrast_property) {
> + state->tv.contrast = val;
> + } else if (property == config->tv_flicker_reduction_property) {
> + state->tv.flicker_reduction = val;
> + } else if (property == config->tv_overscan_property) {
> + state->tv.overscan = val;
> + } else if (property == config->tv_saturation_property) {
> + state->tv.saturation = val;
> + } else if (property == config->tv_hue_property) {
> + state->tv.hue = val;
>   } else if (connector->funcs->atomic_set_property) {
>   return connector->funcs->atomic_set_property(connector,
>   state, property, val);
>   } else {
>   return -EINVAL;
>   }
> +
> + return 0;
>  }
>  EXPORT_SYMBOL(drm_atomic_connector_set_property);
>  
> @@ -1022,6 +1048,30 @@ drm_atomic_connector_get_property(struct drm_connector 
> *connector,
>   *val = (state->crtc) ? state->crtc->base.id : 0;
>   } else if (property == config->dpms_property) {
>   *val = connector->dpms;
> + } else if (property == config->tv_select_subconnector_property) {
> + *val = state->tv.subconnector;
> + } else if (property == config->tv_left_margin_property) {
> + *val = state->tv.margins.left;
> + } else if (property == config->tv_right_margin_property) {
> + *val = state->tv.margins.right;
> + } else if (property == config->tv_top_margin_property) {
> + *val = state->tv.margins.bottom;

s/bottom/top/
> + } else if (property == config->tv_bottom_margin_property) {
> + *val = state->tv.margins.right;

s/right/bottom/

> + } else if (property == config->tv_mode_property) {
> + *val = state->tv.mode;
> + } else if (property == config->tv_brightness_property) {
> + *val = state->tv.brightness;
> + } else if (property == config->tv_contrast_property) {
> + *val = state->tv.contrast;
> + } else if (property == config->tv_flicker_reduction_property) {
> + *val = state->tv.flicker_reduction;
> + } else if (property == config->tv_overscan_property) {
> + *val = state->tv.overscan;
> + } else if (property == config->tv_saturation_property) {
> + *val = state->tv.saturation;
> + } else if (property == config->tv_hue_property) {
> + *val = state->tv.hue;
>   } else if (connector->funcs->atomic_get_property) {
>   return connector->funcs->atomic_get_property(connector,
>   state, property, val);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index ac9d7d8e0e43..2382d44e5fff 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -194,10 +194,40 @@ int drm_display_info_set_bus_formats(struct 
> drm_display_info *info,
>

[PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI

[Jean-Francois Moine]

On Tue, 29 Nov 2016 21:33 +0200
Laurent Pinchart  wrote:

> > > You need a third port for the HDMI encoder output, connected to an HDMI
> > > connector DT node.
> > 
> > I don't see what you mean. The HDMI device is both the encoder
> > and connector (as the TDA998x):
> 
> The driver might create both an encoder and a connector, but I very much 
> doubt 
> that the "allwinner,sun8i-a83t-hdmi" hardware contains a connector, unless 
> the 
> SoC package has an HDMI connector coming out of it :-)
> 
> > plane -> DE2 mixer ---> TCON -> HDMI -> display device
> > - plane --- CRTC -   - encoder  \
> >connector -- (HDMI cable)
> >  audio-controller -   - audio-codec /

The schema is the same as the Dove Cubox: the TDA998x is just a chip
with some wires going out and the physical connector is supposed to be
at the end of the wires.

Here, the HDMI pins of the SoC go to a pure hardware chip and then to
the physical connector. Which software entity do you want to add?

-- 
Ken ar c'hentañ| ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/

[Intel-gfx] [PATCH 2/3] drm/dp/mst: Calculate total link bandwidth instead of hardcoding it

[Pandiyan, Dhinakaran]

On Tue, 2016-11-29 at 22:58 +0200, Ville Syrjälä wrote:
> On Thu, Nov 17, 2016 at 06:03:47PM -0800, Dhinakaran Pandiyan wrote:
> > The total or the nominal link bandwidth, which we save in terms of PBN, is
> > a factor of link rate and lane count. But, currently we hardcode it to
> > 2560 PBN. This results in incorrect computation of total slots.
> > 
> > E.g, 2 lane HBR2 configuration and 4k at 60Hz, 24bpp mode
> >   nominal link bw = 1080 MBps = 1280PBN = 64 slots
> >   required bw 533.25 MHz*3 = 1599.75 MBps or 1896 PBN
> >  with +0.6% margin = 1907.376 PBN = 96 slots
> >   This is greater than the max. possible value of 64 slots. But, we
> >   incorrectly compute available slots as 2560 PBN = 128 slots and don't
> >   return error.
> > 
> > So, let's fix this by calculating the total link bandwidth as
> > link bw (PBN) = BW per time slot(PBN) * max. time slots , where max. time
> > slots is 64
> > 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 04e4571..26dfd99 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -2038,9 +2038,8 @@ int drm_dp_mst_topology_mgr_set_mst(struct 
> > drm_dp_mst_topology_mgr *mgr, bool ms
> > ret = -EINVAL;
> > goto out_unlock;
> > }
> > -
> > -   mgr->total_pbn = 2560;
> > -   mgr->total_slots = DIV_ROUND_UP(mgr->total_pbn, mgr->pbn_div);
> > +   mgr->total_pbn = 64 * mgr->pbn_div;
> 
> Do we actually have a use in mind for total_pbn? It seems unused now.

Not really, I will remove it and submit this patch separately.

> 
> > +   mgr->total_slots = 64;
> 
> Same for total_slots.
> 
> > mgr->avail_slots = mgr->total_slots;
> 
> So avail_slots is all that's used. And shouldn't it actually start
> out at 63 instead of 64 on account of the MTP header always taking
> up one slot?
> 

Yeah, I had a check for < avail_slots in the patch that followed.


-DK
> >  
> > /* add initial branch device at LCT 1 */
> > -- 
> > 2.7.4
> 

[PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

[Laurent Pinchart]

Hi Daniel,

On Tuesday 29 Nov 2016 10:48:21 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:04:34AM +0200, Laurent Pinchart wrote:
> > Most drivers that use bridges forgot to detach them at cleanup time.
> > Instead of fixing them one by one, detach the bridge in the core
> > drm_encoder_cleanup() function.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/gpu/drm/drm_encoder.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > index 5c067719164d..9c1f99646e0d 100644
> > --- a/drivers/gpu/drm/drm_encoder.c
> > +++ b/drivers/gpu/drm/drm_encoder.c
> > @@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
> > 
> >  * the indices on the drm_encoder after us in the encoder_list.
> >  */
> > 
> > +   if (encoder->bridge)
> > +   drm_bridge_detach(encoder->bridge);
> 
> Means we bake in drm_bridge much more as a core thing, but I guess that's
> ok.
> 
> But there's 3 callers of drm_bridge_detach outside of the drm core, can't
> we remove them and drop the EXPORT_SYMBOL for drm_bridge_detach? What's
> it still needed for?

Agreed, I'll fix that.

> I think that cleanup should done in this patch here - drm_bridge_detach
> WARN_ONs if the bridge is already detached ...
>
> > +
> > drm_modeset_lock_all(dev);
> > drm_mode_object_unregister(dev, >base);
> > kfree(encoder->name);

-- 
Regards,

Laurent Pinchart

[PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

[Boris Brezillon]

On Tue, 29 Nov 2016 11:04:33 +0200
Laurent Pinchart  wrote:

> Instead of linking encoders and bridges in every driver (and getting it
> wrong half of the time, as many drivers forget to set the drm_bridge
> encoder pointer), do so in core code. The drm_bridge_attach() function
> needs the encoder and optional previous bridge to perform that task,
> update all the callers.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-

For atmel-hlcdc

Acked-by: Boris Brezillon 

>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
>  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
>  drivers/gpu/drm/drm_bridge.c   | 46 
> --
>  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
>  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
>  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
>  drivers/gpu/drm/imx/parallel-display.c |  4 +-
>  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
>  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
>  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
>  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
>  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
>  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
>  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
>  include/drm/drm_bridge.h   |  3 +-
>  23 files changed, 83 insertions(+), 103 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> index 6119b5085501..e7799b6ee829 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> @@ -230,9 +230,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device 
> *dev,
>   of_node_put(np);
>  
>   if (bridge) {
> - output->encoder.bridge = bridge;
> - bridge->encoder = >encoder;
> - ret = drm_bridge_attach(dev, bridge);
> + ret = drm_bridge_attach(>encoder, bridge, NULL);
>   if (!ret)
>   return 0;
>   }
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 6e0447f329a2..1835f1fdad19 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1227,12 +1227,10 @@ static int analogix_dp_create_bridge(struct 
> drm_device *drm_dev,
>  
>   dp->bridge = bridge;
>  
> - dp->encoder->bridge = bridge;
>   bridge->driver_private = dp;
> - bridge->encoder = dp->encoder;
>   bridge->funcs = _dp_bridge_funcs;
>  
> - ret = drm_bridge_attach(drm_dev, bridge);
> + ret = drm_bridge_attach(dp->encoder, bridge, NULL);
>   if (ret) {
>   DRM_ERROR("failed to attach drm bridge\n");
>   return -EINVAL;
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index b71088dab268..432e0e3fff72 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -1841,13 +1841,12 @@ static int dw_hdmi_register(struct drm_device *drm, 
> struct dw_hdmi *hdmi)
>   hdmi->bridge = bridge;
>   bridge->driver_private = hdmi;
>   bridge->funcs = _hdmi_bridge_funcs;
> - ret = drm_bridge_attach(drm, bridge);
> + ret = drm_bridge_attach(encoder, bridge, NULL);
>   if (ret) {
>   DRM_ERROR("Failed to initialize bridge with drm\n");
>   return -EINVAL;
>   }
>  
> - encoder->bridge = bridge;
>   hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
>  
>   drm_connector_helper_add(>connector,
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 0ee052b7c21a..850bd6509ef1 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -26,6 +26,7 @@
>  #include 
>  
>  #include 
> +#include 
>  
>  /**
>   * DOC: overview
> @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
>  EXPORT_SYMBOL(drm_bridge_remove);
>  
>  /**
> - * drm_bridge_attach - associate given bridge to our DRM device
> + * drm_bridge_attach - attach the bridge to an encoder's chain
>   *
> - * @dev: DRM device
> - * @bridge: bridge control structure
> 

[PATCH 1/2] drm/i915/dsi: Fix swapping of MIPI_SEQ_DEASSERT_RESET / MIPI_SEQ_ASSERT_RESET

[Ville Syrjälä]

On Tue, Nov 29, 2016 at 04:06:31PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 29, 2016 at 02:06:20PM +0100, Hans de Goede wrote:
> > Hi,
> > 
> > Thanks for the quick reply.
> > 
> > On 29-11-16 13:48, Ville Syrjälä wrote:
> > > On Tue, Nov 29, 2016 at 01:38:57PM +0100, Hans de Goede wrote:
> > >> Looking at the ADF code from the Android kernel sources for a
> > >> cherrytrail tablet I noticed that it is calling the
> > >> MIPI_SEQ_ASSERT_RESET sequence from the panel prepare hook.
> > >>
> > >> Until commit b1cb1bd29189 ("drm/i915/dsi: update reset and power 
> > >> sequences
> > >> in panel prepare/unprepare hooks") the mainline i915 code was doing the
> > >> same. That commits effectively swaps the calling of 
> > >> MIPI_SEQ_ASSERT_RESET /
> > >> MIPI_SEQ_DEASSERT_RESET.
> > >>
> > >> Looking at the naming of the sequences that is the right thing to do,
> > >> but the problem is, that the old mainline code and the ADF code was
> > >> actually calling the right sequence (tested on a cube iwork8 air tablet),
> > >> and the swapping of the calling breaks things.
> > >>
> > >> This breakage was likely not noticed in testing because on cherrytrail,
> > >> currently chv_exec_gpio ends up disabling the gpio pins rather then
> > >> setting them (this is fixed in the next patch in this patch-set).
> > >>
> > >> This commit fixes the swapping by fixing MIPI_SEQ_ASSERT/DEASSERT_RESET's
> > >> places in the enum defining them, so that their (new) names match their
> > >> actual use.
> > >>
> > >> Fixes: b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences...")
> > >> Cc: Jani Nikula 
> > >> Cc: Ville Syrjälä 
> > >> Signed-off-by: Hans de Goede 
> > >> ---
> > >>  drivers/gpu/drm/i915/intel_bios.h  | 4 ++--
> > >>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 4 ++--
> > >>  2 files changed, 4 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_bios.h 
> > >> b/drivers/gpu/drm/i915/intel_bios.h
> > >> index 8405b5a..642a5eb 100644
> > >> --- a/drivers/gpu/drm/i915/intel_bios.h
> > >> +++ b/drivers/gpu/drm/i915/intel_bios.h
> > >> @@ -49,11 +49,11 @@ struct edp_power_seq {
> > >>  /* MIPI Sequence Block definitions */
> > >>  enum mipi_seq {
> > >>  MIPI_SEQ_END = 0,
> > >> -MIPI_SEQ_ASSERT_RESET,
> > >> +MIPI_SEQ_DEASSERT_RESET,
> > >>  MIPI_SEQ_INIT_OTP,
> > >>  MIPI_SEQ_DISPLAY_ON,
> > >>  MIPI_SEQ_DISPLAY_OFF,
> > >> -MIPI_SEQ_DEASSERT_RESET,
> > >> +MIPI_SEQ_ASSERT_RESET,
> > >
> > > That naming would be against the spec, so I don't think we want to do it
> > > like this. Unless the spec is totally wrong, that is.
> > 
> > Given that both the ADF code and the i915 driver until now have been using
> > assert on prepare and deassert on unprepare I'm inclined to believe that
> > the spec is wrong. Is the spec available somewhere public ?
> 
> I don't think so. And sadly even if it would it wouldn't really help
> since about the only thing it says is:
> 
> 00 – Reserved
> 01 - MIPIAssertResetPin
> 02 – MIPISendInitialDcsCmds (Use this sequence type for sending 
> initialization commands in LP mode)
> 03 - MIPIDisplayOn (Use this sequence type for sending initialization 
> commands in HS mode)
> 04 – MIPIDisplayOff (Use this sequence type for sending DisplayOff commands 
> in LP mode)
> 05 – MIPIDeassertResetPin
> 06 – MIPIBacklightOn
> 07 - MIPIBacklightOff
> 08 – MIPITearOn
> 09 - MIPITearOff
> 10 - MIPIPanelPowerOn
> 11 - MIPIPanelPowerOff
> Others – Reserved
> 
> So pretty much useless if you actually want to write a working driver.
> 
> > 
> > Also if you look at the v1 sequences with the new names:
> > 
> > MIPI_SEQ_DEASSERT_RESET,
> > MIPI_SEQ_INIT_OTP,
> > MIPI_SEQ_DISPLAY_ON,
> > MIPI_SEQ_DISPLAY_OFF,
> > MIPI_SEQ_ASSERT_RESET,
> > 
> > Then they are exactly in the order as one would call them on
> > enable, followed by disable, which I believe is not a coincidence.
> > 
> > > Can you provide the VBT for the affected machine so other people can
> > > have a look at it?
> > 
> > Sure I can do that, what would be the easiest way (both for me to
> > produce and for you to consume) to do this ?
> 
> /sys/kernel/debug/dri/0/i915_opregion
> 
> For the best chance of preserving the dump for posterity I would
> suggest filing a new bug and attaching it there.
> 
> https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/Intel
> 
> > 
> > While developing this set, I've added printk calls to the code executing the
> > sequences, there are 2 gpios involved nr 60 (backlight enable AFAICT, also 
> > used
> > by the BACKLIGHT sequences) and 72 (reset / panel_enable ?).
> > When efifb is up both are 1 / high.
> > 
> > With the OLD naming, MIPI_SEQ_ASSERT_RESET does:
> > 
> > gpio 72 high
> > delay
> > gpio 72 low
> > delay
> > gpio 72 high
> 
> Hmm. OK so it just toggles the reset pin it seems.
> 
> > 
> > And DEASSERT does:
> > 
> > gpio 72 low
> > gpio 60 low
> 
> 

[PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

[Laurent Pinchart]

Hi Archit,

On Tuesday 29 Nov 2016 16:04:08 Archit Taneja wrote:
> On 11/29/2016 02:34 PM, Laurent Pinchart wrote:
> > Most drivers that use bridges forgot to detach them at cleanup time.
> > Instead of fixing them one by one, detach the bridge in the core
> > drm_encoder_cleanup() function.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/gpu/drm/drm_encoder.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > index 5c067719164d..9c1f99646e0d 100644
> > --- a/drivers/gpu/drm/drm_encoder.c
> > +++ b/drivers/gpu/drm/drm_encoder.c
> > @@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
> >  * the indices on the drm_encoder after us in the encoder_list.
> >  */
> > +   if (encoder->bridge)
> > +   drm_bridge_detach(encoder->bridge);
> 
> This would require the kms driver to still detach the remaining
> n - 1 bridges in a possible chain. We could probably detach all of
> them here, or maybe leave detaching of all to the kms driver, and just
> report a warning here.

I'd prefer detaching them all here, but that's a bit intrusive and should be 
tested correctly. The patch series is already growing big, could we do that in 
a separate patch ?

> > +
> > 
> > drm_modeset_lock_all(dev);
> > drm_mode_object_unregister(dev, >base);
> > kfree(encoder->name);

-- 
Regards,

Laurent Pinchart

[PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

[Laurent Pinchart]

Hi Daniel,

On Tuesday 29 Nov 2016 20:02:01 Laurent Pinchart wrote:
> On Tuesday 29 Nov 2016 11:05:27 Daniel Vetter wrote:
> > On Tue, Nov 29, 2016 at 11:43:19AM +0200, Laurent Pinchart wrote:
> >> On Tuesday 29 Nov 2016 10:35:24 Daniel Vetter wrote:
> >>> On Tue, Nov 29, 2016 at 11:04:33AM +0200, Laurent Pinchart wrote:
>  Instead of linking encoders and bridges in every driver (and getting
>  it wrong half of the time, as many drivers forget to set the
>  drm_bridge encoder pointer), do so in core code. The
>  drm_bridge_attach() function needs the encoder and optional previous
>  bridge to perform that task, update all the callers.
>  
>  Signed-off-by: Laurent Pinchart
>  
>  ---

[snip]

>  diff --git a/drivers/gpu/drm/drm_bridge.c
>  b/drivers/gpu/drm/drm_bridge.c
>  index 0ee052b7c21a..850bd6509ef1 100644
>  --- a/drivers/gpu/drm/drm_bridge.c
>  +++ b/drivers/gpu/drm/drm_bridge.c
> 
> [snip]
> 
>  -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge
>  *bridge)
>  +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge
>  *bridge,
>  +  struct drm_bridge *previous)
>   {
>  -if (!dev || !bridge)
>  +int ret;
>  +
>  +if (!encoder || !bridge)
>  +return -EINVAL;
>  +
>  +if (previous && (!previous->dev || previous->encoder !=
>  encoder))
>   return -EINVAL;
> >>> 
> >>> Not sure we want to allow setting both encoder and bridge for chaining.
> >>> I'd kinda expect that for chained use-case the bridge doesn't care that
> >>> much who started the chain. And if not we can always create a helper to
> >>> look up the drm_encoder.
> >> 
> >> As bridge drivers currently create connectors (I'd like to change that
> >> at some point, but one thing at a time) they need to call
> >> drm_mode_connector_attach_encoder() and thus need to have access to the
> >> drm_encoder object at the beginning of the chain. The drm_bridge
> >> structure has an encoder field, it seems to me that the easiest is to
> >> always populate it, regardless of the position of the bridge in the
> >> chain.
> > 
> > I mean the function inteface, not what we fill out in the drm_bridge
> > struct. I.e. instead of expecting callers to give you the encoder for
> > 
> > chained bridges, look it up yourself:
> > bridge->encoder = previous : previous->encoder ? encoder;
> > 
> > Or something  like that. Just feels confusing to connect a bridge to both
> > a bridge _and_ the first encoder.
> 
> Right. Archit proposed doing it the other way around:
> 
> previous = encoder->bridge;
> while (previous && previous->next)
> previous = previous->next;
> 
> That would simplify the API, at the cost of preventing us from catching
> drivers that attach bridges in the wrong order (through the !previous->dev
> check that you suggested should be turned into a WARN_ON). The previous-
> 
> >encoder != encoder check is also a safety net.
> 
> Any opinion on which option you like better ? I'd be very tempted to go for
> Archit's proposal as it removes the previous parameter from the API, if it
> wasn't for the loss of sanity checking.
> 
> > Wrt creating the connector: Some helpers to make that easier could be
> > useful, and probably we need a separate ->register callback. That's needed
> > for proper demidlayered init/teardown sequence anyway, and then the
> > drm_bridge.c code make sure to only call ->register for the very last
> > bridge. Other bridges could still create ther connectors (less conditions
> > in the code), as long as they don't register them no one will ever see
> > them ;-)

One thing at a time, we'll get there :-)

-- 
Regards,

Laurent Pinchart

[PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI

[Laurent Pinchart]

Hi Jean-François,

Thank you for the patch.

On Tuesday 29 Nov 2016 10:08:25 Jean-Francois Moine wrote:
> Signed-off-by: Jean-Francois Moine 
> ---
>  .../devicetree/bindings/display/sunxi/hdmi.txt | 56 +++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt new file mode
> 100644
> index 000..1e107cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> @@ -0,0 +1,56 @@
> +Allwinner HDMI Transmitter
> +==
> +
> +The Allwinner HDMI transmitters are included in the SoCs.
> +They support audio and video.
> +
> +Required properties:
> + - compatible : should be one of
> + "allwinner,sun8i-a83t-hdmi"
> + "allwinner,sun8i-h3-hdmi"
> + - reg: base address and size of the I/O memory
> + - clocks : phandles to the HDMI clocks as described in
> + Documentation/devicetree/bindings/clock/clock-bindings.txt
> + - clock-names : must be
> + "bus" : bus gate
> + "clock" : streaming clock
> + "ddc-clock" : DDC clock
> + - resets : One or two phandles to the HDMI resets
> + - reset-names : when 2 phandles, must be
> + "hdmi0" and "hdmi1"
> + - #address-cells : should be <1>
> + - #size-cells : should be <0>
> +
> +Required nodes:
> + - port: Audio and video input port nodes with endpoint definitions
> + as defined in Documentation/devicetree/bindings/graph.txt.
> + port at 0 is video and port at 1 is audio.
> +
> +Example:
> +
> + hdmi: hdmi at 01ee {
> + compatible = "allwinner,sun8i-a83t-hdmi";
> + reg = <0x01ee 0x2>;
> + clocks = < CLK_BUS_HDMI>, < CLK_HDMI>,
> +  < CLK_HDMI_DDC>;
> + clock-names = "bus", "clock", "ddc-clock";
> + resets = < RST_HDMI0>, < RST_HDMI1>;
> + reset-names = "hdmi0", "hdmi1";
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins_a>;
> + status = "disabled";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port at 0 { /* video */
> + reg = <0>;
> + hdmi_tcon1: endpoint {
> + remote-endpoint = <_hdmi>;
> + };
> + };
> + port at 1 { /* audio */
> + reg = <1>;
> + hdmi_i2s2: endpoint {
> + remote-endpoint = <_hdmi>;
> + };
> + };

You need a third port for the HDMI encoder output, connected to an HDMI 
connector DT node.

> + };

-- 
Regards,

Laurent Pinchart

[PATCH v7 2/8] drm/sun8i: Add DT bindings documentation of Allwinner DE2

[Laurent Pinchart]

Hi Jean-François,

A brief update.

On Tuesday 29 Nov 2016 20:41:30 Laurent Pinchart wrote:
> On Monday 28 Nov 2016 19:02:39 Jean-Francois Moine wrote:
> > Signed-off-by: Jean-Francois Moine 
> > ---
> > 
> >  .../bindings/display/sunxi/sun8i-de2.txt   | 121 
> >  1 file changed, 121 insertions(+)
> >  create mode 100644
> > 
> > Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> > b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt new file
> > mode 100644
> > index 000..edf38b8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> > @@ -0,0 +1,121 @@
> > +Allwinner sun8i Display Engine 2 subsystem
> > +==
> > +
> > +The Allwinner DE2 subsystem contains a display controller (DE2),
> > +one or two LCD controllers (Timing CONtrollers) and their external
> > +interfaces (encoders/connectors).
> > +
> > +  +---+
> > +  | DE2   |
> > +  |   |
> > +  | +---+ |
> > +  plane --->|   | |   +--+
> > +  | | mixer |>| TCON |---> encoder  ---> display
> > +  plane --->|   | |   +--+ connector device
> > +  | +---+ |
> > +  |   |
> > +  | +---+ |
> > +  plane --->|   | |   +--+
> > +  | | mixer |>| TCON |---> encoder  ---> display
> > +  plane --->|   | |   +--+ connector device
> > +  | +---+ |
> > +  +---+
> > +
> > +The DE2 contains a processor which mixes the input planes and creates
> > +the images which are sent to the TCONs.
> > +From the software point of vue, there are 2 independent real-time
> > +mixers, each one being statically associated to one TCON.
> > +
> > +The TCONs adjust the image format to the one of the display device.
> > +
> > +Display controller (DE2)
> > +
> > +
> > +Required properties:
> > +
> > +- compatible: value should be one of the following
> > +   "allwinner,sun8i-a83t-display-engine"
> > +   "allwinner,sun8i-h3-display-engine"
> > +
> > +- reg: base address and size of the I/O memory
> > +
> > +- clocks: must include clock specifiers corresponding to entries in the
> > +   clock-names property.
> > +
> > +- clock-names: must contain
> > +   "bus": bus gate
> > +   "clock": clock
> > +
> > +- resets: phandle to the reset of the device
> > +
> > +- ports: must contain a list of 2 phandles, indexed by mixer number,
> > +   and pointing to display interface ports of TCONs
> > +
> > +LCD controller (TCON)
> > +=
> > +
> > +Required properties:
> > +
> > +- compatible: should be
> > +   "allwinner,sun8i-a83t-tcon"
> > +
> > +- reg: base address and size of the I/O memory
> > +
> > +- clocks: must include clock specifiers corresponding to entries in the
> > +   clock-names property.
> > +
> > +- clock-names: must contain
> > +   "bus": bus gate
> > +   "clock": pixel clock
> > +
> > +- resets: phandle to the reset of the device
> > +
> > +- interrupts: interrupt number for the TCON
> > +
> > +- port: port node with endpoint definitions as defined in
> > +   Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +Example:
> > +
> > +   de: de-controller at 0100 {
> > +   compatible = "allwinner,sun8i-h3-display-engine";
> > +   reg = <0x0100 0x40>;
> > +   clocks = < CLK_BUS_DE>, < CLK_DE>;
> > +   clock-names = "bus", "clock";
> > +   resets = < RST_BUS_DE>;
> > +   ports = <_p>, <_p>;
> 
> This isn't how the OF graph DT bindings are used. You should instead have
> 
>   ports {
>   #address-cells = <1>;
>   #size-cells = <0>;
>   port at 0 {

I forgot to add reg = <0>; (and similarly for port 1).

>   de_out_0: endpoint {
>   remote_endpoint = <_hdmi>;
>   };
>   };
>   port at 1 {
>   /* No endpoint as the port is not connected */
>   };
>   };
> 
> > +   };
> > +
> > +   tcon0: lcd-controller at 01c0c000 {
> > +   compatible = "allwinner,sun8i-a83t-tcon";
> > +   reg = <0x01c0c000 0x400>;
> > +   clocks = < CLK_BUS_TCON0>, < CLK_TCON0>;
> > +   clock-names = "bus", "clock";
> > +   resets = < RST_BUS_TCON0>;
> > +   interrupts = ;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   tcon0_p: port {
> > +   tcon0_hdmi: endpoint {
> > +   remote-endpoint = <_tcon0>;
> > +   };
> > +   };
> 
> and here
> 
>   port {
>   tcon0_hdmi: endpoint {
>   

[PATCH v7 2/8] drm/sun8i: Add DT bindings documentation of Allwinner DE2

[Laurent Pinchart]

Hi Jean-François,

Thank you for the patch.

On Monday 28 Nov 2016 19:02:39 Jean-Francois Moine wrote:
> Signed-off-by: Jean-Francois Moine 
> ---
>  .../bindings/display/sunxi/sun8i-de2.txt   | 121 ++
>  1 file changed, 121 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt new file
> mode 100644
> index 000..edf38b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> @@ -0,0 +1,121 @@
> +Allwinner sun8i Display Engine 2 subsystem
> +==
> +
> +The Allwinner DE2 subsystem contains a display controller (DE2),
> +one or two LCD controllers (Timing CONtrollers) and their external
> +interfaces (encoders/connectors).
> +
> +  +---+
> +  | DE2   |
> +  |   |
> +  | +---+ |
> +  plane --->|   | |   +--+
> +  | | mixer |>| TCON |---> encoder  ---> display
> +  plane --->|   | |   +--+ connector device
> +  | +---+ |
> +  |   |
> +  | +---+ |
> +  plane --->|   | |   +--+
> +  | | mixer |>| TCON |---> encoder  ---> display
> +  plane --->|   | |   +--+ connector device
> +  | +---+ |
> +  +---+
> +
> +The DE2 contains a processor which mixes the input planes and creates
> +the images which are sent to the TCONs.
> +From the software point of vue, there are 2 independent real-time
> +mixers, each one being statically associated to one TCON.
> +
> +The TCONs adjust the image format to the one of the display device.
> +
> +Display controller (DE2)
> +
> +
> +Required properties:
> +
> +- compatible: value should be one of the following
> + "allwinner,sun8i-a83t-display-engine"
> + "allwinner,sun8i-h3-display-engine"
> +
> +- reg: base address and size of the I/O memory
> +
> +- clocks: must include clock specifiers corresponding to entries in the
> + clock-names property.
> +
> +- clock-names: must contain
> + "bus": bus gate
> + "clock": clock
> +
> +- resets: phandle to the reset of the device
> +
> +- ports: must contain a list of 2 phandles, indexed by mixer number,
> + and pointing to display interface ports of TCONs
> +
> +LCD controller (TCON)
> +=
> +
> +Required properties:
> +
> +- compatible: should be
> + "allwinner,sun8i-a83t-tcon"
> +
> +- reg: base address and size of the I/O memory
> +
> +- clocks: must include clock specifiers corresponding to entries in the
> + clock-names property.
> +
> +- clock-names: must contain
> + "bus": bus gate
> + "clock": pixel clock
> +
> +- resets: phandle to the reset of the device
> +
> +- interrupts: interrupt number for the TCON
> +
> +- port: port node with endpoint definitions as defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> + de: de-controller at 0100 {
> + compatible = "allwinner,sun8i-h3-display-engine";
> + reg = <0x0100 0x40>;
> + clocks = < CLK_BUS_DE>, < CLK_DE>;
> + clock-names = "bus", "clock";
> + resets = < RST_BUS_DE>;
> + ports = <_p>, <_p>;

This isn't how the OF graph DT bindings are used. You should instead have

ports {
#address-cells = <1>;
#size-cells = <0>;
port at 0 {
de_out_0: endpoint {
remote_endpoint = <_hdmi>;
};
};
port at 1 {
/* No endpoint as the port is not connected */
};
};

> + };
> +
> + tcon0: lcd-controller at 01c0c000 {
> + compatible = "allwinner,sun8i-a83t-tcon";
> + reg = <0x01c0c000 0x400>;
> + clocks = < CLK_BUS_TCON0>, < CLK_TCON0>;
> + clock-names = "bus", "clock";
> + resets = < RST_BUS_TCON0>;
> + interrupts = ;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + tcon0_p: port {
> + tcon0_hdmi: endpoint {
> + remote-endpoint = <_tcon0>;
> + };
> + };

and here

port {
tcon0_hdmi: endpoint {
remote-endpoint = <_out_0>;
};
};

> + };
> +
> + /* not used */
> + tcon1: lcd-controller at 01c0d000 {
> + compatible = "allwinner,sun8i-h3-tcon";
> + reg = <0x01c0d000 0x400>;
> + clocks = < CLK_BUS_TCON1>,
> +

[PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI

[Jean-Francois Moine]

On Tue, 29 Nov 2016 20:46:22 +0200
Laurent Pinchart  wrote:
[snip]
> > +Example:
> > +
> > +   hdmi: hdmi at 01ee {
> > +   compatible = "allwinner,sun8i-a83t-hdmi";
> > +   reg = <0x01ee 0x2>;
> > +   clocks = < CLK_BUS_HDMI>, < CLK_HDMI>,
> > +< CLK_HDMI_DDC>;
> > +   clock-names = "bus", "clock", "ddc-clock";
> > +   resets = < RST_HDMI0>, < RST_HDMI1>;
> > +   reset-names = "hdmi0", "hdmi1";
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_pins_a>;
> > +   status = "disabled";
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   port at 0 { /* video */
> > +   reg = <0>;
> > +   hdmi_tcon1: endpoint {
> > +   remote-endpoint = <_hdmi>;
> > +   };
> > +   };
> > +   port at 1 { /* audio */
> > +   reg = <1>;
> > +   hdmi_i2s2: endpoint {
> > +   remote-endpoint = <_hdmi>;
> > +   };
> > +   };
> 
> You need a third port for the HDMI encoder output, connected to an HDMI 
> connector DT node.

I don't see what you mean. The HDMI device is both the encoder
and connector (as the TDA998x):

plane -> DE2 mixer ---> TCON -> HDMI -> display device
- plane --- CRTC -   - encoder  \
   connector -- (HDMI cable)
 audio-controller -   - audio-codec /

> > +   };

-- 
Ken ar c'hentañ| ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/

[PATCH] drm: parse hf-vsdb

[Shashank Sharma]

HDMI 2.0 / CEA-861-F specs define a new CEA extension data block,
called hdmi-forum vendor specific data block (HF-VSDB). This block
contains information about sink's support for HDMI 2.0 compliant
features. These features are:
- Deep color YUV 420 support and BPC
- 3D flags for
- OSD Displarity
- Dual view signaling
- independent view signaling
- SCDC support
- Max TMDS char rate
- Scrambling support

This patch adds a parser function for this block, and add flags to
indicate support for new features, in drm_display_info structure.

Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/drm_edid.c  | 73 +
 include/drm/drm_connector.h | 48 +
 include/drm/drm_edid.h  |  5 
 include/linux/hdmi.h|  1 +
 4 files changed, 127 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 336be31..b18bfe0 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3223,6 +3223,27 @@ static int add_3d_struct_modes(struct drm_connector 
*connector, u16 structure,
return 0;
 }

+static bool cea_db_is_hf_vsdb(const u8 *db)
+{
+   u8 len;
+   int hfvsdb_id;
+
+   if (cea_db_tag(db) != VENDOR_BLOCK)
+   return false;
+
+   len = cea_db_payload_len(db);
+   if (len < 7 || len > 31)
+   return false;
+
+   /* version should be 1 */
+   if (db[4]  != 1)
+   return false;
+
+   hfvsdb_id = db[1] | (db[2] << 8) | (db[3] << 16);
+
+   return hfvsdb_id == HDMI_IEEE_OUI_HFVSDB;
+}
+
 static bool cea_db_is_hdmi_vsdb(const u8 *db)
 {
int hdmi_id;
@@ -3767,6 +3788,56 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
 }
 EXPORT_SYMBOL(drm_rgb_quant_range_selectable);

+static void drm_parse_yuv420_deep_color_info(struct drm_connector *connector,
+   const u8 *db)
+{
+   struct drm_display_info *info = >display_info;
+
+   if (db[7] & DRM_EDID_YUV420_DC_48)
+   info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_48;
+   if (db[7] & DRM_EDID_YUV420_DC_36)
+   info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_36;
+   if (db[7] & DRM_EDID_YUV420_DC_30)
+   info->edid_yuv420_dc_modes |= DRM_EDID_YUV420_DC_30;
+
+   if (!info->edid_yuv420_dc_modes) {
+   DRM_DEBUG("%s: No YUV 420 deep color support in sink.\n",
+ connector->name);
+   return;
+   }
+}
+
+static void
+drm_parse_hf_vsdb(struct drm_connector *connector, const u8 *db)
+{
+   struct drm_display_info *info = >display_info;
+
+   if (db[5]) {
+   /*
+* If the sink supplies max tmds char rate in db,
+* the actual max tmds rate = db[5] * 5Mhz.
+*/
+   info->max_tmds_clock = db[5] * 5000;
+   DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
+   info->max_tmds_clock);
+   }
+
+   if (db[6] & DRM_HFVSDB_SCDC_SUPPORT)
+   info->scdc_supported = true;
+   if (db[6] & DRM_HFVSDB_SCDC_RR_CAP)
+   info->scdc_rr_cap = true;
+   if (db[6] & DRM_HFVSDB_SCRAMBLING)
+   info->scrambling = true;
+   if (db[6] & DRM_HFVSDB_INDEPENDENT_VIEW)
+   info->independent_view_3d = true;
+   if (db[6] & DRM_HFVSDB_DUAL_VIEW)
+   info->dual_view_3d = true;
+   if (db[6] & DRM_HFVSDB_3D_OSD)
+   info->osd_disparity_3d = true;
+
+   drm_parse_yuv420_deep_color_info(connector, db);
+}
+
 static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
   const u8 *hdmi)
 {
@@ -3881,6 +3952,8 @@ static void drm_parse_cea_ext(struct drm_connector 
*connector,

if (cea_db_is_hdmi_vsdb(db))
drm_parse_hdmi_vsdb_video(connector, db);
+   if (cea_db_is_hf_vsdb(db))
+   drm_parse_hf_vsdb(connector, db);
}
 }

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 34f9741..d011dd5 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -167,6 +167,46 @@ struct drm_display_info {
 */
u32 bus_flags;

+#define DRM_HFVSDB_SCDC_SUPPORT(1<<7)
+#define DRM_HFVSDB_SCDC_RR_CAP (1<<6)
+#define DRM_HFVSDB_SCRAMBLING  (1<<3)
+#define DRM_HFVSDB_INDEPENDENT_VIEW(1<<2)
+#define DRM_HFVSDB_DUAL_VIEW   (1<<1)
+#define DRM_HFVSDB_3D_OSD  (1<<0)
+
+   /**
+* @scdc_supported: Sink supports SCDC functionality.
+*/
+   bool scdc_supported;
+
+   /**
+* @scdc_rr_cap: Sink has SCDC read request capability.
+*/
+   bool scdc_rr_cap;
+
+   /**
+* @scrambling: Sync supports scrambling for <=340 Mcsc TMDS
+* char rates. 

[PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart]

Hi Rob,

On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
> On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
> > On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> >> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> >>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
>  Document properties common to several display panels in a central
>  location that can be referenced by the panel device tree bindings.
> >>> 
> >>> Looks good. Just one comment...
> >>> 
> >>> [...]
> >>> 
>  +Connectivity
>  +
>  +
>  +- ports: Panels receive video data through one or multiple
>  connections. While
>  +  the nature of those connections is specific to the panel type, the
>  +  connectivity is expressed in a standard fashion using ports as
>  specified in
>  +  the device graph bindings defined in
>  +  Documentation/devicetree/bindings/graph.txt.
> >>> 
> >>> We allow panels to either use graph binding or be a child of the
> >>> display controller.
> >> 
> >> I knew that some display controllers use a phandle to the panel (see the
> >> fsl,panel and nvidia,panel properties), but I didn't know we had panels
> >> as children of display controller nodes. I don't think we should allow
> >> that for anything but DSI panels, as the DT hierarchy is based on control
> >> buses. Are you sure we have other panels instantiated through that
> >> mechanism ?
>
> Some panels have no control bus, so were do we place them?

I'd say under the root node, like all similar control-less devices.

> I would say the hierarchy is based on buses with a preference for the
> control bus when there are multiple buses. I'm not a fan of just sticking
> things are the top level.

OK, so much for my comment a few lines up :-)

The problem with placing non-DSI panels as children of the display controller 
and not using OF graph is that the panel bindings become dependent of the 
display controller being used. A display controller using OF graph would 
require the panel to do the same, while a display controller expecting a panel 
child node (with a specific name) would require DT properties for the panel 
node.

I'm also not sure the complexity of OF graph is really that prohibitive if you 
compare it to panels as child nodes. To get the panel driver to bind to the 
panel DT node the display controller driver would need to create a platform 
device for the panel and register it. That's not very difficult, but parsing a 
single port and endpoint isn't either (and we could even provide a helper 
function for that, a version of of_drm_find_panel() that would take as an 
argument the display controller device node instead of the panel device node).

> > Ping ?
> > 
> > Please note that this file documents properties common to multiple panel
> > DT bindings, but in no way makes it mandatory to use the OF graph bindings
> > for panels. The decision is left to individual bindings.
> 
> It is mandatory in the sense that we don't want more cases of "fsl,panel".

That I agree with :-)

-- 
Regards,

Laurent Pinchart

[PATCH libdrm 5/5] xf86drm: implement an OpenBSD specific drmGetDevice

[Emil Velikov]

On 26 November 2016 at 00:40, Jonathan Gray  wrote:
> This avoids walking all of /dev and directly maps the fd to a path to a
> primary node.
>
I realise that the code is pretty ugly/bad/etc, but I would stay way
from similar optimisations. As-is it will just work as you guys get
support for render nodes/other.
That is unless things are noticeably slow [or bad in general].

Thanks
Emil

[Bug 98005] VCE dual instance encoding inconsistent since st/va: enable dual instances encode by sync surface

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98005

--- Comment #34 from Boyuan Zhang  ---
Hi Andy,

I re-checked the logic, cqp behaviour is affected patch#1. By re-testing cqp, I
confirmed that force to submit two consecutive single job logic should be
applied to cqp as well, otherwise we have a chance to miss a flush or end up
with sending last p and next i frame together which may protentially cause
corruption. Nice catch!

Solution is simple, as you already tried, remove the "if statement" to apply
new logic to cqp as well.

New patches can be found at:

https://lists.freedesktop.org/archives/mesa-dev/2016-November/136860.html
https://lists.freedesktop.org/archives/mesa-dev/2016-November/136855.html

I will push them to upstream once you confirmed the fix. :-)

Regards,
Boyuan

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/0ff4c287/attachment.html>

[PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

[Laurent Pinchart]

Hi Daniel,

On Tuesday 29 Nov 2016 11:05:27 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:43:19AM +0200, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 10:35:24 Daniel Vetter wrote:
> >> On Tue, Nov 29, 2016 at 11:04:33AM +0200, Laurent Pinchart wrote:
> >>> Instead of linking encoders and bridges in every driver (and getting
> >>> it wrong half of the time, as many drivers forget to set the
> >>> drm_bridge encoder pointer), do so in core code. The
> >>> drm_bridge_attach() function needs the encoder and optional previous
> >>> bridge to perform that task, update all the callers.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> 
> >>> ---
> >>> 
> >>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
> >>>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
> >>>  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
> >>>  drivers/gpu/drm/drm_bridge.c   | 46 +
> >>>  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
> >>>  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
> >>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
> >>>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
> >>>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
> >>>  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
> >>>  drivers/gpu/drm/imx/parallel-display.c |  4 +-
> >>>  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
> >>>  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
> >>>  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
> >>>  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
> >>>  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
> >>>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
> >>>  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
> >>>  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
> >>>  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
> >>>  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
> >>>  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
> >>>  include/drm/drm_bridge.h   |  3 +-
> >>>  23 files changed, 83 insertions(+), 103 deletions(-)
> > 
> > [snip]
> > 
> >>> diff --git a/drivers/gpu/drm/drm_bridge.c
> >>> b/drivers/gpu/drm/drm_bridge.c
> >>> index 0ee052b7c21a..850bd6509ef1 100644
> >>> --- a/drivers/gpu/drm/drm_bridge.c
> >>> +++ b/drivers/gpu/drm/drm_bridge.c

[snip]

> >>> -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge
> >>> *bridge)
> >>> +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge
> >>> *bridge,
> >>> +   struct drm_bridge *previous)
> >>>  {
> >>> - if (!dev || !bridge)
> >>> + int ret;
> >>> +
> >>> + if (!encoder || !bridge)
> >>> + return -EINVAL;
> >>> +
> >>> + if (previous && (!previous->dev || previous->encoder != encoder))
> >>>   return -EINVAL;
> >> 
> >> Not sure we want to allow setting both encoder and bridge for chaining.
> >> I'd kinda expect that for chained use-case the bridge doesn't care that
> >> much who started the chain. And if not we can always create a helper to
> >> look up the drm_encoder.
> > 
> > As bridge drivers currently create connectors (I'd like to change that at
> > some point, but one thing at a time) they need to call
> > drm_mode_connector_attach_encoder() and thus need to have access to the
> > drm_encoder object at the beginning of the chain. The drm_bridge structure
> > has an encoder field, it seems to me that the easiest is to always
> > populate it, regardless of the position of the bridge in the chain.
> 
> I mean the function inteface, not what we fill out in the drm_bridge
> struct. I.e. instead of expecting callers to give you the encoder for
> chained bridges, look it up yourself:
> 
>   bridge->encoder = previous : previous->encoder ? encoder;
> 
> Or something  like that. Just feels confusing to connect a bridge to both
> a bridge _and_ the first encoder.

Right. Archit proposed doing it the other way around:

previous = encoder->bridge;
while (previous && previous->next)
previous = previous->next;

That would simplify the API, at the cost of preventing us from catching 
drivers that attach bridges in the wrong order (through the !previous->dev 
check that you suggested should be turned into a WARN_ON). The previous-
>encoder != encoder check is also a safety net.

Any opinion on which option you like better ? I'd be very tempted to go for 
Archit's proposal as it removes the previous parameter from the API, if it 
wasn't for the loss of sanity checking.

> Wrt creating the connector: Some helpers to make that easier could be
> useful, and probably we need a separate ->register callback. That's needed
> for proper demidlayered init/teardown sequence anyway, and then the
> drm_bridge.c 

[PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

[Laurent Pinchart]

Hi Archit,

On Tuesday 29 Nov 2016 15:57:06 Archit Taneja wrote:
> On 11/29/2016 02:34 PM, Laurent Pinchart wrote:
> > Instead of linking encoders and bridges in every driver (and getting it
> > wrong half of the time, as many drivers forget to set the drm_bridge
> > encoder pointer), do so in core code. The drm_bridge_attach() function
> > needs the encoder and optional previous bridge to perform that task,
> > update all the callers.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
> >  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
> >  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
> >  drivers/gpu/drm/drm_bridge.c   | 46 -
> >  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
> >  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
> >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
> >  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
> >  drivers/gpu/drm/imx/parallel-display.c |  4 +-
> >  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
> >  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
> >  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
> >  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
> >  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
> >  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
> >  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
> >  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
> >  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
> >  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
> >  include/drm/drm_bridge.h   |  3 +-
> >  23 files changed, 83 insertions(+), 103 deletions(-)

[snip]

> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 0ee052b7c21a..850bd6509ef1 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c

[snip]

> > @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
> >  EXPORT_SYMBOL(drm_bridge_remove);
> >  
> >  /**
> > - * drm_bridge_attach - associate given bridge to our DRM device
> > + * drm_bridge_attach - attach the bridge to an encoder's chain
> >   *
> > - * @dev: DRM device
> > - * @bridge: bridge control structure
> > + * @encoder: DRM encoder
> > + * @bridge: bridge to attach
> > + * @previous: previous bridge in the chain (optional)
> >   *
> > - * Called by a kms driver to link one of our encoder/bridge to the given
> > - * bridge.
> > + * Called by a kms driver to link the bridge to an encoder's chain. The
> > previous
> > + * argument specifies the previous bridge in the chain. If NULL, the
> > bridge is
> > + * linked directly at the encoder's output. Otherwise it is linked at the
> > + * previous bridge's output.
> >   *
> > - * Note that setting up links between the bridge and our encoder/bridge
> > - * objects needs to be handled by the kms driver itself.
> > + * If non-NULL the previous bridge must be already attached by a call to
> > this
> > + * function.
> >   *
> >   * RETURNS:
> >   * Zero on success, error code on failure
> >   */
> > -int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge)
> > +int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge
> > *bridge,
> > + struct drm_bridge *previous)
> >  {
> > -   if (!dev || !bridge)
> > +   int ret;
> > +
> > +   if (!encoder || !bridge)
> > +   return -EINVAL;
> 
> I think we could derive previous from the encoder itself. Something like:
> 
>   previous = encoder->bridge;
>   while (previous && previous->next)
>   previous = previous->next;

That's a very good point. It would however prevent us from catching drivers 
that attach bridges in the wrong order, which the !previous->dev currently 
allows us to do (and it should be turned into a WARN_ON as Daniel proposed).

I'm fine losing that ability, as your proposal makes the API simpler. I'll let 
you decide, which option do you prefer ?

> > +
> > +   if (previous && (!previous->dev || previous->encoder != encoder))
> > return -EINVAL;
> > 
> > if (bridge->dev)
> > return -EBUSY;
> > 
> > -   bridge->dev = dev;
> > +   bridge->dev = encoder->dev;
> > +   bridge->encoder = encoder;
> > +
> > +   if (bridge->funcs->attach) {
> > +   ret = bridge->funcs->attach(bridge);
> > +   if (ret < 0) {
> > +   bridge->dev = NULL;
> > +   bridge->encoder = NULL;
> > +   return ret;
> > +   }
> > +   }
> > 
> > - 

[PATCH libdrm 3/5] xf86drm: implement drmParsePciDeviceInfo for OpenBSD

[Emil Velikov]

On 26 November 2016 at 00:40, Jonathan Gray  wrote:
> Implement drmParsePciDeviceInfo for OpenBSD by using the new
> DRM_IOCTL_GET_PCIINFO ioctl.
>
> Signed-off-by: Jonathan Gray 
> ---
>  xf86drm.c | 51 +++
>  1 file changed, 51 insertions(+)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index b355c83..581527b 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -102,6 +102,26 @@
>  #define DRM_MAJOR 226 /* Linux */
>  #endif
>
> +#ifdef __OpenBSD__
> +
> +#define X_PRIVSEP
> +
> +struct drm_pciinfo {
> +   uint16_tdomain;
> +   uint8_t bus;
> +   uint8_t dev;
> +   uint8_t func;
> +   uint16_tvendor_id;
> +   uint16_tdevice_id;
> +   uint16_tsubvendor_id;
> +   uint16_tsubdevice_id;
> +   uint8_t revision_id;
> +};
> +
> +#define DRM_IOCTL_GET_PCIINFO  DRM_IOR(0x15, struct drm_pciinfo)
> +
> +#endif
> +
>  #define DRM_MSG_VERBOSITY 3
>
>  #define memclear(s) memset(, 0, sizeof(s))
> @@ -2991,6 +3011,37 @@ static int drmParsePciDeviceInfo(const char *d_name,
>  device->subdevice_id = config[46] | (config[47] << 8);
>
>  return 0;
> +#elif defined(__OpenBSD__)
> +struct drm_pciinfo pinfo;
> +char buf[PATH_MAX + 1];
> +int fd, n;
> +
> +n = snprintf(buf, sizeof(buf), "%s/%s", DRM_DIR_NAME, d_name);
> +if (n == -1 || n >= sizeof(buf))
> +return -errno;
> +
> +#ifndef X_PRIVSEP
> +fd = open(buf, O_RDWR, 0);
> +#else
> +fd = priv_open_device(buf);
> +#endif
> +
Since X_PRIVSEP is always set one can drop the ifndef case alongside
the define X_PRIVSEP all together. At the same time, priv_open_device
isn't defined thus one might well use drmOpenMinor() like in 4/5 ?

Sidenote: In the future we might fold drmParsePciBusInfo and
drmParsePciDeviceInfo, but for the moment we have to keep them
separate :-(

Thanks
Emil

[PATCH v3 09/13] drm: Add encoder_type field to the drm_bridge structure

[Laurent Pinchart]

Hi Daniel,

On Tuesday 29 Nov 2016 11:27:20 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:58:44AM +0200, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 10:56:53 Daniel Vetter wrote:
> >> On Tue, Nov 29, 2016 at 11:04:39AM +0200, Laurent Pinchart wrote:
> >>> The drm_bridge object models on- or off-chip hardware encoders and
> >>> provide an abstract control API to display drivers. In order to help
> >>> display drivers creating the right kind of drm_encoder object, expose
> >>> the type of the hardware encoder associated with each bridge.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> 
> >> 
> >> DRM_MODE_ENCODER_BRIDGE. Problem solved, because in reality no one cares
> >> one iota about the encoder type.
> > 
> > It's exposed to userspace though, are you 100% sure we won't break
> > anything ?
>
> We've added DP, DSI, DPMST and DPI encoder types thus far, no one
> screamed.

In that case why don't we go one step further and remove the encoder type 
completely ? We can't remove the field from the API, but we can hardcode it to 
a single value.

There are however drivers that rely on the encoder type (radeon, nouveau, sti, 
amdgpu, msm and rcar-du, but I'll fix the last one) so we'd need to address 
that first. If we don't want to remove the encoder_type field from in-kernel 
structures and let drivers use it, then I don't think DRM_MODE_ENCODER_BRIDGE 
would be a good option, we should report the real type instead.

> We should be fine I think. Connector types are a bit different, userspace
> (at least X) wants to pretty-print them for xrandr names.

-- 
Regards,

Laurent Pinchart

[PATCH libdrm 2/5] xf86drm: implement drmParseSubsystemType for OpenBSD

[Emil Velikov]

On 26 November 2016 at 00:40, Jonathan Gray  wrote:
> Implement drmParseSubsystemType for OpenBSD by always returning
> DRM_BUS_PCI.  No non-pci drm drivers are in the kernel and this is
> unlikely to change anytime soon as the existing ones aren't permissively
> licensed.
>
A few noticeable X11 MIT style licensed drivers include qxl, vgem and
virtio. Two of which PCI ones and vgem in it's own unique category ;-)

There was a question about re-licensing [some] drivers a few years ago
at XDC in France. IIRC devs were fine with it, yet it doesn't seem
like things changed much.
Have you/fellow BSD developers considered reaching out [as a group] to
interested drivers/developers ? Pardon if I've already
mentioned/already asked about this.

Thanks
Emil

[PATCH libdrm 1/5] xf86drm: implement drmGetMinorNameForFD for non-sysfs

[Emil Velikov]

On 26 November 2016 at 00:40, Jonathan Gray  wrote:
> Implement drmGetMinorNameForFD for systems without sysfs by
> adapting drm_get_device_name_for_fd() from the Mesa loader.
>
> Signed-off-by: Jonathan Gray 
> ---
>  xf86drm.c | 20 +++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index ed924a7..216220c 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2818,7 +2818,25 @@ static char *drmGetMinorNameForFD(int fd, int type)
>  out_close_dir:
>  closedir(sysdir);
>  #else
> -#warning "Missing implementation of drmGetMinorNameForFD"
> +struct stat sbuf;
> +unsigned int maj, min;
> +char buf[PATH_MAX + 1];
> +int n;
> +
> +if (fstat(fd, ))
> +return NULL;
> +
> +maj = major(sbuf.st_rdev);
> +min = minor(sbuf.st_rdev);
> +
> +if (maj != DRM_MAJOR || !S_ISCHR(sbuf.st_mode))
> +return NULL;
> +
> +n = snprintf(buf, sizeof(buf), DRM_DEV_NAME, DRM_DIR_NAME, min);
> +if (n == -1 || n >= sizeof(buf))
> +return NULL;
> +
> +return strdup(buf);
Doesn't look too good I'm afraid:
 - you ignore the node type, making the whole helper and API that
depends on it useless.
Note: mesa wants to know the render node name for the given fd. We can
replace with drmGetDevice but I'd like to check the double-auth [and
related fun] trimming things down before changing things.

 - implementation seems identical to drmGetDeviceNameFromFd(). Barring
a few trivial bits of course.
Speaking of which there is drmGetDeviceNameFromFd2 which attributes
for any node type(s) - the present primary, control and render plus
any future ones.
I'm leaning towards using it in the next (or one after) version in mesa.

Have you and fellow OpenBSD developers considered render nodes. Do you
have any preliminary ideas how it will be exposed, such that you can
build a comprehensive interface here ?

Thanks
Emil

[Bug 98905] XFX Radeon RX 480 XXX OC GPU hangs in games with "auto" power_dpm_force_performance_level

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98905

Bug ID: 98905
   Summary: XFX Radeon RX 480 XXX OC GPU hangs in games with
"auto" power_dpm_force_performance_level
   Product: DRI
   Version: unspecified
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: haagch at frickel.club

I've had this across several applications and several kernel/mesa/llvm versions
and I'm not sure I get everything here right, but here it goes.

Most easily I can reproduce the GPU hang on SOMA when starting a new game - it
will hang while the intro video plays or shortly after, in the game - other and
very demanding games work without hangs.

It would hang like this:

[Di Nov 29 19:46:21 2016] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx
timeout, last signaled seq=3374996, last emitted seq=3374998
[Di Nov 29 19:46:21 2016] [drm] IP block:5 is hang!

Just now is the first time it actually recovered, usually I need to hard reset
the PC.

Now this GPU is factory overclocked a little bit:
/sys/class/drm/card0/device/pp_dpm_sclk
0: 300Mhz
1: 608Mhz
2: 910Mhz
3: 1077Mhz
4: 1145Mhz
5: 1191Mhz
6: 1236Mhz
7: 1288Mhz *

I am reasonably confident that when I set a power level with this command:

echo manual > /sys/class/drm/card0/device/power_dpm_force_performance_level
echo 7 > /sys/class/drm/card0/device/pp_dpm_sclk

the GPU will not hang while the power level is fixed.
Therefore I think that the GPU hang is related to switching between power
levels.
It's possible that this problem is specific to this factory overclocked model.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/988f5bc1/attachment.html>

[Bug 95306] Random Blank(black) screens on "Carrizo"

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=95306

--- Comment #36 from Patrick Laurin  ---
Still got black screen after grub boot, event with 4.9 rc-7.
Sometime I have time to play around for 5 minutes, sometime it turns black
almost immediately. The screen is still powered on and laptop still
running, just plain black image.

Sometimes, when it turns black, it's like I can see the image drop down the
screen in a fraction of a second.

On 29 November 2016 at 07:32,  wrote:

> *Comment # 35 <https://bugs.freedesktop.org/show_bug.cgi?id=95306#c35> on
> bug 95306 <https://bugs.freedesktop.org/show_bug.cgi?id=95306> from Tom
>  *
>
> Kernel 4.9rc7 seems very stable after it boots up. But overheats a lot and
> booting up often takes several attempts.
>
> --
> You are receiving this mail because:
>
>- You are on the CC list for the bug.
>
>

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/bb575416/attachment-0001.html>

[Bug/RFC] drm/imx: warning: vblank wait timed out

[Christopher Spinrath]

Hi Philipp,

thanks for your answer!

On 11/29/2016 05:38 PM, Philipp Zabel wrote:
> Hi Christopher,
> 
> Am Dienstag, den 29.11.2016, 16:45 +0100 schrieb Christopher Spinrath:
>> Hi all,
>>
>> I am trying to enable the second display pipeline of my imx6q based
>> Utilite Pro. I appended a devicetree patch to this email for reference.
>>
>> While it works perfectly on its own there occur kernel warnings (see
>> below) if both pipelines are connected. Furthermore, the machine becomes
>> almost unresponsive. Note that these warnings start to happen around the
>> time getty is started. No other userspace stuff (e.g. X11) is involved.
>>
>> I can make it work by adding the following hack to the devicetree:
>>
>> display-subsystem {
>>  compatible = "fsl,imx-display-subsystem";
>>  ports = <_di0>, <_di0>, <_di1>, <_di1>;
>> };
>>
>> (i.e. reordering the ports of the display-subsystem.)
>>
>> To my understanding the imx drm driver maps both outputs to ipu1 by
>> default which cannot handle the load.
> 
> What is the load?

I meant the workload of an ipu when driving both outputs. My setup
consists of two displays set to the mode 1920x1200 at 60Hz.

>>  But using ipu1 and ipu2 for a
>> single output each works. Unfortunately, I didn't find a way to model
>> this restriction in the devicetree (in a sane way).
> 
> You can sever the links between HDMI encoder mux and IPU1 display
> interfaces to keep it from being connected to IPU1 DI1:
> 
> /delete-node/_di0_hdmi;
> /delete-node/_mux_0;
> 
> /delete-node/_di1_hdmi;
> /delete-node/_mux_1;

Great! That works and is better than the display-subsystem workaround
(for the record the syntax is wrong; I had to use the node names not the
labels).

>> IMHO the driver should at least disable one of the outputs instead of
>> rendering the whole system unusable. Optimally, it should figure out a
>> correct mapping between outputs and ipu's itself.
> 
> It should. The problem with the IPU limitations is that they are
> cross-crtc. The reference manual says that the maximum combined rate for
> both display interfaces in an IPU is 240 MP/s, so we'd need to take into
> account the pixel clocks of already enabled sibling crtcs when deciding
> whether a mode can be set.

So deleting the links between HDMI encoder mux and IPU1 in the
devicetree is the way to go/the preferred solution?

Cheers,
Christopher

> regards
> Philipp
> 

[PATCH V3 3/3] drm: Add new driver for MXSFB controller

[Marek Vasut]

On 11/14/2016 12:47 PM, Daniel Vetter wrote:
> On Mon, Nov 14, 2016 at 11:10:36AM +0100, Marek Vasut wrote:
>> Add new driver for the MXSFB controller found in i.MX23/28/6SX .
>> The MXSFB controller is a simple framebuffer controller with one
>> parallel LCD output. Unlike the MXSFB fbdev driver that is used
>> on these systems now, this driver uses the DRM/KMS framework.
>>
>> Signed-off-by: Marek Vasut 
>> Cc: Lucas Stach 
>> Cc: Fabio Estevam 
>> Cc: Shawn Guo 
>> Cc: Daniel Vetter 
>> --
>> V2: - Use drm_simple_kms_helper to reduce amount of common code
>> - Add dedicated OF compatible for i.MX6SX
>> V3: - Update to latest next/master
> 
> Imo looks all pretty. Please wrap up in a pull request as soon as you have
> acks from dt and all that and then send a pull request to Dave.

I finally got an ACK from Rob on 2/3 , which Dave do you mean, Airlie ?

-- 
Best regards,
Marek Vasut

[PATCH V3 2/3] dt-bindings: mxsfb: Add new bindings for the MXSFB driver

[Marek Vasut]

On 11/29/2016 04:29 PM, Rob Herring wrote:
> On Fri, Nov 25, 2016 at 12:26 PM, Marek Vasut  wrote:
>> On 11/16/2016 01:21 PM, Marek Vasut wrote:
>>> Add new DT bindings for new MXSFB driver that is using the
>>> OF graph to parse the video output structure instead of
>>> hard-coding the display properties into the MXSFB node.
>>> The old MXSFB fbdev driver bindings are preserved in the
>>> same file in the "Old bindings" section.
>>>
>>> Signed-off-by: Marek Vasut 
>>> Cc: Rob Herring 
>>> Cc: Lucas Stach 
>>> Cc: Fabio Estevam 
>>> Cc: Shawn Guo 
>>> --
>>> V2: - Merge the new bindings into mxsfb.txt file instead of keeping
>>>   them in separate mxsfb-drm.txt file.
>>> - Add dedicated compatible for i.MX6SX
>>> - Drop all references to DRM/KMS
>>> - Repair the required bits in clock node
>>> V3: - Replace lcdif with LCDIF, lcdif at 0 with display-controller@,
>>>   Old with Deprecated to address V2 feedback
>>
>> Bump ?
> 
> If you don't send to DT list, it won't be in my queue.
> 
> Acked-by: Rob Herring 
> 
Did I miss the list _again_ ? Ug, apologies, I'll apply brown paper
bag now.

-- 
Best regards,
Marek Vasut

[PATCH] dim: Add status command

[Jani Nikula]

On Tue, 29 Nov 2016, Daniel Vetter  wrote:
> Useful with our branch proliferation to make sure nothing is stuck (we
> now also have drm-misc-next/-next-fixes/-fixes).
>
> v2: Gracefully handle if some remotes arent' there.
>
> Acked-by: seanpaul at chromium.org
> Signed-off-by: Daniel Vetter 

Acked-by: Jani Nikula 

Push away!

> ---
>  dim | 22 ++
>  dim.rst |  5 +
>  2 files changed, 27 insertions(+)
>
> diff --git a/dim b/dim
> index 3dd2d1dec796..b21d1321af09 100755
> --- a/dim
> +++ b/dim
> @@ -1285,6 +1285,28 @@ function dim_update_branches
>   update_rerere_cache
>  }
>  
> +function dim_status
> +{
> + local repo remote drm_remote patches
> +
> + cd $DIM_PREFIX/$DIM_DRM_INTEL
> +
> + drm_remote=$(url_to_remote $drm_upstream_git)
> +
> + for branch in $dim_branches ; do
> + repo=`branch_to_repo $branch`
> + if ! remote=`repo_to_remote $repo` ; then
> + continue
> + fi
> +
> + patches=$(git log --oneline $remote/$branch ^origin/master 
> ^$drm_remote/drm-next ^$drm_remote/drm-fixes | wc -l)
> +
> + if [[ $patches -ne 0 ]] ; then
> + echo $repo/$branch: $patches unmerged patches
> + fi
> + done
> +}
> +
>  function setup_aux_checkout # name url directory
>  {
>   local name url dir remote
> diff --git a/dim.rst b/dim.rst
> index 9a045a43e7b7..0c52ab43e400 100644
> --- a/dim.rst
> +++ b/dim.rst
> @@ -80,6 +80,11 @@ always maintained from the same machine, even if different 
> branches are
>  maintained on different machines (by different maintainers), there's no need 
> to
>  run this command.
>  
> +status
> +--
> +
> +Lists all branches with unmerged patches, and how many patches are unmerged.
> +
>  rebuild-tip
>  ---
>  Rebuild and push the integration tree.

-- 
Jani Nikula, Intel Open Source Technology Center

[Bug 98901] amdgpu doesn't reinitialize after reboot in Xen PV DomU

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98901

--- Comment #3 from Mateusz Zalega  ---
(In reply to Alex Deucher from comment #1)
> Can you try a more recent kernel?  4.8 or 4.9?

I added a DomU startup log from a 4.9 series kernel. The result is the same in
4.8.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/4d596b00/attachment.html>

[Bug 98901] amdgpu doesn't reinitialize after reboot in Xen PV DomU

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98901

--- Comment #2 from Mateusz Zalega  ---
Created attachment 128271
  --> https://bugs.freedesktop.org/attachment.cgi?id=128271=edit
domU dmesg linux-4.9.0-rc7

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/c11b1b5b/attachment.html>

[Bug/RFC] drm/imx: warning: vblank wait timed out

[Philipp Zabel]

Hi Christopher,

Am Dienstag, den 29.11.2016, 16:45 +0100 schrieb Christopher Spinrath:
> Hi all,
> 
> I am trying to enable the second display pipeline of my imx6q based
> Utilite Pro. I appended a devicetree patch to this email for reference.
> 
> While it works perfectly on its own there occur kernel warnings (see
> below) if both pipelines are connected. Furthermore, the machine becomes
> almost unresponsive. Note that these warnings start to happen around the
> time getty is started. No other userspace stuff (e.g. X11) is involved.
> 
> I can make it work by adding the following hack to the devicetree:
> 
> display-subsystem {
>   compatible = "fsl,imx-display-subsystem";
>   ports = <_di0>, <_di0>, <_di1>, <_di1>;
> };
> 
> (i.e. reordering the ports of the display-subsystem.)
> 
> To my understanding the imx drm driver maps both outputs to ipu1 by
> default which cannot handle the load.

What is the load?

>  But using ipu1 and ipu2 for a
> single output each works. Unfortunately, I didn't find a way to model
> this restriction in the devicetree (in a sane way).

You can sever the links between HDMI encoder mux and IPU1 display
interfaces to keep it from being connected to IPU1 DI1:

/delete-node/_di0_hdmi;
/delete-node/_mux_0;

/delete-node/_di1_hdmi;
/delete-node/_mux_1;

> IMHO the driver should at least disable one of the outputs instead of
> rendering the whole system unusable. Optimally, it should figure out a
> correct mapping between outputs and ipu's itself.

It should. The problem with the IPU limitations is that they are
cross-crtc. The reference manual says that the maximum combined rate for
both display interfaces in an IPU is 240 MP/s, so we'd need to take into
account the pixel clocks of already enabled sibling crtcs when deciding
whether a mode can be set.

regards
Philipp

[Bug/RFC] drm/imx: warning: vblank wait timed out

[Christopher Spinrath]

Hi all,

I am trying to enable the second display pipeline of my imx6q based
Utilite Pro. I appended a devicetree patch to this email for reference.

While it works perfectly on its own there occur kernel warnings (see
below) if both pipelines are connected. Furthermore, the machine becomes
almost unresponsive. Note that these warnings start to happen around the
time getty is started. No other userspace stuff (e.g. X11) is involved.

I can make it work by adding the following hack to the devicetree:

display-subsystem {
compatible = "fsl,imx-display-subsystem";
ports = <_di0>, <_di0>, <_di1>, <_di1>;
};

(i.e. reordering the ports of the display-subsystem.)

To my understanding the imx drm driver maps both outputs to ipu1 by
default which cannot handle the load. But using ipu1 and ipu2 for a
single output each works. Unfortunately, I didn't find a way to model
this restriction in the devicetree (in a sane way).

IMHO the driver should at least disable one of the outputs instead of
rendering the whole system unusable. Optimally, it should figure out a
correct mapping between outputs and ipu's itself.

Any help would be much appreciated.

Cheers,
Christopher

The warning occurring is the following (I used a clean v4.9-rc5 branch
but copied the config of ArchlinuxARM which is why -ARCH+ got appended):

[6.546989] WARNING: CPU: 0 PID: 185 at 
drivers/gpu/drm/drm_atomic_helper.c:1140 
drm_atomic_helper_wait_for_vblanks+0x274/0x278
[6.547005] [CRTC:24] vblank wait timed out
[6.547118] Modules linked in: dw_hdmi_ahb_audio imx_ipuv3_crtc(+) caam_jr 
mwifiex_sdio(+) mwifiex cfg80211 btmrvl_sdio(+) btmrvl snd_soc_imx_spdif 
bluetooth imx_ipu_v3 rfkill ofpart caam snd_soc_fsl_spdif snd_soc_fsl_asrc 
imx_pcm_dma snd_soc_core snd_pcm_dmaengine snd_pcm spi_imx coda snd_timer 
videobuf2_dma_contig igb v4l2_mem2mem videobuf2_vmalloc videobuf2_memops 
videobuf2_v4l2 videobuf2_core imx2_wdt dw_hdmi_imx dw_hdmi etnaviv 
parallel_display uio_pdrv_genirq ti_tfp410 uio imxdrm evdev joydev mousedev 
sch_fq_codel ip_tables x_tables
[6.547128] CPU: 0 PID: 185 Comm: systemd-udevd Not tainted 4.9.0-rc5-ARCH+ 
#3
[6.547131] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[6.547154] [] (unwind_backtrace) from [] 
(show_stack+0x10/0x14)
[6.547167] [] (show_stack) from [] 
(dump_stack+0x88/0x9c)
[6.547181] [] (dump_stack) from [] (__warn+0xe8/0x100)
[6.547191] [] (__warn) from [] 
(warn_slowpath_fmt+0x48/0x6c)
[6.547204] [] (warn_slowpath_fmt) from [] 
(drm_atomic_helper_wait_for_vblanks+0x274/0x278)
[6.547228] [] (drm_atomic_helper_wait_for_vblanks) from 
[] (imx_drm_atomic_commit_tail+0x48/0x58 [imxdrm])
[6.547247] [] (imx_drm_atomic_commit_tail [imxdrm]) from 
[] (commit_tail+0x40/0x5c)
[6.547256] [] (commit_tail) from [] 
(drm_atomic_helper_commit+0x94/0xc8)
[6.547267] [] (drm_atomic_helper_commit) from [] 
(restore_fbdev_mode+0x148/0x298)
[6.547275] [] (restore_fbdev_mode) from [] 
(drm_fb_helper_restore_fbdev_mode_unlocked+0x30/0x74)
[6.547283] [] (drm_fb_helper_restore_fbdev_mode_unlocked) from 
[] (drm_fb_helper_set_par+0x30/0x60)
[6.547294] [] (drm_fb_helper_set_par) from [] 
(fbcon_init+0x4f8/0x544)
[6.547305] [] (fbcon_init) from [] 
(visual_init+0xc0/0x108)
[6.547316] [] (visual_init) from [] 
(do_bind_con_driver+0x1b0/0x37c)
[6.547323] [] (do_bind_con_driver) from [] 
(do_take_over_console+0x144/0x1b0)
[6.547331] [] (do_take_over_console) from [] 
(do_fbcon_takeover+0x70/0xcc)
[6.547346] [] (do_fbcon_takeover) from [] 
(notifier_call_chain+0x44/0x84)
[6.547355] [] (notifier_call_chain) from [] 
(__blocking_notifier_call_chain+0x48/0x60)
[6.547364] [] (__blocking_notifier_call_chain) from [] 
(blocking_notifier_call_chain+0x18/0x20)
[6.547373] [] (blocking_notifier_call_chain) from [] 
(register_framebuffer+0x1bc/0x2cc)
[6.547381] [] (register_framebuffer) from [] 
(drm_fb_helper_initial_config+0x2b4/0x480)
[6.547389] [] (drm_fb_helper_initial_config) from [] 
(drm_fbdev_cma_init_with_funcs+0x7c/0xf4)
[6.547397] [] (drm_fbdev_cma_init_with_funcs) from [] 
(drm_fbdev_cma_init+0x18/0x20)
[6.547409] [] (drm_fbdev_cma_init) from [] 
(imx_drm_bind+0xf4/0x184 [imxdrm])
[6.547423] [] (imx_drm_bind [imxdrm]) from [] 
(try_to_bring_up_master+0x148/0x184)
[6.547431] [] (try_to_bring_up_master) from [] 
(component_add+0x94/0x140)
[6.547439] [] (component_add) from [] 
(platform_drv_probe+0x50/0xb0)
[6.547449] [] (platform_drv_probe) from [] 
(driver_probe_device+0x204/0x2b0)
[6.547457] [] (driver_probe_device) from [] 
(__driver_attach+0xb8/0xbc)
[6.547465] [] (__driver_attach) from [] 
(bus_for_each_dev+0x7c/0xc0)
[6.547473] [] (bus_for_each_dev) from [] 
(bus_add_driver+0x108/0x214)
[6.547482] [] (bus_add_driver) from [] 
(driver_register+0x78/0xf4)
[6.547491] [] (driver_register) from [] 
(do_one_initcall+0x54/0x190)
[6.547501] [] (do_one_initcall) 

[PATCH v2 2/2] ARM: dts: da850-lcdk: specify the maximum pixel clock rate for tilcdc

[Sekhar Nori]

On Monday 28 November 2016 05:45 PM, Bartosz Golaszewski wrote:
> Due to memory throughput constraints any display mode for which the
> pixel clock rate exceeds the recommended value of 37500 KHz must be
> filtered out.

I think there might be more reasons than memory throughput constraints
for the reasoning behind 37.5Mhz cap on pixel clock. Why not just refer
to the datasheet section that places this constraint so we know its a
hardware restriction.

> 
> Specify the max-pixelclock property for the display node for
> da850-lcdk.
> 
> Signed-off-by: Bartosz Golaszewski 
> ---
>  arch/arm/boot/dts/da850-lcdk.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts 
> b/arch/arm/boot/dts/da850-lcdk.dts
> index d864f11..1283263 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -285,6 +285,7 @@
>  
>   {
>   status = "okay";
> + max-pixelclock = <37500>;

Should this not be in da850.dtsi since its an SoC imposed constraint? If
a board needs narrower constraint, it can override it. But I guess most
well designed boards will just hit the SoC constraint.

Thanks,
Sekhar

[PATCH] drm: Make the connector .detect() callback optional

[Sean Paul]

On Tue, Nov 29, 2016 at 3:56 PM, Laurent Pinchart
 wrote:
> Many drivers (21 to be exact) create connectors that are always
> connected (for instance to an LVDS or DSI panel). Instead of forcing
> them to implement a dummy .detect() handler, make the callback optional
> and consider the connector as always connected in that case.
>

I wonder if we should be a little bit smarter about this and default
connected only for built-in types (LVDS, edp, DSI), and return unknown
for others?

Sean


> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c |  7 ---
>  drivers/gpu/drm/arc/arcpgu_sim.c |  7 ---
>  drivers/gpu/drm/ast/ast_mode.c   |  7 ---
>  drivers/gpu/drm/bochs/bochs_kms.c|  7 ---
>  drivers/gpu/drm/bridge/nxp-ptn3460.c |  7 ---
>  drivers/gpu/drm/bridge/parade-ps8622.c   |  7 ---
>  drivers/gpu/drm/bridge/tc358767.c|  7 ---
>  drivers/gpu/drm/cirrus/cirrus_mode.c |  7 ---
>  drivers/gpu/drm/drm_probe_helper.c   | 14 +++---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c|  7 ---
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c  | 14 --
>  drivers/gpu/drm/gma500/psb_intel_lvds.c  | 14 --
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c |  7 ---
>  drivers/gpu/drm/i915/intel_dsi.c |  7 ---
>  drivers/gpu/drm/imx/imx-ldb.c|  7 ---
>  drivers/gpu/drm/imx/imx-tve.c|  7 ---
>  drivers/gpu/drm/imx/parallel-display.c   |  7 ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c   |  7 ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c   |  7 ---
>  drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c|  7 ---
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c   |  7 ---
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c|  7 ---
>  drivers/gpu/drm/sti/sti_hda.c|  7 ---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c|  7 ---
>  drivers/gpu/drm/sun4i/sun4i_tv.c |  7 ---
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c|  8 
>  include/drm/drm_connector.h  |  3 +++
>  27 files changed, 14 insertions(+), 193 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index 81cbf0b05dff..1d93e123532d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -429,12 +429,6 @@ dce_virtual_dpms(struct drm_connector *connector, int 
> mode)
> return 0;
>  }
>
> -static enum drm_connector_status
> -dce_virtual_detect(struct drm_connector *connector, bool force)
> -{
> -   return connector_status_connected;
> -}
> -
>  static int
>  dce_virtual_set_property(struct drm_connector *connector,
>  struct drm_property *property,
> @@ -463,7 +457,6 @@ static const struct drm_connector_helper_funcs 
> dce_virtual_connector_helper_func
>
>  static const struct drm_connector_funcs dce_virtual_connector_funcs = {
> .dpms = dce_virtual_dpms,
> -   .detect = dce_virtual_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .set_property = dce_virtual_set_property,
> .destroy = dce_virtual_destroy,
> diff --git a/drivers/gpu/drm/arc/arcpgu_sim.c 
> b/drivers/gpu/drm/arc/arcpgu_sim.c
> index 2bf06d71556a..bca3a678c955 100644
> --- a/drivers/gpu/drm/arc/arcpgu_sim.c
> +++ b/drivers/gpu/drm/arc/arcpgu_sim.c
> @@ -41,12 +41,6 @@ static int arcpgu_drm_connector_get_modes(struct 
> drm_connector *connector)
> return count;
>  }
>
> -static enum drm_connector_status
> -arcpgu_drm_connector_detect(struct drm_connector *connector, bool force)
> -{
> -   return connector_status_connected;
> -}
> -
>  static void arcpgu_drm_connector_destroy(struct drm_connector *connector)
>  {
> drm_connector_unregister(connector);
> @@ -61,7 +55,6 @@ arcpgu_drm_connector_helper_funcs = {
>  static const struct drm_connector_funcs arcpgu_drm_connector_funcs = {
> .dpms = drm_helper_connector_dpms,
> .reset = drm_atomic_helper_connector_reset,
> -   .detect = arcpgu_drm_connector_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .destroy = arcpgu_drm_connector_destroy,
> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 5957c3e659fe..e26c98f51eb4 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -839,12 +839,6 @@ static void ast_connector_destroy(struct drm_connector 
> *connector)
> kfree(connector);
>  }
>
> -static enum 

[PATCH 1/2] drm/i915/dsi: Fix swapping of MIPI_SEQ_DEASSERT_RESET / MIPI_SEQ_ASSERT_RESET

[Ville Syrjälä]

On Tue, Nov 29, 2016 at 02:06:20PM +0100, Hans de Goede wrote:
> Hi,
> 
> Thanks for the quick reply.
> 
> On 29-11-16 13:48, Ville Syrjälä wrote:
> > On Tue, Nov 29, 2016 at 01:38:57PM +0100, Hans de Goede wrote:
> >> Looking at the ADF code from the Android kernel sources for a
> >> cherrytrail tablet I noticed that it is calling the
> >> MIPI_SEQ_ASSERT_RESET sequence from the panel prepare hook.
> >>
> >> Until commit b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences
> >> in panel prepare/unprepare hooks") the mainline i915 code was doing the
> >> same. That commits effectively swaps the calling of MIPI_SEQ_ASSERT_RESET /
> >> MIPI_SEQ_DEASSERT_RESET.
> >>
> >> Looking at the naming of the sequences that is the right thing to do,
> >> but the problem is, that the old mainline code and the ADF code was
> >> actually calling the right sequence (tested on a cube iwork8 air tablet),
> >> and the swapping of the calling breaks things.
> >>
> >> This breakage was likely not noticed in testing because on cherrytrail,
> >> currently chv_exec_gpio ends up disabling the gpio pins rather then
> >> setting them (this is fixed in the next patch in this patch-set).
> >>
> >> This commit fixes the swapping by fixing MIPI_SEQ_ASSERT/DEASSERT_RESET's
> >> places in the enum defining them, so that their (new) names match their
> >> actual use.
> >>
> >> Fixes: b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences...")
> >> Cc: Jani Nikula 
> >> Cc: Ville Syrjälä 
> >> Signed-off-by: Hans de Goede 
> >> ---
> >>  drivers/gpu/drm/i915/intel_bios.h  | 4 ++--
> >>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 4 ++--
> >>  2 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_bios.h 
> >> b/drivers/gpu/drm/i915/intel_bios.h
> >> index 8405b5a..642a5eb 100644
> >> --- a/drivers/gpu/drm/i915/intel_bios.h
> >> +++ b/drivers/gpu/drm/i915/intel_bios.h
> >> @@ -49,11 +49,11 @@ struct edp_power_seq {
> >>  /* MIPI Sequence Block definitions */
> >>  enum mipi_seq {
> >>MIPI_SEQ_END = 0,
> >> -  MIPI_SEQ_ASSERT_RESET,
> >> +  MIPI_SEQ_DEASSERT_RESET,
> >>MIPI_SEQ_INIT_OTP,
> >>MIPI_SEQ_DISPLAY_ON,
> >>MIPI_SEQ_DISPLAY_OFF,
> >> -  MIPI_SEQ_DEASSERT_RESET,
> >> +  MIPI_SEQ_ASSERT_RESET,
> >
> > That naming would be against the spec, so I don't think we want to do it
> > like this. Unless the spec is totally wrong, that is.
> 
> Given that both the ADF code and the i915 driver until now have been using
> assert on prepare and deassert on unprepare I'm inclined to believe that
> the spec is wrong. Is the spec available somewhere public ?

I don't think so. And sadly even if it would it wouldn't really help
since about the only thing it says is:

00 – Reserved
01 - MIPIAssertResetPin
02 – MIPISendInitialDcsCmds (Use this sequence type for sending 
initialization commands in LP mode)
03 - MIPIDisplayOn (Use this sequence type for sending initialization commands 
in HS mode)
04 – MIPIDisplayOff (Use this sequence type for sending DisplayOff commands 
in LP mode)
05 – MIPIDeassertResetPin
06 – MIPIBacklightOn
07 - MIPIBacklightOff
08 – MIPITearOn
09 - MIPITearOff
10 - MIPIPanelPowerOn
11 - MIPIPanelPowerOff
Others – Reserved

So pretty much useless if you actually want to write a working driver.

> 
> Also if you look at the v1 sequences with the new names:
> 
>   MIPI_SEQ_DEASSERT_RESET,
>   MIPI_SEQ_INIT_OTP,
>   MIPI_SEQ_DISPLAY_ON,
>   MIPI_SEQ_DISPLAY_OFF,
>   MIPI_SEQ_ASSERT_RESET,
> 
> Then they are exactly in the order as one would call them on
> enable, followed by disable, which I believe is not a coincidence.
> 
> > Can you provide the VBT for the affected machine so other people can
> > have a look at it?
> 
> Sure I can do that, what would be the easiest way (both for me to
> produce and for you to consume) to do this ?

/sys/kernel/debug/dri/0/i915_opregion

For the best chance of preserving the dump for posterity I would
suggest filing a new bug and attaching it there.

https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/Intel

> 
> While developing this set, I've added printk calls to the code executing the
> sequences, there are 2 gpios involved nr 60 (backlight enable AFAICT, also 
> used
> by the BACKLIGHT sequences) and 72 (reset / panel_enable ?).
> When efifb is up both are 1 / high.
> 
> With the OLD naming, MIPI_SEQ_ASSERT_RESET does:
> 
> gpio 72 high
> delay
> gpio 72 low
> delay
> gpio 72 high

Hmm. OK so it just toggles the reset pin it seems.

> 
> And DEASSERT does:
> 
> gpio 72 low
> gpio 60 low

And this leaves the reset pin asserted, assuming it's active low,
which your patch would seem to confirm.

> 
> So with the old naming DEASSERT leaves the panel disabled / in reset and
> the backlight disabled, which is exactly what we do not want...

Right. Hmm. If we do flip them over like you suggest I think we'll at
least need a big comment to inform people why we seem to go 

[PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

[Archit Taneja]

On 11/29/2016 02:34 PM, Laurent Pinchart wrote:
> Most drivers that use bridges forgot to detach them at cleanup time.
> Instead of fixing them one by one, detach the bridge in the core
> drm_encoder_cleanup() function.
>
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/drm_encoder.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 5c067719164d..9c1f99646e0d 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
>* the indices on the drm_encoder after us in the encoder_list.
>*/
>
> + if (encoder->bridge)
> + drm_bridge_detach(encoder->bridge);

This would require the kms driver to still detach the remaining
n - 1 bridges in a possible chain. We could probably detach all of
them here, or maybe leave detaching of all to the kms driver, and just
report a warning here.

Archit

> +
>   drm_modeset_lock_all(dev);
>   drm_mode_object_unregister(dev, >base);
>   kfree(encoder->name);
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 03/13] drm: bridge: Link encoder and bridge in core code

[Archit Taneja]

On 11/29/2016 02:34 PM, Laurent Pinchart wrote:
> Instead of linking encoders and bridges in every driver (and getting it
> wrong half of the time, as many drivers forget to set the drm_bridge
> encoder pointer), do so in core code. The drm_bridge_attach() function
> needs the encoder and optional previous bridge to perform that task,
> update all the callers.
>
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c   |  4 +-
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  4 +-
>  drivers/gpu/drm/bridge/dw-hdmi.c   |  3 +-
>  drivers/gpu/drm/drm_bridge.c   | 46 
> --
>  drivers/gpu/drm/drm_simple_kms_helper.c|  4 +-
>  drivers/gpu/drm/exynos/exynos_dp.c |  5 +--
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c|  6 +--
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  |  5 +--
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c   |  5 +--
>  drivers/gpu/drm/imx/imx-ldb.c  |  6 +--
>  drivers/gpu/drm/imx/parallel-display.c |  4 +-
>  drivers/gpu/drm/mediatek/mtk_dpi.c |  8 ++--
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 24 ++-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c| 11 +++---
>  drivers/gpu/drm/msm/dsi/dsi_manager.c  | 17 +---
>  drivers/gpu/drm/msm/edp/edp_bridge.c   |  2 +-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c  |  5 +--
>  drivers/gpu/drm/sti/sti_dvo.c  |  3 +-
>  drivers/gpu/drm/sti/sti_hda.c  |  3 +-
>  drivers/gpu/drm/sti/sti_hdmi.c |  3 +-
>  drivers/gpu/drm/sun4i/sun4i_rgb.c  | 13 +++---
>  include/drm/drm_bridge.h   |  3 +-
>  23 files changed, 83 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> index 6119b5085501..e7799b6ee829 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c
> @@ -230,9 +230,7 @@ static int atmel_hlcdc_attach_endpoint(struct drm_device 
> *dev,
>   of_node_put(np);
>
>   if (bridge) {
> - output->encoder.bridge = bridge;
> - bridge->encoder = >encoder;
> - ret = drm_bridge_attach(dev, bridge);
> + ret = drm_bridge_attach(>encoder, bridge, NULL);
>   if (!ret)
>   return 0;
>   }
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 6e0447f329a2..1835f1fdad19 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1227,12 +1227,10 @@ static int analogix_dp_create_bridge(struct 
> drm_device *drm_dev,
>
>   dp->bridge = bridge;
>
> - dp->encoder->bridge = bridge;
>   bridge->driver_private = dp;
> - bridge->encoder = dp->encoder;
>   bridge->funcs = _dp_bridge_funcs;
>
> - ret = drm_bridge_attach(drm_dev, bridge);
> + ret = drm_bridge_attach(dp->encoder, bridge, NULL);
>   if (ret) {
>   DRM_ERROR("failed to attach drm bridge\n");
>   return -EINVAL;
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/dw-hdmi.c
> index b71088dab268..432e0e3fff72 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -1841,13 +1841,12 @@ static int dw_hdmi_register(struct drm_device *drm, 
> struct dw_hdmi *hdmi)
>   hdmi->bridge = bridge;
>   bridge->driver_private = hdmi;
>   bridge->funcs = _hdmi_bridge_funcs;
> - ret = drm_bridge_attach(drm, bridge);
> + ret = drm_bridge_attach(encoder, bridge, NULL);
>   if (ret) {
>   DRM_ERROR("Failed to initialize bridge with drm\n");
>   return -EINVAL;
>   }
>
> - encoder->bridge = bridge;
>   hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
>
>   drm_connector_helper_add(>connector,
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 0ee052b7c21a..850bd6509ef1 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -26,6 +26,7 @@
>  #include 
>
>  #include 
> +#include 
>
>  /**
>   * DOC: overview
> @@ -92,32 +93,53 @@ void drm_bridge_remove(struct drm_bridge *bridge)
>  EXPORT_SYMBOL(drm_bridge_remove);
>
>  /**
> - * drm_bridge_attach - associate given bridge to our DRM device
> + * drm_bridge_attach - attach the bridge to an encoder's chain
>   *
> - * @dev: DRM device
> - * @bridge: bridge control structure
> + * @encoder: DRM encoder
> + * @bridge: bridge to attach
> + * @previous: previous bridge in the chain (optional)
>   *
> 

[Intel-gfx] [PATCH v2 3/3] drm: Protect fb_helper list manipulation with a mutex

[Sean Paul]

On Tue, Nov 29, 2016 at 7:02 AM, Chris Wilson  
wrote:
> Though we only walk the kernel_fb_helper_list inside a panic (or single
> thread debugging), we still need to protect the list manipulation on
> creating/removing a framebuffer device in order to prevent list
> corruption.
>
> Signed-off-by: Chris Wilson 
> Reviewed-by: Daniel Vetter 

Applied to drm-misc

Sean

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 90da28d2fcf3..e934b541feea 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -49,6 +49,7 @@ MODULE_PARM_DESC(fbdev_emulation,
>  "Enable legacy fbdev emulation [default=true]");
>
>  static LIST_HEAD(kernel_fb_helper_list);
> +static DEFINE_MUTEX(kernel_fb_helper_lock);
>
>  /**
>   * DOC: fbdev helpers
> @@ -855,12 +856,14 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
> if (!drm_fbdev_emulation)
> return;
>
> +   mutex_lock(_fb_helper_lock);
> if (!list_empty(_helper->kernel_fb_list)) {
> list_del(_helper->kernel_fb_list);
> if (list_empty(_fb_helper_list)) {
> unregister_sysrq_key('v', 
> _drm_fb_helper_restore_op);
> }
> }
> +   mutex_unlock(_fb_helper_lock);
>
> drm_fb_helper_crtc_free(fb_helper);
>
> @@ -2258,10 +2261,12 @@ int drm_fb_helper_initial_config(struct drm_fb_helper 
> *fb_helper, int bpp_sel)
> dev_info(dev->dev, "fb%d: %s frame buffer device\n",
>  info->node, info->fix.id);
>
> +   mutex_lock(_fb_helper_lock);
> if (list_empty(_fb_helper_list))
> register_sysrq_key('v', _drm_fb_helper_restore_op);
>
> list_add(_helper->kernel_fb_list, _fb_helper_list);
> +   mutex_unlock(_fb_helper_lock);
>
> return 0;
>  }
> --
> 2.10.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/3] drm: Pull together probe + setup for drm_fb_helper

[Sean Paul]

On Tue, Nov 29, 2016 at 3:29 PM, Daniel Vetter  wrote:
> On Tue, Nov 29, 2016 at 12:02:16PM +, Chris Wilson wrote:
>> drm_fb_helper_probe_connector_modes() is always called before
>> drm_setup_crtcs(), so just move the call into drm_setup_crtcs for a
>> small bit of code compaction.
>>
>> Signed-off-by: Chris Wilson 
>> Reviewed-by: Daniel Vetter 
>
> rb not entirely correct, since it's missing the crucial note I asked for
> to understand things:
>
> "Note that register_framebuffer will do a modeset (when fbcon is enabled)
> and hence must be moved out of the critical section. A follow-up patch
> will add new locking for the fb list, hence move all the related
> registration code together."
>
> Sean, since you reviewed all, can you pls add this note to the commit
> message when merging?
>

Applied to misc with the added note.

Sean


> Thanks, Daniel
>
>> ---
>>  drivers/gpu/drm/drm_fb_helper.c | 37 +++--
>>  1 file changed, 11 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 380a0ec01033..90da28d2fcf3 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -2099,20 +2099,19 @@ static int drm_pick_crtcs(struct drm_fb_helper 
>> *fb_helper,
>>   return best_score;
>>  }
>>
>> -static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
>> +static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>> + u32 width, u32 height)
>>  {
>>   struct drm_device *dev = fb_helper->dev;
>>   struct drm_fb_helper_crtc **crtcs;
>>   struct drm_display_mode **modes;
>>   struct drm_fb_offset *offsets;
>>   bool *enabled;
>> - int width, height;
>>   int i;
>>
>>   DRM_DEBUG_KMS("\n");
>> -
>> - width = dev->mode_config.max_width;
>> - height = dev->mode_config.max_height;
>> + if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
>> + DRM_DEBUG_KMS("No connectors reported connected with modes\n");
>>
>>   /* prevent concurrent modification of connector_count by hotplug */
>>   lockdep_assert_held(_helper->dev->mode_config.mutex);
>> @@ -2236,23 +2235,15 @@ int drm_fb_helper_initial_config(struct 
>> drm_fb_helper *fb_helper, int bpp_sel)
>>  {
>>   struct drm_device *dev = fb_helper->dev;
>>   struct fb_info *info;
>> - int count = 0;
>>   int ret;
>>
>>   if (!drm_fbdev_emulation)
>>   return 0;
>>
>>   mutex_lock(>mode_config.mutex);
>> - count = drm_fb_helper_probe_connector_modes(fb_helper,
>> - dev->mode_config.max_width,
>> - 
>> dev->mode_config.max_height);
>> - /*
>> -  * we shouldn't end up with no modes here.
>> -  */
>> - if (count == 0)
>> - dev_info(fb_helper->dev->dev, "No connectors reported 
>> connected with modes\n");
>> -
>> - drm_setup_crtcs(fb_helper);
>> + drm_setup_crtcs(fb_helper,
>> + dev->mode_config.max_width,
>> + dev->mode_config.max_height);
>>   ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
>>   mutex_unlock(>mode_config.mutex);
>>   if (ret)
>> @@ -2300,28 +2291,22 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
>>  int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>>  {
>>   struct drm_device *dev = fb_helper->dev;
>> - u32 max_width, max_height;
>>
>>   if (!drm_fbdev_emulation)
>>   return 0;
>>
>> - mutex_lock(_helper->dev->mode_config.mutex);
>> + mutex_lock(>mode_config.mutex);
>>   if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
>>   fb_helper->delayed_hotplug = true;
>> - mutex_unlock(_helper->dev->mode_config.mutex);
>> + mutex_unlock(>mode_config.mutex);
>>   return 0;
>>   }
>>   DRM_DEBUG_KMS("\n");
>>
>> - max_width = fb_helper->fb->width;
>> - max_height = fb_helper->fb->height;
>> + drm_setup_crtcs(fb_helper, fb_helper->fb->width, 
>> fb_helper->fb->height);
>>
>> - drm_fb_helper_probe_connector_modes(fb_helper, max_width, max_height);
>> - mutex_unlock(_helper->dev->mode_config.mutex);
>> + mutex_unlock(>mode_config.mutex);
>>
>> - drm_modeset_lock_all(dev);
>> - drm_setup_crtcs(fb_helper);
>> - drm_modeset_unlock_all(dev);
>>   drm_fb_helper_set_par(fb_helper->fbdev);
>>
>>   return 0;
>> --
>> 2.10.2
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Intel-gfx] [PATCH v2 1/3] drm: Hold mode_config.lock to prevent hotplug whilst setting up crtcs

[Sean Paul]

On Tue, Nov 29, 2016 at 7:02 AM, Chris Wilson  
wrote:
> The fb_helper->connector_count is modified when a new connector is
> constructed following a hotplug event (e.g. DP-MST). This causes trouble
> for drm_setup_crtcs() and friends that assume that fb_helper is
> constant:
>
> [ 1250.872997] BUG: KASAN: slab-out-of-bounds in drm_setup_crtcs+0x320/0xf80 
> at addr 88074cdd2608
> [ 1250.873020] Write of size 40 by task kworker/u8:3/480
> [ 1250.873039] CPU: 2 PID: 480 Comm: kworker/u8:3 Tainted: G U  
> 4.9.0-rc6+ #285
> [ 1250.873043] Hardware name:  /NUC6i3SYB, BIOS 
> SYSKLi35.86A.0024.2015.1027.2142 10/27/2015
> [ 1250.873050] Workqueue: events_unbound async_run_entry_fn
> [ 1250.873056]  88070f9d78f0 814b72aa 88074e40c5c0 
> 88074cdd2608
> [ 1250.873067]  88070f9d7918 8124ff3c 88070f9d79b0 
> 88074cdd2600
> [ 1250.873079]  88074e40c5c0 88070f9d79a0 812501e4 
> 0005
> [ 1250.873090] Call Trace:
> [ 1250.873099]  [] dump_stack+0x67/0x9d
> [ 1250.873106]  [] kasan_object_err+0x1c/0x70
> [ 1250.873113]  [] kasan_report_error+0x204/0x4f0
> [ 1250.873120]  [] ? drm_dev_printk+0x140/0x140
> [ 1250.873127]  [] kasan_report+0x53/0x60
> [ 1250.873134]  [] ? drm_setup_crtcs+0x320/0xf80
> [ 1250.873142]  [] check_memory_region+0x13e/0x1a0
> [ 1250.873147]  [] memset+0x23/0x40
> [ 1250.873154]  [] drm_setup_crtcs+0x320/0xf80
> [ 1250.873161]  [] ? wake_up_q+0x45/0x80
> [ 1250.873169]  [] ? mutex_lock_nested+0x5a0/0x5a0
> [ 1250.873176]  [] drm_fb_helper_initial_config+0x206/0x7a0
> [ 1250.873183]  [] ? drm_fb_helper_set_par+0x90/0x90
> [ 1250.873303]  [] ? intel_fbdev_fini+0x140/0x140 [i915]
> [ 1250.873387]  [] intel_fbdev_initial_config+0x22/0x40 
> [i915]
> [ 1250.873391]  [] async_run_entry_fn+0x7f/0x270
> [ 1250.873394]  [] process_one_work+0x3d0/0x960
> [ 1250.873398]  [] ? process_one_work+0x33d/0x960
> [ 1250.873401]  [] ? max_active_store+0xf0/0xf0
> [ 1250.873406]  [] ? do_raw_spin_lock+0x10d/0x1a0
> [ 1250.873413]  [] worker_thread+0x8d/0x840
> [ 1250.873419]  [] ? create_worker+0x2e0/0x2e0
> [ 1250.873426]  [] kthread+0x194/0x1c0
> [ 1250.873432]  [] ? kthread_park+0x60/0x60
> [ 1250.873438]  [] ? trace_hardirqs_on+0xd/0x10
> [ 1250.873446]  [] ? kthread_park+0x60/0x60
> [ 1250.873453]  [] ? kthread_park+0x60/0x60
> [ 1250.873457]  [] ret_from_fork+0x27/0x40
> [ 1250.873460] Object at 88074cdd2608, in cache kmalloc-32 size: 32
>
> However, when holding the mode_config.lock around the fb_helper, we have
> to be careful of any callbacks that may reenter the fb_helper and so try
> to reacquire the mode_config.lock (e.g. register_framebuffer). To avoid
> the mutex recursion, we have to rearrange the sequence to move the
> registration into the caller outside of the mode_config.lock.
>
> v2: drop the 1; following the lockdep assertion inside the for(;;), I
> anticipated an error that doesn't happen!
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98826
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 

Applied to drm-misc

Sean

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 73 
> ++---
>  1 file changed, 40 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 1f26634f53d8..380a0ec01033 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -97,6 +97,10 @@ static LIST_HEAD(kernel_fb_helper_list);
>   * mmap page writes.
>   */
>
> +#define drm_fb_helper_for_each_connector(fbh, i__) \
> +   for (({ lockdep_assert_held(&(fbh)->dev->mode_config.mutex); }), \
> +i__ = 0; i__ < (fbh)->connector_count; i__++)
> +
>  /**
>   * drm_fb_helper_single_add_all_connectors() - add all connectors to fbdev
>   *emulation helper
> @@ -130,7 +134,7 @@ int drm_fb_helper_single_add_all_connectors(struct 
> drm_fb_helper *fb_helper)
> mutex_unlock(>mode_config.mutex);
> return 0;
>  fail:
> -   for (i = 0; i < fb_helper->connector_count; i++) {
> +   drm_fb_helper_for_each_connector(fb_helper, i) {
> struct drm_fb_helper_connector *fb_helper_connector =
> fb_helper->connector_info[i];
>
> @@ -565,7 +569,7 @@ static void drm_fb_helper_dpms(struct fb_info *info, int 
> dpms_mode)
> continue;
>
> /* Walk the connectors & encoders on this fb turning them 
> on/off */
> -   for (j = 0; j < fb_helper->connector_count; j++) {
> +   drm_fb_helper_for_each_connector(fb_helper, j) {
> connector = fb_helper->connector_info[j]->connector;
> connector->funcs->dpms(connector, dpms_mode);
> drm_object_property_set_value(>base,
> @@ -1469,7 +1473,6 @@ static int drm_fb_helper_single_fb_probe(struct 
> drm_fb_helper *fb_helper,
> 

[Bug 98903] [AMDGPU] System hang with latest CS:GO update

[bugzilla-dae...@freedesktop.org]

[]
amdgpu_gem_va_ioctl+0x21f/0x2e0 [amdgpu]
Nov 29 16:39:51 mammut kernel: [ 1209.379547]  []
drm_ioctl+0x21b/0x4c0 [drm]
Nov 29 16:39:51 mammut kernel: [ 1209.379565]  [] ?
amdgpu_gem_metadata_ioctl+0x1d0/0x1d0 [amdgpu]
Nov 29 16:39:51 mammut kernel: [ 1209.379580]  []
amdgpu_drm_ioctl+0x4f/0x90 [amdgpu]
Nov 29 16:39:51 mammut kernel: [ 1209.379583]  []
do_vfs_ioctl+0xa3/0x600
Nov 29 16:39:51 mammut kernel: [ 1209.379586]  [] ?
__do_page_fault+0x266/0x4e0
Nov 29 16:39:51 mammut kernel: [ 1209.379589]  []
SyS_ioctl+0x79/0x90
Nov 29 16:39:51 mammut kernel: [ 1209.379592]  []
entry_SYSCALL_64_fastpath+0x1e/0xad
Nov 29 16:41:16 mammut kernel: [ 1294.482836] sysrq: SysRq : Emergency Sync
Nov 29 16:41:16 mammut kernel: [ 1294.484975] Emergency Sync complete
Nov 29 16:41:17 mammut kernel: [ 1296.178828] sysrq: SysRq : Emergency Remount
R/O

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/c8e3e886/attachment-0001.html>

[Bug 96868] AMDGPU Tonga only does 2560x1440 at 120hz, switching to 144hz causes display errors, same thing used to happen with fglrx.

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=96868

--- Comment #12 from iuno at posteo.net ---
Yeah, forcing high clock rates seem to fix the flickering issue.

The problem is actually the memory clock. However, today, without my
intervention, the memory clock stays at max (1250 MHz) for high refresh rates,
while the core clock still goes down to 300, as it should. So this god "fixed"
indeed.

I still have the 144 Hz problem with corrupt image, though.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/81b1c252/attachment.html>

[Intel-gfx] [PATCH v2 2/3] drm: Pull together probe + setup for drm_fb_helper

[Chris Wilson]

On Tue, Nov 29, 2016 at 10:28:03AM -0500, Sean Paul wrote:
> On Tue, Nov 29, 2016 at 7:02 AM, Chris Wilson  
> wrote:
> > drm_fb_helper_probe_connector_modes() is always called before
> > drm_setup_crtcs(), so just move the call into drm_setup_crtcs for a
> > small bit of code compaction.
> >
> > Signed-off-by: Chris Wilson 
> > Reviewed-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c | 37 +++--
> >  1 file changed, 11 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> > b/drivers/gpu/drm/drm_fb_helper.c
> > index 380a0ec01033..90da28d2fcf3 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2099,20 +2099,19 @@ static int drm_pick_crtcs(struct drm_fb_helper 
> > *fb_helper,
> > return best_score;
> >  }
> >
> > -static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
> > +static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> > +   u32 width, u32 height)
> >  {
> > struct drm_device *dev = fb_helper->dev;
> > struct drm_fb_helper_crtc **crtcs;
> > struct drm_display_mode **modes;
> > struct drm_fb_offset *offsets;
> > bool *enabled;
> > -   int width, height;
> > int i;
> >
> > DRM_DEBUG_KMS("\n");
> > -
> > -   width = dev->mode_config.max_width;
> > -   height = dev->mode_config.max_height;
> > +   if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) 
> > == 0)
> > +   DRM_DEBUG_KMS("No connectors reported connected with 
> > modes\n");
> >
> > /* prevent concurrent modification of connector_count by hotplug */
> > lockdep_assert_held(_helper->dev->mode_config.mutex);
> > @@ -2236,23 +2235,15 @@ int drm_fb_helper_initial_config(struct 
> > drm_fb_helper *fb_helper, int bpp_sel)
> >  {
> > struct drm_device *dev = fb_helper->dev;
> > struct fb_info *info;
> > -   int count = 0;
> > int ret;
> >
> > if (!drm_fbdev_emulation)
> > return 0;
> >
> > mutex_lock(>mode_config.mutex);
> > -   count = drm_fb_helper_probe_connector_modes(fb_helper,
> > -   
> > dev->mode_config.max_width,
> > -   
> > dev->mode_config.max_height);
> > -   /*
> > -* we shouldn't end up with no modes here.
> > -*/
> > -   if (count == 0)
> > -   dev_info(fb_helper->dev->dev, "No connectors reported 
> > connected with modes\n");
> > -
> > -   drm_setup_crtcs(fb_helper);
> > +   drm_setup_crtcs(fb_helper,
> > +   dev->mode_config.max_width,
> > +   dev->mode_config.max_height);
> > ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
> > mutex_unlock(>mode_config.mutex);
> > if (ret)
> > @@ -2300,28 +2291,22 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
> >  int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> >  {
> > struct drm_device *dev = fb_helper->dev;
> > -   u32 max_width, max_height;
> >
> > if (!drm_fbdev_emulation)
> > return 0;
> >
> > -   mutex_lock(_helper->dev->mode_config.mutex);
> > +   mutex_lock(>mode_config.mutex);
> > if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
> > fb_helper->delayed_hotplug = true;
> > -   mutex_unlock(_helper->dev->mode_config.mutex);
> > +   mutex_unlock(>mode_config.mutex);
> > return 0;
> > }
> > DRM_DEBUG_KMS("\n");
> >
> > -   max_width = fb_helper->fb->width;
> > -   max_height = fb_helper->fb->height;
> > +   drm_setup_crtcs(fb_helper, fb_helper->fb->width, 
> > fb_helper->fb->height);
> >
> > -   drm_fb_helper_probe_connector_modes(fb_helper, max_width, 
> > max_height);
> > -   mutex_unlock(_helper->dev->mode_config.mutex);
> > +   mutex_unlock(>mode_config.mutex);
> >
> > -   drm_modeset_lock_all(dev);
> 
> Stupid question: Why don't we need to do this any longer?

We don't do (never did) any modesets along this path, so locking
everything is overkill and misleading, i.e. I was trimming it down to
the locking as required by setup_crtc which is just that the config is
stable.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] dim: Add status command

[Daniel Vetter]

Useful with our branch proliferation to make sure nothing is stuck (we
now also have drm-misc-next/-next-fixes/-fixes).

v2: Gracefully handle if some remotes arent' there.

Acked-by: seanpaul at chromium.org
Signed-off-by: Daniel Vetter 
---
 dim | 22 ++
 dim.rst |  5 +
 2 files changed, 27 insertions(+)

diff --git a/dim b/dim
index 3dd2d1dec796..b21d1321af09 100755
--- a/dim
+++ b/dim
@@ -1285,6 +1285,28 @@ function dim_update_branches
update_rerere_cache
 }

+function dim_status
+{
+   local repo remote drm_remote patches
+
+   cd $DIM_PREFIX/$DIM_DRM_INTEL
+
+   drm_remote=$(url_to_remote $drm_upstream_git)
+
+   for branch in $dim_branches ; do
+   repo=`branch_to_repo $branch`
+   if ! remote=`repo_to_remote $repo` ; then
+   continue
+   fi
+
+   patches=$(git log --oneline $remote/$branch ^origin/master 
^$drm_remote/drm-next ^$drm_remote/drm-fixes | wc -l)
+
+   if [[ $patches -ne 0 ]] ; then
+   echo $repo/$branch: $patches unmerged patches
+   fi
+   done
+}
+
 function setup_aux_checkout # name url directory
 {
local name url dir remote
diff --git a/dim.rst b/dim.rst
index 9a045a43e7b7..0c52ab43e400 100644
--- a/dim.rst
+++ b/dim.rst
@@ -80,6 +80,11 @@ always maintained from the same machine, even if different 
branches are
 maintained on different machines (by different maintainers), there's no need to
 run this command.

+status
+--
+
+Lists all branches with unmerged patches, and how many patches are unmerged.
+
 rebuild-tip
 ---
 Rebuild and push the integration tree.
-- 
2.10.2

[PATCH v7 1/8] drm: sun8i: Add a basic DRM driver for Allwinner DE2

[Daniel Vetter]

On Mon, Nov 28, 2016 at 03:23:54PM +0100, Jean-Francois Moine wrote:
> Allwinner's recent SoCs, as A64, A83T and H3, contain a new display
> engine, DE2.
> This patch adds a DRM video driver for this device.
> 
> Signed-off-by: Jean-Francois Moine 

Scrolled around a bit, seemed all reasonable.

Acked-by: Daniel Vetter 

Not sure a new driver for each chip is reasonable, experience says that
long-term you want to share quite a pile of code between different hw
platforms from the same vendor. But that's entirely up to you.
-Daniel


> ---
>  drivers/gpu/drm/Kconfig   |   2 +
>  drivers/gpu/drm/Makefile  |   1 +
>  drivers/gpu/drm/sun8i/Kconfig |  19 +
>  drivers/gpu/drm/sun8i/Makefile|   7 +
>  drivers/gpu/drm/sun8i/de2_crtc.c  | 449 +++
>  drivers/gpu/drm/sun8i/de2_crtc.h  |  50 +++
>  drivers/gpu/drm/sun8i/de2_drv.c   | 317 
>  drivers/gpu/drm/sun8i/de2_drv.h   |  48 +++
>  drivers/gpu/drm/sun8i/de2_plane.c | 734 
> ++
>  9 files changed, 1627 insertions(+)
>  create mode 100644 drivers/gpu/drm/sun8i/Kconfig
>  create mode 100644 drivers/gpu/drm/sun8i/Makefile
>  create mode 100644 drivers/gpu/drm/sun8i/de2_crtc.c
>  create mode 100644 drivers/gpu/drm/sun8i/de2_crtc.h
>  create mode 100644 drivers/gpu/drm/sun8i/de2_drv.c
>  create mode 100644 drivers/gpu/drm/sun8i/de2_drv.h
>  create mode 100644 drivers/gpu/drm/sun8i/de2_plane.c
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 95fc041..bb1bfbc 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -202,6 +202,8 @@ source "drivers/gpu/drm/shmobile/Kconfig"
>  
>  source "drivers/gpu/drm/sun4i/Kconfig"
>  
> +source "drivers/gpu/drm/sun8i/Kconfig"
> +
>  source "drivers/gpu/drm/omapdrm/Kconfig"
>  
>  source "drivers/gpu/drm/tilcdc/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 883f3e7..3e1eaa0 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -72,6 +72,7 @@ obj-$(CONFIG_DRM_RCAR_DU) += rcar-du/
>  obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/
>  obj-y+= omapdrm/
>  obj-$(CONFIG_DRM_SUN4I) += sun4i/
> +obj-$(CONFIG_DRM_SUN8I) += sun8i/
>  obj-y+= tilcdc/
>  obj-$(CONFIG_DRM_QXL) += qxl/
>  obj-$(CONFIG_DRM_BOCHS) += bochs/
> diff --git a/drivers/gpu/drm/sun8i/Kconfig b/drivers/gpu/drm/sun8i/Kconfig
> new file mode 100644
> index 000..6940895
> --- /dev/null
> +++ b/drivers/gpu/drm/sun8i/Kconfig
> @@ -0,0 +1,19 @@
> +#
> +# Allwinner DE2 Video configuration
> +#
> +
> +config DRM_SUN8I
> + bool
> +
> +config DRM_SUN8I_DE2
> + tristate "Support for Allwinner Video with DE2 interface"
> + depends on DRM && OF
> + depends on ARCH_SUNXI || COMPILE_TEST
> + select DRM_GEM_CMA_HELPER
> + select DRM_KMS_CMA_HELPER
> + select DRM_KMS_HELPER
> + select DRM_SUN8I
> + help
> +   Choose this option if your Allwinner chipset has the DE2 interface
> +   as the A64, A83T and H3. If M is selected the module will be called
> +   sun8i-de2-drm.
> diff --git a/drivers/gpu/drm/sun8i/Makefile b/drivers/gpu/drm/sun8i/Makefile
> new file mode 100644
> index 000..f107919
> --- /dev/null
> +++ b/drivers/gpu/drm/sun8i/Makefile
> @@ -0,0 +1,7 @@
> +#
> +# Makefile for Allwinner's sun8i DRM device driver
> +#
> +
> +sun8i-de2-drm-objs := de2_drv.o de2_crtc.o de2_plane.o
> +
> +obj-$(CONFIG_DRM_SUN8I_DE2) += sun8i-de2-drm.o
> diff --git a/drivers/gpu/drm/sun8i/de2_crtc.c 
> b/drivers/gpu/drm/sun8i/de2_crtc.c
> new file mode 100644
> index 000..4e94ccc
> --- /dev/null
> +++ b/drivers/gpu/drm/sun8i/de2_crtc.c
> @@ -0,0 +1,449 @@
> +/*
> + * Allwinner DRM driver - DE2 CRTC
> + *
> + * Copyright (C) 2016 Jean-Francois Moine 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "de2_drv.h"
> +#include "de2_crtc.h"
> +
> +/* I/O map */
> +
> +#define TCON_GCTL_REG0x00
> +#define  TCON_GCTL_TCON_ENABLE BIT(31)
> +#define TCON_GINT0_REG   0x04
> +#define  TCON_GINT0_TCON1_Vb_Int_En BIT(30)
> +#define  TCON_GINT0_TCON1_Vb_Int_Flag BIT(14)
> +#define  TCON_GINT0_TCON1_Vb_Line_Int_Flag BIT(12)
> +#define TCON0_CTL_REG0x40
> +#define  TCON0_CTL_TCON_ENABLE BIT(31)
> +#define TCON1_CTL_REG0x90
> +#define  TCON1_CTL_TCON_ENABLE BIT(31)
> +#define  TCON1_CTL_INTERLACE_ENABLE BIT(20)
> +#define  TCON1_CTL_Start_Delay_SHIFT 4
> +#define  TCON1_CTL_Start_Delay_MASK GENMASK(8, 4)
> +#define TCON1_BASIC0_REG 0x94

[PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Jernej Skrabec]

Hi Laurent,

Dne torek, 29. november 2016 23.56.31 UTC+1 je oseba Laurent Pinchart 
napisala:
>
> Hi Jernej, 
>
> (CC'ing Kieran Bingham) 
>
> On Tuesday 29 Nov 2016 14:47:20 Jernej Skrabec wrote: 
> > Dne torek, 29. november 2016 22.37.03 UTC+1 je oseba Maxime Ripard 
> napisala: 
> > > On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote: 
> > >> This patchset series adds HDMI video support to the Allwinner 
> > >> sun8i SoCs which include the display engine 2 (DE2). 
> > >> The driver contains the code for the A83T and H3 SoCs, and 
> > >> some H3 boards, but it could be used/extended for other SoCs 
> > >> (A64, H2, H5) and boards (Banana PIs, Orange PIs). 
> > > 
> > > Honestly, I'm getting a bit worried by the fact that you ignore 
> > > reviews. 
> > > 
> > > On the important reviews that you got that are to be seen as major 
> > > 
> > > issues that block the inclusion, we have: 
> > >   - The fact that the HDMI driver is actually just a designware IP, 
> > > and while you should use the driver that already exists, you just 
> > > duplicated all that code. 
> > 
> > That might be hard thing to do. A83T fits perfectly, but H3 and newer 
> SoCs 
> > do not. They are using completely different HDMI phy. Decoupling 
> controller 
> > and phy code means rewritting a good portion of the code, unless some 
> tricks 
> > are applied, like calling phy function pointers, if they are defined. 
>
> Same HDMI TX but different HDMI TX PHY ? Kieran is working on decoupling 
> the 
> PHY configuration code for a Renesas SoC, that might be of interest to 
> you. 
>

Exactly. I'm developing only U-Boot driver, but Jean-Francois will probably
have more interest in this.


>
> > Register addresses also differ, but that can be easily solved by using 
> > undocumented magic value to restore them. 
>
> I love that :-) 
>
>
Is it allowed to use magic number which was found in binary blob? I'm new in
all this.


> > >   - The fact that you ignored Rob (v6) and I (v5) comment on using OF 
> > > graph to model the connection between the display engine and the 
> > > TCON. Something that Laurent also pointed out in this version. 
> > >   
> > >   - The fact that you ignored that you needed an HDMI connector node 
> > > as a child of the HDMI controller. This has been reported by Rob 
> > > (v6) and yet again in this version by Laurent. 
> > >   
> > >   - And finally the fact that we can't have several display engine in 
> > > parallel, if needs be. This has happened in the past already on 
> > > Allwinner SoCs, so it's definitely something we should consider in 
> > >     the DT bindings, since we can't break them. 
> > > 
> > > Until those are fixed, I cannot see how this driver can be merged, 
> > > unfortunately. 
>
> -- 
> Regards, 
>
> Laurent Pinchart 
>
>
Best regards,
Jernej Å krabec 
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/233bc908/attachment.html>

[PATCH 0/4] drm: Add support for the Amlogic Video Processing Unit

[Daniel Vetter]

On Tue, Nov 29, 2016 at 11:47:45AM +0100, Neil Armstrong wrote:
> This a repost of the previous RFC at [1] with fixes, the following patches 
> will
> be sent via a PULL Request once the Amlogic maintainer acks and takes the DT
> patches to avoid merges conflicts.
> 
> The Amlogic Meson SoCs embeds a Video Processing Unit able to output at least
> a Composite/CVBS Video with embedded VDAC and an HDMI Link with Embedded HDMI
> Transceiver.
> 
> Thus, the current driver does not support HDMI yet.
> 
> The Video Processig Unit is composed of multiple modules like the Video
> Input Unit and the Video Post Processing that can be associated to a
> CRTC with Planes management.
> The last Unit is the Venc that embeds at least 3 Encoders, ENCI for Interlace
> Video used by CVBS or HDMI, ENCP for Progressive Video used by the HDMI
> Transceiver and ENCL for LCD Display.
> 
> The LCD Display is not planned to be supported on the Meson GX Family.
> 
> This driver is a DRM/KMS driver using the following DRM components :
>  - GEM-CMA
>  - PRIME-CMA
>  - Atomic Modesetting
>  - FBDev-CMA
> 
> For the following SoCs :
>  - GXBB Family (S905)
>  - GXL Family (S905X, S905D)
>  - GXM Family (S912)
> 
> The current driver only supports the CVBS PAL/NTSC output modes, but the
> CRTC/Planes management should support bigger modes.
> But Advanced Colorspace Conversion, Scaling and HDMI Modes will be added in
> a second time.
> 
> The Device Tree bindings makes use of the endpoints video interface 
> definitions
> to connect to the optional CVBS and in the future the HDMI Connector nodes.
> 
> The driver has been tested with Xorg modesetting driver and Weston DRM 
> backend.
> 
> Changes since RFC at [1] :
>  - Add maintainers entry
>  - Move all Plane and CRTC code from backend to corresponding DRM code
>  - Keep only init and common code in backend source files
>  - Move the CVBS encoder out of the CVBS DT node, only keep the connector
>  - Various cleanups using DRM helpers
>  - Cleanup of copyright headers
>  - Fixup of bindings documentation
> 
> [1] http://lkml.kernel.org/r/1480089791-12517-1-git-send-email-narmstrong at 
> baylibre.com
> 
> Neil Armstrong (4):
>   drm: Add support for Amlogic Meson Graphic Controller
>   ARM64: dts: meson-gx: Add Graphic Controller nodes
>   dt-bindings: display: add Amlogic Meson DRM Bindings
>   MAINTAINERS: add entry for Amlogic DRM drivers

I think this is all reasonable, once you have an ack from DT maintainers
pls send the pull request to Dave.

Acked-by: Daniel Vetter 
> 
>  .../bindings/display/meson/meson-drm.txt   |  134 ++
>  MAINTAINERS|9 +
>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi  |   46 +
>  .../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts|4 +
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi   |4 +
>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi|8 +
>  .../boot/dts/amlogic/meson-gxl-nexbox-a95x.dts |4 +
>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi |8 +
>  .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts |4 +
>  arch/arm64/boot/dts/amlogic/meson-gxm.dtsi |8 +
>  drivers/gpu/drm/Kconfig|2 +
>  drivers/gpu/drm/Makefile   |1 +
>  drivers/gpu/drm/meson/Kconfig  |9 +
>  drivers/gpu/drm/meson/Makefile |5 +
>  drivers/gpu/drm/meson/meson_canvas.c   |   68 +
>  drivers/gpu/drm/meson/meson_canvas.h   |   42 +
>  drivers/gpu/drm/meson/meson_crtc.c |  208 +++
>  drivers/gpu/drm/meson/meson_crtc.h |   32 +
>  drivers/gpu/drm/meson/meson_cvbs.c |  177 +++
>  drivers/gpu/drm/meson/meson_drv.c  |  385 ++
>  drivers/gpu/drm/meson/meson_drv.h  |   61 +
>  drivers/gpu/drm/meson/meson_plane.c|  230 
>  drivers/gpu/drm/meson/meson_plane.h|   30 +
>  drivers/gpu/drm/meson/meson_registers.h| 1395 
> 
>  drivers/gpu/drm/meson/meson_vclk.c |  167 +++
>  drivers/gpu/drm/meson/meson_vclk.h |   34 +
>  drivers/gpu/drm/meson/meson_venc.c |  254 
>  drivers/gpu/drm/meson/meson_venc.h |   72 +
>  drivers/gpu/drm/meson/meson_venc_cvbs.c|  187 +++
>  drivers/gpu/drm/meson/meson_venc_cvbs.h|   41 +
>  drivers/gpu/drm/meson/meson_viu.c  |  331 +
>  drivers/gpu/drm/meson/meson_viu.h  |   64 +
>  drivers/gpu/drm/meson/meson_vpp.c  |  162 +++
>  drivers/gpu/drm/meson/meson_vpp.h  |   35 +
>  34 files changed, 4221 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/meson/meson-drm.txt
>  create mode 100644 drivers/gpu/drm/meson/Kconfig
>  create mode 100644 drivers/gpu/drm/meson/Makefile
>  create mode 100644 

[PATCH] drm: Fix locking cargo-cult in encoder/plane init/cleanup

[Daniel Vetter]

On Tue, Nov 29, 2016 at 10:39:03AM +, Frank Binns wrote:
> On 29/11/16 09:45, Daniel Vetter wrote:
> > Encoders can't be hotplugged, we dont need looking for this
> 
> s/looking/locking/
> 
> With that changed:
> Reviewed-by: Frank Binns 

Fixed, thanks for your review.
-Daniel

> 
> > since it's all single-threaded driver setup/teardown code. CRTCs
> > already don't grab locks.
> > 
> > While at it I noticed that plane's are missing the
> > drm_modeset_lock_fini() call, so add it.
> > 
> > Signed-off-by: Daniel Vetter 
> > ---
> >   drivers/gpu/drm/drm_encoder.c | 9 +
> >   drivers/gpu/drm/drm_plane.c   | 4 ++--
> >   2 files changed, 3 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > index 5c067719164d..992879f15f23 100644
> > --- a/drivers/gpu/drm/drm_encoder.c
> > +++ b/drivers/gpu/drm/drm_encoder.c
> > @@ -110,11 +110,9 @@ int drm_encoder_init(struct drm_device *dev,
> >   {
> > int ret;
> > -   drm_modeset_lock_all(dev);
> > -
> > ret = drm_mode_object_get(dev, >base, DRM_MODE_OBJECT_ENCODER);
> > if (ret)
> > -   goto out_unlock;
> > +   return ret;
> > encoder->dev = dev;
> > encoder->encoder_type = encoder_type;
> > @@ -142,9 +140,6 @@ int drm_encoder_init(struct drm_device *dev,
> > if (ret)
> > drm_mode_object_unregister(dev, >base);
> > -out_unlock:
> > -   drm_modeset_unlock_all(dev);
> > -
> > return ret;
> >   }
> >   EXPORT_SYMBOL(drm_encoder_init);
> > @@ -164,12 +159,10 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
> >  * the indices on the drm_encoder after us in the encoder_list.
> >  */
> > -   drm_modeset_lock_all(dev);
> > drm_mode_object_unregister(dev, >base);
> > kfree(encoder->name);
> > list_del(>head);
> > dev->mode_config.num_encoder--;
> > -   drm_modeset_unlock_all(dev);
> > memset(encoder, 0, sizeof(*encoder));
> >   }
> > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> > index 419ac313c36f..9147aab182c4 100644
> > --- a/drivers/gpu/drm/drm_plane.c
> > +++ b/drivers/gpu/drm/drm_plane.c
> > @@ -221,7 +221,8 @@ void drm_plane_cleanup(struct drm_plane *plane)
> >   {
> > struct drm_device *dev = plane->dev;
> > -   drm_modeset_lock_all(dev);
> > +   drm_modeset_lock_fini(>mutex);
> > +
> > kfree(plane->format_types);
> > drm_mode_object_unregister(dev, >base);
> > @@ -236,7 +237,6 @@ void drm_plane_cleanup(struct drm_plane *plane)
> > dev->mode_config.num_total_plane--;
> > if (plane->type == DRM_PLANE_TYPE_OVERLAY)
> > dev->mode_config.num_overlay_plane--;
> > -   drm_modeset_unlock_all(dev);
> > WARN_ON(plane->state && !plane->funcs->atomic_destroy_state);
> > if (plane->state && plane->funcs->atomic_destroy_state)
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH] drm/doc: Fix indenting in drm_modeset_lock.c comment

[Daniel Vetter]

On Tue, Nov 29, 2016 at 10:24:40AM +0100, Daniel Vetter wrote:
> This isn't part of the code snippet anymore ...
> 
> Signed-off-by: Daniel Vetter 

Merged with Rob's irc-ack.
-Daniel

> ---
>  drivers/gpu/drm/drm_modeset_lock.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modeset_lock.c 
> b/drivers/gpu/drm/drm_modeset_lock.c
> index 9059fe3145a1..3551ae31f143 100644
> --- a/drivers/gpu/drm/drm_modeset_lock.c
> +++ b/drivers/gpu/drm/drm_modeset_lock.c
> @@ -52,12 +52,12 @@
>   * drm_modeset_drop_locks();
>   * drm_modeset_acquire_fini();
>   *
> - *  On top of of these per-object locks using _mutex there's also an 
> overall
> - *  dev->mode_config.lock, for protecting everything else. Mostly this means
> - *  probe state of connectors, and preventing hotplug add/removal of 
> connectors.
> + * On top of of these per-object locks using _mutex there's also an 
> overall
> + * dev->mode_config.lock, for protecting everything else. Mostly this means
> + * probe state of connectors, and preventing hotplug add/removal of 
> connectors.
>   *
> - *  Finally there's a bunch of dedicated locks to protect drm core internal
> - *  lists and lookup data structures.
> + * Finally there's a bunch of dedicated locks to protect drm core internal
> + * lists and lookup data structures.
>   */
>  
>  static DEFINE_WW_CLASS(crtc_ww_class);
> -- 
> 2.10.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 2/2] drm/i915/dsi: Fix chv_exec_gpio disabling the GPIOs it is setting

[Ville Syrjälä]

On Tue, Nov 29, 2016 at 01:38:58PM +0100, Hans de Goede wrote:
> Set the CHV_GPIO_GPIOEN bit when updating GPIOs from chv_exec_gpio.
> 
> Fixes: a0a6d4ffd2ad ("drm/i915/dsi: add support for gpio elements on CHV")
> Cc: Jani Nikula 
> Cc: Ville Syrjälä 
> Signed-off-by: Hans de Goede 
> ---
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c 
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 579d2f5..47cd1b2 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -300,7 +300,8 @@ static void chv_exec_gpio(struct drm_i915_private 
> *dev_priv,
>   mutex_lock(_priv->sb_lock);
>   vlv_iosf_sb_write(dev_priv, port, cfg1, 0);
>   vlv_iosf_sb_write(dev_priv, port, cfg0,
> -   CHV_GPIO_GPIOCFG_GPO | CHV_GPIO_GPIOTXSTATE(value));
> +   CHV_GPIO_GPIOEN | CHV_GPIO_GPIOCFG_GPO |
> +   CHV_GPIO_GPIOTXSTATE(value));

Indeed.

Reviewed-by: Ville Syrjälä 
Cc: stable at vger.kernel.org

>   mutex_unlock(_priv->sb_lock);
>  }
>  
> -- 
> 2.9.3

-- 
Ville Syrjälä
Intel OTC

[PATCH 1/2] drm/i915/dsi: Fix swapping of MIPI_SEQ_DEASSERT_RESET / MIPI_SEQ_ASSERT_RESET

[Ville Syrjälä]

On Tue, Nov 29, 2016 at 01:38:57PM +0100, Hans de Goede wrote:
> Looking at the ADF code from the Android kernel sources for a
> cherrytrail tablet I noticed that it is calling the
> MIPI_SEQ_ASSERT_RESET sequence from the panel prepare hook.
> 
> Until commit b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences
> in panel prepare/unprepare hooks") the mainline i915 code was doing the
> same. That commits effectively swaps the calling of MIPI_SEQ_ASSERT_RESET /
> MIPI_SEQ_DEASSERT_RESET.
> 
> Looking at the naming of the sequences that is the right thing to do,
> but the problem is, that the old mainline code and the ADF code was
> actually calling the right sequence (tested on a cube iwork8 air tablet),
> and the swapping of the calling breaks things.
> 
> This breakage was likely not noticed in testing because on cherrytrail,
> currently chv_exec_gpio ends up disabling the gpio pins rather then
> setting them (this is fixed in the next patch in this patch-set).
> 
> This commit fixes the swapping by fixing MIPI_SEQ_ASSERT/DEASSERT_RESET's
> places in the enum defining them, so that their (new) names match their
> actual use.
> 
> Fixes: b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences...")
> Cc: Jani Nikula 
> Cc: Ville Syrjälä 
> Signed-off-by: Hans de Goede 
> ---
>  drivers/gpu/drm/i915/intel_bios.h  | 4 ++--
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.h 
> b/drivers/gpu/drm/i915/intel_bios.h
> index 8405b5a..642a5eb 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -49,11 +49,11 @@ struct edp_power_seq {
>  /* MIPI Sequence Block definitions */
>  enum mipi_seq {
>   MIPI_SEQ_END = 0,
> - MIPI_SEQ_ASSERT_RESET,
> + MIPI_SEQ_DEASSERT_RESET,
>   MIPI_SEQ_INIT_OTP,
>   MIPI_SEQ_DISPLAY_ON,
>   MIPI_SEQ_DISPLAY_OFF,
> - MIPI_SEQ_DEASSERT_RESET,
> + MIPI_SEQ_ASSERT_RESET,

That naming would be against the spec, so I don't think we want to do it
like this. Unless the spec is totally wrong, that is.

Can you provide the VBT for the affected machine so other people can
have a look at it?

>   MIPI_SEQ_BACKLIGHT_ON,  /* sequence block v2+ */
>   MIPI_SEQ_BACKLIGHT_OFF, /* sequence block v2+ */
>   MIPI_SEQ_TEAR_ON,   /* sequence block v2+ */
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c 
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 0d8ff00..579d2f5 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -376,11 +376,11 @@ static const fn_mipi_elem_exec exec_elem[] = {
>   */
>  
>  static const char * const seq_name[] = {
> - [MIPI_SEQ_ASSERT_RESET] = "MIPI_SEQ_ASSERT_RESET",
> + [MIPI_SEQ_DEASSERT_RESET] = "MIPI_SEQ_DEASSERT_RESET",
>   [MIPI_SEQ_INIT_OTP] = "MIPI_SEQ_INIT_OTP",
>   [MIPI_SEQ_DISPLAY_ON] = "MIPI_SEQ_DISPLAY_ON",
>   [MIPI_SEQ_DISPLAY_OFF]  = "MIPI_SEQ_DISPLAY_OFF",
> - [MIPI_SEQ_DEASSERT_RESET] = "MIPI_SEQ_DEASSERT_RESET",
> + [MIPI_SEQ_ASSERT_RESET] = "MIPI_SEQ_ASSERT_RESET",
>   [MIPI_SEQ_BACKLIGHT_ON] = "MIPI_SEQ_BACKLIGHT_ON",
>   [MIPI_SEQ_BACKLIGHT_OFF] = "MIPI_SEQ_BACKLIGHT_OFF",
>   [MIPI_SEQ_TEAR_ON] = "MIPI_SEQ_TEAR_ON",
> -- 
> 2.9.3

-- 
Ville Syrjälä
Intel OTC

[PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Jernej Skrabec]

Hi Maxime,

Dne torek, 29. november 2016 22.37.03 UTC+1 je oseba Maxime Ripard napisala:
>
> On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote: 
> > This patchset series adds HDMI video support to the Allwinner 
> > sun8i SoCs which include the display engine 2 (DE2). 
> > The driver contains the code for the A83T and H3 SoCs, and 
> > some H3 boards, but it could be used/extended for other SoCs 
> > (A64, H2, H5) and boards (Banana PIs, Orange PIs). 
>
> Honestly, I'm getting a bit worried by the fact that you ignore 
> reviews. 
>
> On the important reviews that you got that are to be seen as major 
> issues that block the inclusion, we have: 
>   - The fact that the HDMI driver is actually just a designware IP, 
> and while you should use the driver that already exists, you just 
> duplicated all that code. 
>
>  
That might be hard thing to do. A83T fits perfectly, but H3 and newer SoCs 
do
not. They are using completely different HDMI phy. Decoupling controller and
phy code means rewritting a good portion of the code, unless some tricks are
applied, like calling phy function pointers, if they are defined.

Register addresses also differ, but that can be easily solved by using
undocumented magic value to restore them.


>   - The fact that you ignored Rob (v6) and I (v5) comment on using OF 
> graph to model the connection between the display engine and the 
> TCON. Something that Laurent also pointed out in this version. 
>
>   - The fact that you ignored that you needed an HDMI connector node 
> as a child of the HDMI controller. This has been reported by Rob 
> (v6) and yet again in this version by Laurent. 
>
>   - And finally the fact that we can't have several display engine in 
> parallel, if needs be. This has happened in the past already on 
> Allwinner SoCs, so it's definitely something we should consider in 
> the DT bindings, since we can't break them. 
>
> Until those are fixed, I cannot see how this driver can be merged, 
> unfortunately. 
>
> Maxime 
>
> -- 
> Maxime Ripard, Free Electrons 
> Embedded Linux and Kernel engineering 
> http://free-electrons.com


Best regards,
Jernej Å krabec 
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/244c185f/attachment-0001.html>

[PATCH RFC] drm: Add a new connector atomic property for link status

[Sean Paul]

On Tue, Nov 29, 2016 at 9:45 AM, Sean Paul  wrote:
> On Mon, Nov 28, 2016 at 7:59 PM, Manasi Navare
>  wrote:
>> On Wed, Nov 23, 2016 at 09:09:28AM +0100, Daniel Vetter wrote:
>>> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
>>> > On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
>>> >  wrote:
>>> > > This is RFC patch for adding a connector link-status property
>>> > > and making it atomic by adding it to the drm_connector_state.
>>> > > This is to make sure its wired properly in 
>>> > > drm_atomic_connector_set_property
>>> > > and drm_atomic_connector_get_property functions.
>>> > >
>>> >
>>> > Thanks for sending this. We ran into some re-training awkwardness
>>> > recently, and I
>>> > was hoping for such a patch.
>>> >
>>> > > Cc: Jani Nikula 
>>> > > Cc: Daniel Vetter 
>>> > > Cc: Ville Syrjala 
>>> > > Cc: Chris Wilson 
>>> > > Signed-off-by: Manasi Navare 
>>> > > ---
>>> > >  drivers/gpu/drm/drm_atomic.c|  5 
>>> > >  drivers/gpu/drm/drm_connector.c | 65 
>>> > > +++--
>>> > >  include/drm/drm_connector.h | 12 +++-
>>> > >  include/drm/drm_mode_config.h   |  5 
>>> > >  include/uapi/drm/drm_mode.h |  4 +++
>>> > >  5 files changed, 88 insertions(+), 3 deletions(-)
>>> > >
>>> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>> > > index 89737e4..644d19f 100644
>>> > > --- a/drivers/gpu/drm/drm_atomic.c
>>> > > +++ b/drivers/gpu/drm/drm_atomic.c
>>> > > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct 
>>> > > drm_connector *connector,
>>> > >  * now?) atomic writes to DPMS property:
>>> > >  */
>>> > > return -EINVAL;
>>> > > +   } else if (property == config->link_status_property) {
>>> > > +   state->link_status = val;
>>>
>>> I think we should have a check here to make sure userspace never tries to
>>> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
>>> atomic userspace is supposed to be able to just restore everything to what
>>> it was.
>>>
>>> I don't think trying to set it to "BAD" should cause an error though, just
>>> silently keep the link status at "GOOG". So:
>>>
>>>   /* Never downgrade from GOOD to BAD on userspace's request here,
>>>* only hw issues can do that. */
>>>   if (state->link_status == GOOD)
>>>   return 0;
>>>   state->link_status = val;
>>>   return 0;
>>>
>>> > > +   return 0;
>>> > > } else if (connector->funcs->atomic_set_property) {
>>> > > return connector->funcs->atomic_set_property(connector,
>>> > > state, property, val);
>>> > > @@ -1135,6 +1138,8 @@ static void 
>>> > > drm_atomic_connector_print_state(struct drm_printer *p,
>>> > > *val = (state->crtc) ? state->crtc->base.id : 0;
>>> > > } else if (property == config->dpms_property) {
>>> > > *val = connector->dpms;
>>> > > +   } else if (property == config->link_status_property) {
>>> > > +   *val = state->link_status;
>>> > > } else if (connector->funcs->atomic_get_property) {
>>> > > return connector->funcs->atomic_get_property(connector,
>>> > > state, property, val);
>>> > > diff --git a/drivers/gpu/drm/drm_connector.c 
>>> > > b/drivers/gpu/drm/drm_connector.c
>>> > > index 5a45262..4576c9f 100644
>>> > > --- a/drivers/gpu/drm/drm_connector.c
>>> > > +++ b/drivers/gpu/drm/drm_connector.c
>>> > > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
>>> > > drm_object_attach_property(>base,
>>> > >   config->dpms_property, 0);
>>> > >
>>> > > +   drm_object_attach_property(>base,
>>> > > +  config->link_status_property,
>>> > > +  0);
>>> > > +
>>> > > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
>>> > > drm_object_attach_property(>base, 
>>> > > config->prop_crtc_id, 0);
>>> > > }
>>> > > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum 
>>> > > subpixel_order order)
>>> > >  };
>>> > >  DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
>>> > >
>>> > > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>>> > > +   { DRM_MODE_LINK_STATUS_GOOD, "Good" },
>>> > > +   { DRM_MODE_LINK_STATUS_BAD, "Bad" },
>>> > > +};
>>> > > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>>> > > +
>>> > >  /**
>>> > >   * drm_display_info_set_bus_formats - set the supported bus formats
>>> > >   * @info: display info to store bus formats in
>>> > > @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct 
>>> > > drm_display_info *info,
>>> > >   * path property the MST manager created. Userspace cannot change 
>>> > > this
>>> > >   * property.
>>> > >   * 

[PATCH v6 3/5] ARM: dts: sun8i-h3: add HDMI video nodes

[Jernej Skrabec]

Hi Jean-François,

Dne petek, 25. november 2016 11.22.20 UTC+1 je oseba Jean-François Moine 
napisala:
>
> On Fri, 25 Nov 2016 17:41:51 +0800 
> Icenowy Zheng > wrote: 
>
> > After removing CLK_PLL_DE's assigned-clock, the kernel passes 
> compilation. 
>
> The 'pll-de' and 'de' must have a fixed rate. Otherwise, if you do not 
> use the legacy u-boot, I don't know which can be the rate of the DE. 
>
> > However, it cannot recognize any HDMI screen... 
> > 
> > (My board is Orange Pi One, and I manually added status="okay"; to 
> , , ) 
> > 
> > [   16.507802] sun8i-de2 100.de-controller: bound 
> 1c0c000.lcd-controller (ops de2_lcd_ops [sun8i_de2_drm]) 
> > [   16.675948] sun8i-de2 100.de-controller: bound 1ee.hdmi (ops 
> de2_hdmi_fini [sun8i_de2_hdmi]) 
> > [   16.685120] [drm] Supports vblank timestamp caching Rev 2 
> (21.10.2013). 
> > [   16.695876] [drm] No driver support for vblank timestamp query. 
> > [   16.701862] sun8i-de2 100.de-controller: No connectors reported 
> connected with modes 
> > [   16.713061] [drm] Cannot find any crtc or sizes - going 1024x768 
> > [   16.734214] Console: switching to colour frame buffer device 128x48 
> > [   16.751022] sun8i-de2 100.de-controller: fb0:  frame buffer 
> device 
>
> I put a 'pr_warn' message is case the EDID cannot be read. 
> Have you this message? 
>
> Anyway, there is a problem with the EDID: 
> - my Orange Pi 2 (H3) randomly fails to read it. But this succeeds after 
>   rebooting once or twice. 
>

My U-Boot driver never exhibited a problem with reading EDID on OPi2. 
However,
I'm reusing code from Rockchip HDMI U-Boot driver for this (with some 
Allwinner
adjustments).


> - my Banana Pi M2+ (H3) reads it correctly each time. 
> - my Banana Pi M3 (A83T) can never read it. 
>
> BTW, on first tries, I was forcing a CEA mode thru the kernel command 
> line. This was working with the OPi2 and BPiM3, but there was no sound. 
> In the last version, I use a EDID in edid_firmware for having sound 
> with the BPiM3. This works fine. 
> But, forcing a CEA mode is no more possible, so, when the OPi2 cannot 
> read the EDID, the system switches to a VGA mode (default 1024x768) 
> which is not supported. It seems that this is your case. 
>
> So, in brief, if your board cannot read the EDID, put a EDID somewhere 
> and its path in /sys/module/drm_kms_helper/parameters/edid_firmware. 
> There will be no console, but X11 will work correctly. 
>
> -- 
> Ken ar c'hentañ|  ** Breizh ha Linux atav! ** 
> Jef|    http://moinejf.free.fr/


Best regards,
Jernej Å krabec 
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/a5812287/attachment-0001.html>

[ANNOUNCE] libdrm 2.4.74

[Robert Bragg]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1


Ben Widawsky (1):
  intel: Add Geminilake PCI IDs

Christian Gmeiner (4):
  etnaviv: add API to get drm fd from etna_device
  etnaviv: add API to create etna_device from private dup() fd
  etnaviv: change get_abs_timeout(..) to use ns.
  etnaviv: add etna_pipe_wait_ns(..)

Emil Velikov (2):
  automake: make the build less chatty
  xf86drm: introduce drmGetDeviceNameFromFd2

Eric Anholt (1):
  vc4: Add new GETPARAMs that have been merged to drm-next.

Grazvydas Ignotas (2):
  tests: kms: fix shadowed declaration warning
  libdrm: random typo fixes

Michel Dänzer (1):
  intel: Add drm_intel_gem_context_get_id to intel-symbols-check

Rob Clark (1):
  freedreno: 64bit support

Robert Bragg (2):
  intel: Add a getter for the intel_context ctx_id
  Bump version for release

git tag: libdrm-2.4.74

https://dri.freedesktop.org/libdrm/libdrm-2.4.74.tar.bz2
MD5:  31964aa15bdea1a40c5941d4ce0962ee  libdrm-2.4.74.tar.bz2
SHA1: 0d9c02d5d2c6c2fac862cb687bf45bc20d129017  libdrm-2.4.74.tar.bz2
SHA256: d80dd5a76c401f4c8756dcccd999c63d7e0a3bad258d96a829055cfd86ef840b  
libdrm-2.4.74.tar.bz2
PGP:  https://dri.freedesktop.org/libdrm/libdrm-2.4.74.tar.bz2.sig

https://dri.freedesktop.org/libdrm/libdrm-2.4.74.tar.gz
MD5:  b661a54514109caad3de3b520680b98e  libdrm-2.4.74.tar.gz
SHA1: 7b5a80fbdd432e87934ef3b1256a58ed7b034574  libdrm-2.4.74.tar.gz
SHA256: 3c8fdf5a89826797a8060e6f3455ca22db9ae49576cfcda1c78e3e2ce59af0f1  
libdrm-2.4.74.tar.gz
PGP:  https://dri.freedesktop.org/libdrm/libdrm-2.4.74.tar.gz.sig

-BEGIN PGP SIGNATURE-

iEYEARECAAYFAlg9iT0ACgkQjNHfVSl1KXvN2QCfSj1H53aYHdMVMUN2B64FVF5E
n0QAn0Fn3jDlrl6lpdbTJO3Mclg9WFUZ
=+4Tx
-END PGP SIGNATURE-

[PATCH 1/2] drm/i915/dsi: Fix swapping of MIPI_SEQ_DEASSERT_RESET / MIPI_SEQ_ASSERT_RESET

[Hans de Goede]

Hi,

Thanks for the quick reply.

On 29-11-16 13:48, Ville Syrjälä wrote:
> On Tue, Nov 29, 2016 at 01:38:57PM +0100, Hans de Goede wrote:
>> Looking at the ADF code from the Android kernel sources for a
>> cherrytrail tablet I noticed that it is calling the
>> MIPI_SEQ_ASSERT_RESET sequence from the panel prepare hook.
>>
>> Until commit b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences
>> in panel prepare/unprepare hooks") the mainline i915 code was doing the
>> same. That commits effectively swaps the calling of MIPI_SEQ_ASSERT_RESET /
>> MIPI_SEQ_DEASSERT_RESET.
>>
>> Looking at the naming of the sequences that is the right thing to do,
>> but the problem is, that the old mainline code and the ADF code was
>> actually calling the right sequence (tested on a cube iwork8 air tablet),
>> and the swapping of the calling breaks things.
>>
>> This breakage was likely not noticed in testing because on cherrytrail,
>> currently chv_exec_gpio ends up disabling the gpio pins rather then
>> setting them (this is fixed in the next patch in this patch-set).
>>
>> This commit fixes the swapping by fixing MIPI_SEQ_ASSERT/DEASSERT_RESET's
>> places in the enum defining them, so that their (new) names match their
>> actual use.
>>
>> Fixes: b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences...")
>> Cc: Jani Nikula 
>> Cc: Ville Syrjälä 
>> Signed-off-by: Hans de Goede 
>> ---
>>  drivers/gpu/drm/i915/intel_bios.h  | 4 ++--
>>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 4 ++--
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.h 
>> b/drivers/gpu/drm/i915/intel_bios.h
>> index 8405b5a..642a5eb 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.h
>> +++ b/drivers/gpu/drm/i915/intel_bios.h
>> @@ -49,11 +49,11 @@ struct edp_power_seq {
>>  /* MIPI Sequence Block definitions */
>>  enum mipi_seq {
>>  MIPI_SEQ_END = 0,
>> -MIPI_SEQ_ASSERT_RESET,
>> +MIPI_SEQ_DEASSERT_RESET,
>>  MIPI_SEQ_INIT_OTP,
>>  MIPI_SEQ_DISPLAY_ON,
>>  MIPI_SEQ_DISPLAY_OFF,
>> -MIPI_SEQ_DEASSERT_RESET,
>> +MIPI_SEQ_ASSERT_RESET,
>
> That naming would be against the spec, so I don't think we want to do it
> like this. Unless the spec is totally wrong, that is.

Given that both the ADF code and the i915 driver until now have been using
assert on prepare and deassert on unprepare I'm inclined to believe that
the spec is wrong. Is the spec available somewhere public ?

Also if you look at the v1 sequences with the new names:

MIPI_SEQ_DEASSERT_RESET,
MIPI_SEQ_INIT_OTP,
MIPI_SEQ_DISPLAY_ON,
MIPI_SEQ_DISPLAY_OFF,
MIPI_SEQ_ASSERT_RESET,

Then they are exactly in the order as one would call them on
enable, followed by disable, which I believe is not a coincidence.

> Can you provide the VBT for the affected machine so other people can
> have a look at it?

Sure I can do that, what would be the easiest way (both for me to
produce and for you to consume) to do this ?

While developing this set, I've added printk calls to the code executing the
sequences, there are 2 gpios involved nr 60 (backlight enable AFAICT, also used
by the BACKLIGHT sequences) and 72 (reset / panel_enable ?).
When efifb is up both are 1 / high.

With the OLD naming, MIPI_SEQ_ASSERT_RESET does:

gpio 72 high
delay
gpio 72 low
delay
gpio 72 high

And DEASSERT does:

gpio 72 low
gpio 60 low

So with the old naming DEASSERT leaves the panel disabled / in reset and
the backlight disabled, which is exactly what we do not want...

I guess it would be good if someone could check if my patch helps
or not on other tablets which use these sequences.

Regards,

Hans


p.s.

I'm also trying to come up with some patches which properly
integrate pwm-lpss with the i915 driver instead of it
throwing a "Failed to own the pwm chip" error. But as soon
as I hook up things so that pwm_get() returns the pwm-lpss
pwm0 I hit:

https://bugs.freedesktop.org/show_bug.cgi?id=97330

I get the same pipe-a stuck (or not seeing vblank irqs?)
problems sometimes without the pwm-lpss driver in the loop
but then only sometimes and with pwm-lpss I get this
problem when loading the i915 driver most of the time
(but not all the time).

I've been debugging this a couple of evenings in a row
now (*) but no luck so far before I fixed the gpio and
assert/deassert swapping I had commented out the
chv_exec_gpio call otherwise the screen would go off
and never come back, with that call commented I was
seeing the same issue.

I was hoping that properly resetting the screen when
fbcon does its disable / re-enable dance would fix this,
but it does not :|  Any hints would be greatly appreciated.

*) This series is one result of this, I also have some
designware i2c pmic mux fixes I need to post


>>  MIPI_SEQ_BACKLIGHT_ON,  /* sequence block v2+ */
>>  MIPI_SEQ_BACKLIGHT_OFF, /* sequence block v2+ */
>>  MIPI_SEQ_TEAR_ON,   /* sequence block 

[Bug 98897] Macbook pro 11,5 screen flicker when AC adapter plugged in

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98897

--- Comment #5 from Tom B  ---
Created attachment 128267
  --> https://bugs.freedesktop.org/attachment.cgi?id=128267=edit
Xorg log

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/68d788a3/attachment.html>

[Bug 98897] Macbook pro 11,5 screen flicker when AC adapter plugged in

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98897

--- Comment #4 from Tom B  ---
Created attachment 128266
  --> https://bugs.freedesktop.org/attachment.cgi?id=128266=edit
dmesg output

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161129/e93f2b51/attachment.html>

[PATCH v3 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings

[Archit Taneja]

On 11/29/2016 12:03 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> Thank you for the patch.
>
> On Tuesday 29 Nov 2016 11:37:41 Archit Taneja wrote:
>> Add the regulator supply properties needed by ADV7511 and ADV7533.
>>
>> The regulators are specified as optional properties since there can
>> be boards which have a fixed supply directly routed to the pins, and
>> these may not be modelled as regulator supplies.
>
> That's why we have support for dummy supplies in the kernel, isn't it ? Isn't
> it better to make the supplies mandatory in the bindings (and obviously
> handling them as optional in the driver for backward-compatibility) ?

I'm a bit unclear on this.

I thought we couldn't add mandatory properties once the device is already
present in DT for one or more platforms.

Say, if we do make it mandatory for future additions, we would need to have
DT property for the supplies for the new platforms. If the regulators on
these boards are fixed supplies, they would be need to be modeled
using "regulator-fixed", possibly without any input supply. Is that
what you're suggesting?

Thanks,
Archit

>
> Apart from that,
>
> Acked-by: Laurent Pinchart 
>
>> Cc: devicetree at vger.kernel.org
>> Acked-by: Rob Herring 
>> Signed-off-by: Archit Taneja 
>> ---
>> v3:
>> - Revert back to having a common avdd-supply property for the 1.8V
>>   supplies
>>
>> Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 9 ++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
>> 6532a59..13d53bc 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> @@ -56,6 +56,15 @@ Optional properties:
>>  - adi,disable-timing-generator: Only for ADV7533. Disables the internal
>> timing generator. The chip will rely on the sync signals in the DSI data
>> lanes, rather than generate its own timings for HDMI output.
>> +- avdd-supply: A common 1.8V supply that powers up the AVDD, DVDD and PVDD
>> +  pins. On ADV7511, it also feeds to the BGVDD pin. On ADV7533, it also
>> powers
>> +  up the A2VDD pin.
>> +- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
>> +  ADV7511 and V3P3 on ADV7533.
>> +
>> +ADV7533 specific supplies:
>> +- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be
>> +  either 1.2V or 1.8V.
>>
>>  Required nodes:
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/2] drm/i915/dsi: Fix chv_exec_gpio disabling the GPIOs it is setting

[Hans de Goede]

Set the CHV_GPIO_GPIOEN bit when updating GPIOs from chv_exec_gpio.

Fixes: a0a6d4ffd2ad ("drm/i915/dsi: add support for gpio elements on CHV")
Cc: Jani Nikula 
Cc: Ville Syrjälä 
Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c 
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 579d2f5..47cd1b2 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -300,7 +300,8 @@ static void chv_exec_gpio(struct drm_i915_private *dev_priv,
mutex_lock(_priv->sb_lock);
vlv_iosf_sb_write(dev_priv, port, cfg1, 0);
vlv_iosf_sb_write(dev_priv, port, cfg0,
- CHV_GPIO_GPIOCFG_GPO | CHV_GPIO_GPIOTXSTATE(value));
+ CHV_GPIO_GPIOEN | CHV_GPIO_GPIOCFG_GPO |
+ CHV_GPIO_GPIOTXSTATE(value));
mutex_unlock(_priv->sb_lock);
 }

-- 
2.9.3