[PATCH v5 3/7] drm: Add support for a panel-orientation connector property

2017-11-06 Thread Hans de Goede
On some devices the LCD panel is mounted in the casing in such a way that
the up/top side of the panel does not match with the top side of the
device (e.g. it is mounted upside-down).

This commit adds the necessary infra for lcd-panel drm_connector-s to
have a "panel orientation" property to communicate how the panel is
orientated vs the casing.

Userspace can use this property to check for non-normal orientation and
then adjust the displayed image accordingly by rotating it to compensate.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Rebased on 4.14-rc1
-Store panel_orientation in drm_display_info, so that drm_fb_helper.c can
 access it easily
-Have a single drm_connector_init_panel_orientation_property rather then
 create and attach functions. The caller is expected to set
 drm_display_info.panel_orientation before calling this, then this will
 check for platform specific quirks overriding the panel_orientation and if
 the panel_orientation is set after this then it will attach the property.
---
 drivers/gpu/drm/Kconfig |  1 +
 drivers/gpu/drm/drm_connector.c | 73 +
 include/drm/drm_connector.h | 11 +++
 include/drm/drm_mode_config.h   |  7 
 include/uapi/drm/drm_mode.h |  7 
 5 files changed, 99 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 9d005ac98c2b..0b166e626eb6 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -7,6 +7,7 @@
 menuconfig DRM
tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI 
support)"
depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA
+   select DRM_PANEL_ORIENTATION_QUIRKS
select HDMI
select FB_CMDLINE
select I2C
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 704fc8934616..129c83a84320 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -212,6 +213,8 @@ int drm_connector_init(struct drm_device *dev,
mutex_init(&connector->mutex);
connector->edid_blob_ptr = NULL;
connector->status = connector_status_unknown;
+   connector->display_info.panel_orientation =
+   DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
 
drm_connector_get_cmdline_mode(connector);
 
@@ -664,6 +667,13 @@ static const struct drm_prop_enum_list 
drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
 };
 
+static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = {
+   { DRM_MODE_PANEL_ORIENTATION_NORMAL,"Normal"},
+   { DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down"   },
+   { DRM_MODE_PANEL_ORIENTATION_LEFT_UP,   "Left Side Up"  },
+   { DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,  "Right Side Up" },
+};
+
 static const struct drm_prop_enum_list drm_dvi_i_select_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_Automatic, "Automatic" }, /* DVI-I and TV-out */
{ DRM_MODE_SUBCONNECTOR_DVID,  "DVI-D" }, /* DVI-I  */
@@ -768,6 +778,18 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
  *
  * CRTC_ID:
  * Mode object ID of the &drm_crtc this connector should be connected to.
+ *
+ * Connectors for LCD panels may also have one standardized property:
+ *
+ * panel orientation:
+ * On some devices the LCD panel is mounted in the casing in such a way
+ * that the up/top side of the panel does not match with the top side of
+ * the device. Userspace can use this property to check for this.
+ * Note that input coordinates from touchscreens (input devices with
+ * INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
+ * coordinates, so if userspace rotates the picture to adjust for
+ * the orientation it must also apply the same transformation to the
+ * touchscreen input coordinates.
  */
 
 int drm_connector_create_standard_properties(struct drm_device *dev)
@@ -1234,6 +1256,57 @@ void drm_mode_connector_set_link_status_property(struct 
drm_connector *connector
 }
 EXPORT_SYMBOL(drm_mode_connector_set_link_status_property);
 
+/**
+ * drm_connector_init_panel_orientation_property -
+ * initialize the connecters panel_orientation property
+ * @connector: connector for which to init the panel-orientation property.
+ * @width: width in pixels of the panel, used for panel quirk detection
+ * @height: height in pixels of the panel, used for panel quirk detection
+ *
+ * This function should only be called for built-in panels, after setting
+ * connector->display_info.panel_orientation first (if known).
+ *
+ * This function will check for platform specific (e.g. DMI based) quirks
+ * overriding display_info.panel_orientation first, then if panel_orientation
+ * is not DRM_MODE_PANEL_ORIENTATION_UNKNOWN it will attach the
+ * "panel orientation" property to the connector.
+ *
+ 

Re: [PATCH 08/10] drm/tegra: Implement dynamic channel allocation model

2017-11-06 Thread Dmitry Osipenko
On 05.11.2017 14:01, Mikko Perttunen wrote:
> In the traditional channel allocation model, a single hardware channel
> was allocated for each client. This is simple from an implementation
> perspective but prevents use of hardware scheduling.
> 
> This patch implements a channel allocation model where when a user
> submits a job for a context, a hardware channel is allocated for
> that context. The same channel is kept for as long as there are
> incomplete jobs for that context. This way we can use hardware
> scheduling and channel isolation between userspace processes, but
> also prevent idling contexts from taking up hardware resources.
> 

The dynamic channels resources (pushbuf) allocation is very expensive,
neglecting all benefits that this model should bring at least in non-IOMMU case.
We could have statically preallocated channels resources or defer resources 
freeing.

> For now, this patch only adapts VIC to the new model.
> 

I think VIC's conversion should be a distinct patch.

> Signed-off-by: Mikko Perttunen 
> ---
>  drivers/gpu/drm/tegra/drm.c | 46 ++
>  drivers/gpu/drm/tegra/drm.h |  7 +++-
>  drivers/gpu/drm/tegra/vic.c | 79 
> +++--
>  3 files changed, 92 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index b964e18e3058..658bc8814f38 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -382,6 +382,51 @@ static int host1x_waitchk_copy_from_user(struct 
> host1x_waitchk *dest,
>   return 0;
>  }
>  
> +/**
> + * tegra_drm_context_get_channel() - Get a channel for submissions
> + * @context: Context for which to get a channel for
> + *
> + * Request a free hardware host1x channel for this user context, or if the
> + * context already has one, bump its refcount.
> + *
> + * Returns 0 on success, or -EBUSY if there were no free hardware channels.
> + */
> +int tegra_drm_context_get_channel(struct tegra_drm_context *context)
> +{
> + struct host1x_client *client = &context->client->base;
> +
> + mutex_lock(&context->lock);
> +
> + if (context->pending_jobs == 0) {
> + context->channel = host1x_channel_request(client->dev);
> + if (!context->channel) {
> + mutex_unlock(&context->lock);
> + return -EBUSY;
> + }
> + }
> +
> + context->pending_jobs++;
> +
> + mutex_unlock(&context->lock);
> +
> + return 0;
> +}
> +
> +/**
> + * tegra_drm_context_put_channel() - Put a previously gotten channel
> + * @context: Context which channel is no longer needed
> + *
> + * Decrease the refcount of the channel associated with this context,
> + * freeing it if the refcount drops to zero.
> + */
> +void tegra_drm_context_put_channel(struct tegra_drm_context *context)
> +{
> + mutex_lock(&context->lock);
> + if (--context->pending_jobs == 0)
> + host1x_channel_put(context->channel);
> + mutex_unlock(&context->lock);
> +}
> +
>  static void tegra_drm_job_done(struct host1x_job *job)
>  {
>   struct tegra_drm_context *context = job->callback_data;
> @@ -737,6 +782,7 @@ static int tegra_open_channel(struct drm_device *drm, 
> void *data,
>   kfree(context);
>  
>   kref_init(&context->ref);
> + mutex_init(&context->lock);
>  
>   mutex_unlock(&fpriv->lock);
>   return err;
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index 11d690846fd0..d0c3f1f779f6 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -78,9 +78,12 @@ struct tegra_drm_context {
>   struct kref ref;
>  
>   struct tegra_drm_client *client;
> + unsigned int id;
> +
> + struct mutex lock;
>   struct host1x_channel *channel;
>   struct host1x_syncpt *syncpt;
> - unsigned int id;
> + unsigned int pending_jobs;
>  };
>  
>  struct tegra_drm_client_ops {
> @@ -95,6 +98,8 @@ struct tegra_drm_client_ops {
>   void (*submit_done)(struct tegra_drm_context *context);
>  };
>  
> +int tegra_drm_context_get_channel(struct tegra_drm_context *context);
> +void tegra_drm_context_put_channel(struct tegra_drm_context *context);
>  int tegra_drm_submit(struct tegra_drm_context *context,
>struct drm_tegra_submit *args, struct drm_device *drm,
>struct drm_file *file);
> diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> index efe5f3af933e..0cacf023a890 100644
> --- a/drivers/gpu/drm/tegra/vic.c
> +++ b/drivers/gpu/drm/tegra/vic.c
> @@ -33,7 +33,6 @@ struct vic {
>  
>   void __iomem *regs;
>   struct tegra_drm_client client;
> - struct host1x_channel *channel;
>   struct iommu_domain *domain;
>   struct device *dev;
>   struct clk *clk;
> @@ -161,28 +160,12 @@ static int vic_init(struct host1x_client *client)
>   goto detach_device;
>   }
>  
> - vic-

[PATCH v5 4/7] drm/fb-helper: Apply panel orientation connector prop to the primary plane

2017-11-06 Thread Hans de Goede
Apply the "panel orientation" drm connector prop to the primary plane so
that fbcon and fbdev using userspace programs display the right way up.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=94894
Signed-off-by: Hans de Goede 
---
Changes in v2:
-New patch in v2 of this patch-set

Changes in v3:
-Use a rotation member in struct drm_fb_helper_crtc and set that from
 drm_setup_crtcs instead of looping over all crtc's to find the right one
 later
-Since we now no longer look at rotation quirks directly in the fbcon
 code, set fb_info.fbcon_rotate_hint when the panel is not mounted upright
 and we cannot use hardware rotation

Changes in v4:
-Make drm_fb_helper_init() init drm_fb_helper_crtc.rotation to
 DRM_MODE_ROTATE_0 for all crtcs, so that we do not end up setting the
 plane_state's rotation to an invalid value for disabled crtcs
 (caught by Fi.CI)

Changes in v5:
-Only use hardware (crtc primary plane) rotation for DRM_ROTATE_180,
 90 / 270 degree rotation requires special handling which we lack atm
-Add a TODO comment for 90 / 270 degree hardware rotation
-Add some comments to better document the default case when mapping
 sw_rotations to fbcon_rotate_hints
---
 drivers/gpu/drm/drm_fb_helper.c | 90 -
 include/drm/drm_fb_helper.h |  8 
 2 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 116d1f1337c7..5a557488bff4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -41,6 +41,7 @@
 #include 
 #include 
 
+#include "drm_crtc_internal.h"
 #include "drm_crtc_helper_internal.h"
 
 static bool drm_fbdev_emulation = true;
@@ -350,6 +351,7 @@ EXPORT_SYMBOL(drm_fb_helper_debug_leave);
 static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool 
active)
 {
struct drm_device *dev = fb_helper->dev;
+   struct drm_plane_state *plane_state;
struct drm_plane *plane;
struct drm_atomic_state *state;
int i, ret;
@@ -368,8 +370,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper 
*fb_helper, bool activ
 retry:
plane_mask = 0;
drm_for_each_plane(plane, dev) {
-   struct drm_plane_state *plane_state;
-
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {
ret = PTR_ERR(plane_state);
@@ -392,6 +392,11 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper 
*fb_helper, bool activ
 
for (i = 0; i < fb_helper->crtc_count; i++) {
struct drm_mode_set *mode_set = 
&fb_helper->crtc_info[i].mode_set;
+   struct drm_plane *primary = mode_set->crtc->primary;
+
+   /* Cannot fail as we've already gotten the plane state above */
+   plane_state = drm_atomic_get_new_plane_state(state, primary);
+   plane_state->rotation = fb_helper->crtc_info[i].rotation;
 
ret = __drm_atomic_helper_set_config(mode_set, state);
if (ret != 0)
@@ -821,6 +826,7 @@ int drm_fb_helper_init(struct drm_device *dev,
if (!fb_helper->crtc_info[i].mode_set.connectors)
goto out_free;
fb_helper->crtc_info[i].mode_set.num_connectors = 0;
+   fb_helper->crtc_info[i].rotation = DRM_MODE_ROTATE_0;
}
 
i = 0;
@@ -2334,6 +2340,62 @@ static int drm_pick_crtcs(struct drm_fb_helper 
*fb_helper,
return best_score;
 }
 
+/*
+ * This function checks if rotation is necessary because of panel orientation
+ * and if it is, if it is supported.
+ * If rotation is necessary and supported, its gets set in fb_crtc.rotation.
+ * If rotation is necessary but not supported, a DRM_MODE_ROTATE_* flag gets
+ * or-ed into fb_helper->sw_rotations. In drm_setup_crtcs_fb() we check if only
+ * one bit is set and then we set fb_info.fbcon_rotate_hint to make fbcon do
+ * the unsupported rotation.
+ */
+static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
+   struct drm_fb_helper_crtc *fb_crtc,
+   struct drm_connector *connector)
+{
+   struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
+   uint64_t valid_mask = 0;
+   int i, rotation;
+
+   fb_crtc->rotation = DRM_MODE_ROTATE_0;
+
+   switch (connector->display_info.panel_orientation) {
+   case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
+   rotation = DRM_MODE_ROTATE_180;
+   break;
+   case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
+   rotation = DRM_MODE_ROTATE_90;
+   break;
+   case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
+   rotation = DRM_MODE_ROTATE_270;
+   break;
+   default:
+   rotation = DRM_MODE_ROTATE_0;
+   }
+
+   /*
+* TODO: support 90 / 270 degree hardware rotation,
+* depend

Re: [PATCH 04/10] gpu: host1x: Lock classes during job submission

2017-11-06 Thread Dmitry Osipenko
On 05.11.2017 14:01, Mikko Perttunen wrote:
> Host1x has a feature called MLOCKs which allow a certain class
> (~HW unit) to be locked (in the mutex sense) and unlocked during
> command execution, preventing other channels from accessing the
> class while it is locked. This is necessary to prevent concurrent
> jobs from messing up class state.
> 
> This has not been necessary so far since due to our channel allocation
> model, there has only been a single hardware channel submitting
> commands to each class. Future patches, however, change the channel
> allocation model to allow hardware-scheduled concurrency, and as such
> we need to start locking.
> 
> This patch implements locking on all platforms from Tegra20 to
> Tegra186.
> 
> Signed-off-by: Mikko Perttunen 
> ---
>  drivers/gpu/host1x/cdma.c  |   1 +
>  drivers/gpu/host1x/cdma.h  |   1 +
>  drivers/gpu/host1x/hw/cdma_hw.c| 122 
> +
>  drivers/gpu/host1x/hw/channel_hw.c |  71 ++
>  drivers/gpu/host1x/hw/host1x01_hardware.h  |  10 ++
>  drivers/gpu/host1x/hw/host1x02_hardware.h  |  10 ++
>  drivers/gpu/host1x/hw/host1x04_hardware.h  |  10 ++
>  drivers/gpu/host1x/hw/host1x05_hardware.h  |  10 ++
>  drivers/gpu/host1x/hw/host1x06_hardware.h  |  10 ++
>  drivers/gpu/host1x/hw/hw_host1x01_sync.h   |   6 ++
>  drivers/gpu/host1x/hw/hw_host1x02_sync.h   |   6 ++
>  drivers/gpu/host1x/hw/hw_host1x04_sync.h   |   6 ++
>  drivers/gpu/host1x/hw/hw_host1x05_sync.h   |   6 ++
>  drivers/gpu/host1x/hw/hw_host1x06_hypervisor.h |   5 +
>  14 files changed, 257 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
> index 28541b280739..f787cfe69c11 100644
> --- a/drivers/gpu/host1x/cdma.c
> +++ b/drivers/gpu/host1x/cdma.c
> @@ -232,6 +232,7 @@ static void cdma_start_timer_locked(struct host1x_cdma 
> *cdma,
>   }
>  
>   cdma->timeout.client = job->client;
> + cdma->timeout.class = job->class;
>   cdma->timeout.syncpt = host1x_syncpt_get(host, job->syncpt_id);
>   cdma->timeout.syncpt_val = job->syncpt_end;
>   cdma->timeout.start_ktime = ktime_get();
> diff --git a/drivers/gpu/host1x/cdma.h b/drivers/gpu/host1x/cdma.h
> index 286d49386be9..e72660fc83c9 100644
> --- a/drivers/gpu/host1x/cdma.h
> +++ b/drivers/gpu/host1x/cdma.h
> @@ -59,6 +59,7 @@ struct buffer_timeout {
>   ktime_t start_ktime;/* starting time */
>   /* context timeout information */
>   int client;
> + u32 class;
>  };
>  
>  enum cdma_event {
> diff --git a/drivers/gpu/host1x/hw/cdma_hw.c b/drivers/gpu/host1x/hw/cdma_hw.c
> index ce320534cbed..4d5970d863d5 100644
> --- a/drivers/gpu/host1x/hw/cdma_hw.c
> +++ b/drivers/gpu/host1x/hw/cdma_hw.c
> @@ -16,6 +16,7 @@
>   * along with this program.  If not, see .
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -243,6 +244,125 @@ static void cdma_resume(struct host1x_cdma *cdma, u32 
> getptr)
>   cdma_timeout_restart(cdma, getptr);
>  }
>  
> +static int mlock_id_for_class(unsigned int class)
> +{
> +#if HOST1X_HW >= 6
> + switch (class)
> + {
> + case HOST1X_CLASS_HOST1X:
> + return 0;
> + case HOST1X_CLASS_VIC:
> + return 17;

What is the meaning of returned ID values that you have defined here? Why VIC
should have different ID on T186?

> + default:
> + return -EINVAL;
> + }
> +#else
> + switch (class)
> + {
> + case HOST1X_CLASS_HOST1X:
> + return 0;
> + case HOST1X_CLASS_GR2D:
> + return 1;
> + case HOST1X_CLASS_GR2D_SB:
> + return 2;

Note that we are allowing to switch 2d classes in the same jobs context and
currently jobs class is somewhat hardcoded to GR2D.

Even though that GR2D and GR2D_SB use different register banks, is it okay to
trigger execution of different classes simultaneously? Would syncpoint
differentiate classes on OP_DONE event?

I suppose that MLOCK (the module lock) implies the whole module locking,
wouldn't it make sense to just use the module ID's defined in the TRM?

> + case HOST1X_CLASS_VIC:
> + return 3;
> + case HOST1X_CLASS_GR3D:
> + return 4;
> + default:
> + return -EINVAL;
> + }
> +#endif
> +}
> +
> +static void timeout_release_mlock(struct host1x_cdma *cdma)
> +{
> +#if HOST1X_HW >= 6
> + struct host1x_channel *ch = cdma_to_channel(cdma);
> + struct host1x *host = cdma_to_host1x(cdma);
> + u32 pb_pos, pb_temp[3], val;
> + int err, mlock_id;
> +
> + if (!host->hv_regs)
> + return;
> +
> + mlock_id = mlock_id_for_class(cdma->timeout.class);
> + if (WARN(mlock_id < 0, "Invalid class ID"))
> + return;
> +
> + val = host1x_hypervisor_readl(host, HOST1X_HV_MLOCK(mlock_id));
> + if (!HOST1X_HV_MLOCK_LOCKED_V(va

Re: [PATCH] drm/vc4: Convert timers to use timer_setup()

2017-11-06 Thread Kees Cook
On Mon, Oct 30, 2017 at 4:49 PM, Eric Anholt  wrote:
> Kees Cook  writes:
>
>> In preparation for unconditionally passing the struct timer_list pointer to
>> all timer callbacks, switch to using the new timer_setup() and from_timer()
>> to pass the timer pointer explicitly.
>
> Reviewed and applied to drm-misc-next.  Thanks!

Thanks!

I happened to notice that this was in next-20171102, but missing in
next-20171103. Did it get removed, or am I misunderstanding something?

-Kees

-- 
Kees Cook
Pixel Security
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 6/7] efifb: Set info->fbcon_rotate_hint based on drm_get_panel_orientation_quirk

2017-11-06 Thread Hans de Goede
On some hardware the LCD panel is not mounted upright in the casing,
but rotated by 90 degrees. In this case we want the console to
automatically be rotated to compensate.

The drm subsys has a quirk table for this, use the
drm_get_panel_orientation_quirk function to get the panel orientation
and set info->fbcon_rotate_hint based on this, so that the fbcon console
on top of efifb gets automatically rotated to compensate for the panel
orientation.

Signed-off-by: Hans de Goede 
---
 drivers/video/fbdev/Kconfig |  1 +
 drivers/video/fbdev/efifb.c | 21 -
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 5e58f5ec0a28..c4a90c497839 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -772,6 +772,7 @@ config FB_VESA
 config FB_EFI
bool "EFI-based Framebuffer Support"
depends on (FB = y) && !IA64 && EFI
+   select DRM_PANEL_ORIENTATION_QUIRKS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 3a010641f630..8c7f6aeee205 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -15,6 +15,8 @@
 #include 
 #include 
 #include 
+#include  /* For drm_get_panel_orientation_quirk */
+#include   /* For DRM_MODE_PANEL_ORIENTATION_* */
 
 static bool request_mem_succeeded = false;
 static bool nowc = false;
@@ -156,7 +158,7 @@ static u64 bar_offset;
 static int efifb_probe(struct platform_device *dev)
 {
struct fb_info *info;
-   int err;
+   int err, orientation;
unsigned int size_vmode;
unsigned int size_remap;
unsigned int size_total;
@@ -328,6 +330,23 @@ static int efifb_probe(struct platform_device *dev)
info->fix = efifb_fix;
info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
 
+   orientation = drm_get_panel_orientation_quirk(efifb_defined.xres,
+ efifb_defined.yres);
+   switch (orientation) {
+   default:
+   info->fbcon_rotate_hint = FB_ROTATE_UR;
+   break;
+   case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
+   info->fbcon_rotate_hint = FB_ROTATE_UD;
+   break;
+   case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
+   info->fbcon_rotate_hint = FB_ROTATE_CCW;
+   break;
+   case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
+   info->fbcon_rotate_hint = FB_ROTATE_CW;
+   break;
+   }
+
err = sysfs_create_groups(&dev->dev.kobj, efifb_groups);
if (err) {
pr_err("efifb: cannot add sysfs attrs\n");
-- 
2.14.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)

2017-11-06 Thread Nick Bowler
Hi,

I completed bisecting this issue.  See below.

On 2017-11-02, Nick Bowler  wrote:
> ~50% of the time after a hotplug, there is a vertical pink bar on the
> left of the display area and audio is not working at all.  According to
> the sink device the display size is 1282x720 which seems pretty wrong
> (normal and working situation is 1280x720).
>
> I posted photos of non-working versus working states here:
>
>   https://imgur.com/a/qhAZG
>
> Unplugging and plugging the cable again will correct the issue (it seems
> to, for the most part, alternate between working and not-working states,
> although not always).  It always works on power up with the cable initially
> connected.
>
> This is a regression from 4.11, where hotplug works perfectly every time.

Bisection implicates the following commit:

181e0ef092a4952aa523c5b9cb21394cf43bcd46 is the first bad commit
commit 181e0ef092a4952aa523c5b9cb21394cf43bcd46
Author: Laurent Pinchart 
Date:   Mon Mar 6 01:35:57 2017 +0200

drm: bridge: dw-hdmi: Fix the PHY power up sequence

When powering the PHY up we need to wait for the PLL to lock. This is
done by polling the TX_PHY_LOCK bit in the HDMI_PHY_STAT0 register
(interrupt-based wait could be implemented as well but is likely
overkill). The bit is asserted when the PLL locks, but the current code
incorrectly waits for the bit to be deasserted. Fix it, and while at it,
replace the udelay() with a sleep as the code never runs in
non-sleepable context.

To be consistent with the power down implementation move the poll loop
to the power off function.

Signed-off-by: Laurent Pinchart 
Tested-by: Neil Armstrong 
Reviewed-by: Jose Abreu 
Signed-off-by: Archit Taneja 
Link: 
http://patchwork.freedesktop.org/patch/msgid/20170305233557.11945-1-laurent.pinchart+rene...@ideasonboard.com

:04 04 0defad9d1a61c0355f49c679b18eebae2c4b9495
5d260e6db25d6abc1211d61ec3405be99e693a23 M  drivers

This commit does not revert cleanly, but on top of latest master (which has
the problem) I manually changed the relevant code back to its original state
and the problem is fixed, like this:

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..6618aac95a51 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1100,37 +1100,34 @@ static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)

 static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
 {
-   const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
-   unsigned int i;
-   u8 val;
+   u8 val, msec;

-   if (phy->gen == 1) {
-   dw_hdmi_phy_enable_powerdown(hdmi, false);
+   dw_hdmi_phy_enable_powerdown(hdmi, false);

-   /* Toggle TMDS enable. */
-   dw_hdmi_phy_enable_tmds(hdmi, 0);
-   dw_hdmi_phy_enable_tmds(hdmi, 1);
-   return 0;
-   }
+   /* toggle TMDS enable */
+   dw_hdmi_phy_enable_tmds(hdmi, 0);
+   dw_hdmi_phy_enable_tmds(hdmi, 1);

+   /* gen2 tx power on */
dw_hdmi_phy_gen2_txpwron(hdmi, 1);
dw_hdmi_phy_gen2_pddq(hdmi, 0);

/* Wait for PHY PLL lock */
-   for (i = 0; i < 5; ++i) {
+   msec = 5;
+   do {
val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
-   if (val)
+   if (!val)
break;

-   usleep_range(1000, 2000);
-   }
+   if (msec == 0) {
+   dev_err(hdmi->dev, "PHY PLL not locked\n");
+   return -ETIMEDOUT;
+   }

-   if (!val) {
-   dev_err(hdmi->dev, "PHY PLL failed to lock\n");
-   return -ETIMEDOUT;
-   }
+   udelay(1000);
+   msec--;
+   } while (1);

-   dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
return 0;
 }

Cheers,
  Nick
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 10/10] gpu: host1x: Optionally block when acquiring channel

2017-11-06 Thread Dmitry Osipenko
On 05.11.2017 14:01, Mikko Perttunen wrote:
> Add an option to host1x_channel_request to interruptibly wait for a
> free channel. This allows IOCTLs that acquire a channel to block
> the userspace.
> 

Wouldn't it be more optimal to request channel and block after job's pining,
when all patching and checks are completed? Note that right now we have locking
around submission in DRM, which I suppose should go away by making locking fine
grained.

Or maybe it would be more optimal to just iterate over channels, like I
suggested before [0]?

[0]
https://github.com/cyndis/linux/commit/9e6d87f40afb01fbe13ba65c73cb617bdfcd80b2#commitcomment-25012960

> Signed-off-by: Mikko Perttunen 
> ---
>  drivers/gpu/drm/tegra/drm.c  |  9 +
>  drivers/gpu/drm/tegra/gr2d.c |  6 +++---
>  drivers/gpu/drm/tegra/gr3d.c |  6 +++---
>  drivers/gpu/host1x/channel.c | 40 ++--
>  drivers/gpu/host1x/channel.h |  1 +
>  include/linux/host1x.h   |  2 +-
>  6 files changed, 43 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 658bc8814f38..19f77c1a76c0 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -389,7 +389,8 @@ static int host1x_waitchk_copy_from_user(struct 
> host1x_waitchk *dest,
>   * Request a free hardware host1x channel for this user context, or if the
>   * context already has one, bump its refcount.
>   *
> - * Returns 0 on success, or -EBUSY if there were no free hardware channels.
> + * Returns 0 on success, -EINTR if wait for a free channel was interrupted,
> + * or other error.
>   */
>  int tegra_drm_context_get_channel(struct tegra_drm_context *context)
>  {
> @@ -398,10 +399,10 @@ int tegra_drm_context_get_channel(struct 
> tegra_drm_context *context)
>   mutex_lock(&context->lock);
>  
>   if (context->pending_jobs == 0) {
> - context->channel = host1x_channel_request(client->dev);
> - if (!context->channel) {
> + context->channel = host1x_channel_request(client->dev, true);
> + if (IS_ERR(context->channel)) {
>   mutex_unlock(&context->lock);
> - return -EBUSY;
> + return PTR_ERR(context->channel);
>   }
>   }
>  
> diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c
> index 3db3bcac48b9..c1853402f69b 100644
> --- a/drivers/gpu/drm/tegra/gr2d.c
> +++ b/drivers/gpu/drm/tegra/gr2d.c
> @@ -32,9 +32,9 @@ static int gr2d_init(struct host1x_client *client)
>   unsigned long flags = HOST1X_SYNCPT_HAS_BASE;
>   struct gr2d *gr2d = to_gr2d(drm);
>  
> - gr2d->channel = host1x_channel_request(client->dev);
> - if (!gr2d->channel)
> - return -ENOMEM;
> + gr2d->channel = host1x_channel_request(client->dev, false);
> + if (IS_ERR(gr2d->channel))
> + return PTR_ERR(gr2d->channel);
>  
>   client->syncpts[0] = host1x_syncpt_request(client->dev, flags);
>   if (!client->syncpts[0]) {
> diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
> index 279438342c8c..793a91d577cb 100644
> --- a/drivers/gpu/drm/tegra/gr3d.c
> +++ b/drivers/gpu/drm/tegra/gr3d.c
> @@ -42,9 +42,9 @@ static int gr3d_init(struct host1x_client *client)
>   unsigned long flags = HOST1X_SYNCPT_HAS_BASE;
>   struct gr3d *gr3d = to_gr3d(drm);
>  
> - gr3d->channel = host1x_channel_request(client->dev);
> - if (!gr3d->channel)
> - return -ENOMEM;
> + gr3d->channel = host1x_channel_request(client->dev, false);
> + if (IS_ERR(gr3d->channel))
> + return PTR_ERR(gr3d->channel);
>  
>   client->syncpts[0] = host1x_syncpt_request(client->dev, flags);
>   if (!client->syncpts[0]) {
> diff --git a/drivers/gpu/host1x/channel.c b/drivers/gpu/host1x/channel.c
> index 9d8cad12f9d8..eebcd51261df 100644
> --- a/drivers/gpu/host1x/channel.c
> +++ b/drivers/gpu/host1x/channel.c
> @@ -43,6 +43,7 @@ int host1x_channel_list_init(struct host1x_channel_list 
> *chlist,
>   bitmap_zero(chlist->allocated_channels, num_channels);
>  
>   mutex_init(&chlist->lock);
> + sema_init(&chlist->sema, num_channels);
>  
>   return 0;
>  }
> @@ -99,6 +100,8 @@ static void release_channel(struct kref *kref)
>   host1x_cdma_deinit(&channel->cdma);
>  
>   clear_bit(channel->id, chlist->allocated_channels);
> +
> + up(&chlist->sema);
>  }
>  
>  void host1x_channel_put(struct host1x_channel *channel)
> @@ -107,19 +110,30 @@ void host1x_channel_put(struct host1x_channel *channel)
>  }
>  EXPORT_SYMBOL(host1x_channel_put);
>  
> -static struct host1x_channel *acquire_unused_channel(struct host1x *host)
> +static struct host1x_channel *acquire_unused_channel(struct host1x *host,
> +  bool wait)
>  {
>   struct host1x_channel_list *chlist = &host->channel_list;
>   unsigned int max_channels = host->i

Re: [PATCH v3] display: panel: Add Mitsubishi aa070mc01 display support (800x480)

2017-11-06 Thread Lukasz Majewski
Dear All,

> This commit adds support for Mitsubishi aa070mc01 TFT panel working
> with 8 bit ISP mode (pin 19 "mode" HIGH for 20 pin TFT connector).

Gentle ping...

> 
> Signed-off-by: Lukasz Majewski 
> ---
> Changes for v2:
> - Place the code sorted alphabetically
> - Add missing ./Documentation/devicetree/binding/display properties
>   description
> 
> Changes for v3:
> - Fix typo in display name (in property description)
> 
> ---
>  .../display/panel/mitsubishi,aa070mc01.txt |  7 +
>  drivers/gpu/drm/panel/panel-simple.c   | 35
> ++ 2 files changed, 42 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/panel/mitsubishi,aa070mc01.txt
> 
> diff --git
> a/Documentation/devicetree/bindings/display/panel/mitsubishi,aa070mc01.txt
> b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa070mc01.txt
> new file mode 100644 index 000..4f5094f --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/panel/mitsubishi,aa070mc01.txt
> @@ -0,0 +1,7 @@ +Mitsubishi "AA070MC01 7.0" WVGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "mitsubishi,aa070mc01-ca1"
> +
> +This binding is compatible with the simple-panel binding, which is
> specified +in simple-panel.txt in this directory.
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c index 07188dc..72a20fc 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1350,6 +1350,38 @@ static const struct panel_desc lg_lp129qe = {
>   },
>  };
>  
> +static const struct drm_display_mode mitsubishi_aa070mc01_mode = {
> + .clock = 30400,
> + .hdisplay = 800,
> + .hsync_start = 800 + 0,
> + .hsync_end = 800 + 1,
> + .htotal = 800 + 0 + 1 + 160,
> + .vdisplay = 480,
> + .vsync_start = 480 + 0,
> + .vsync_end = 480 + 48 + 1,
> + .vtotal = 480 + 48 + 1 + 0,
> + .vrefresh = 60,
> + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +};
> +
> +static const struct panel_desc mitsubishi_aa070mc01 = {
> + .modes = &mitsubishi_aa070mc01_mode,
> + .num_modes = 1,
> + .bpc = 8,
> + .size = {
> + .width = 152,
> + .height = 91,
> + },
> +
> + .delay = {
> + .enable = 200,
> + .unprepare = 200,
> + .disable = 400,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> + .bus_flags = DRM_BUS_FLAG_DE_HIGH,
> +};
> +
>  static const struct display_timing nec_nl12880bc20_05_timing = {
>   .pixelclock = { 6700, 7100, 7500 },
>   .hactive = { 1280, 1280, 1280 },
> @@ -2080,6 +2112,9 @@ static const struct of_device_id
> platform_of_match[] = { .compatible = "lg,lp129qe",
>   .data = &lg_lp129qe,
>   }, {
> + .compatible = "mitsubishi,aa070mc01-ca1",
> + .data = &mitsubishi_aa070mc01,
> + }, {
>   .compatible = "nec,nl12880bc20-05",
>   .data = &nec_nl12880bc20_05,
>   }, {



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpq0EDrmvUL3.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


GM107GLM: kernel oops during link training when ior becomes NULL

2017-11-06 Thread Pavel Roskin
Hello!

I'm using Dell Precision P7510 with up-to-date Fedora 27. Everything
was working fine when I had two identical monitors (Dell 24")
connected to the dock with DisplayPort cables. Various issues started
when I replaced one of the monitors with a larger Dell 34" monitor. I
could work them around by using DVI to HDMI cables instead.

I tried to use DisplayPort cables again, and I saw that one of the
external monitors would show blank screen occasionally. Changing
display settings can restore the correct picture, but it doesn't
happen every time. Sometimes all monitors go black. I hit Escape to
revert the settings, and I get an oops.

It was happening with the stock Fedora kernel
(kernel-4.13.10-300.fc27.x86_64), so I compiled the current
airlied/drm-next, and I can still reproduce it.

Typically the oops is reported in nvkm_dp_acquire in
drivers/gpu/drm/nouveau/nvkm/engine/disp/dp.c. I found that the actual
oops happens in nvkm_dp_train_cr() on this line:

for (i = 0; i < lt->dp->outp.ior->dp.nr; i++) {

lt->dp->outp.ior becomes NULL for some reason. I started adding checks
for ior to be non-NULL, but the oopses started happening in other code
in the same source file. Every time ior would become NULL.

The issue is easy to reproduce. Change the display configuration using
GNOME, click Apply, press Escape quickly.

I checked the history of dp.c, and it looks like this commit may be responsible:

commit 75eefe95ee7565c695d1e736005876d18742537f
drm/nouveau/disp/dp: store current link configuration in nvkm_ior

Apparently, ior is not a safe place to keep that information, as it
can become NULL.

Not sure if it's related, but there are two messages from nouveau that
appear in the kernel log once in a while.

This happens regardless of the cable types:

nouveau :01:00.0: disp: 0x6878[0]: INIT_GENERIC_CONDITON: unknown 0x07

This only happens with DisplayPort cables. It's printed many times
when one of the monitors goes black.

nouveau :01:00.0: disp: outp 0a:0006:0f42: training failed

I would be happy to provide more information and test patches.

-- 
Regards,
Pavel Roskin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 5/7] drm/i915: Add "panel orientation" property to the panel connector

2017-11-06 Thread Hans de Goede
Ideally we could use the VBT for this, that would be simple, in
intel_dsi_init() check dev_priv->vbt.dsi.config->rotation, set
connector->display_info.panel_orientation accordingly and call
drm_connector_init_panel_orientation_property(), done.

Unfortunately vbt.dsi.config->rotation is always 0 even on tablets
with an upside down LCD and where the GOP is properly rotating the
EFI fb in hardware.

So instead we end up reading the rotation from the primary plane.
To read the info from the primary plane, we need to know which crtc
the panel is hooked up to, so we do this the first time the panel
encoder's get_config function get called, as by then the encoder
crtc routing has been set up.

This commit only implements the panel orientation property for DSI
panels on BYT / CHT / BXT hardware, as all known non normal oriented
panels are only found on this hardware.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Read back the rotation applied by the GOP from the primary plane
 instead of relying on dev_priv->vbt.dsi.config->rotation, because it
 seems that the VBT rotation filed is always 0 even on devices where the
 GOP does apply a rotation

Changes in v3:
-Rewrite the code to read back the orientation from the primary
 plane to contain all of this in intel_dsi.c instead of poking a bunch
 of holes between all the different layers
---
 drivers/gpu/drm/i915/intel_drv.h   |  1 +
 drivers/gpu/drm/i915/intel_dsi.c   | 48 ++
 drivers/gpu/drm/i915/intel_dsi.h   |  2 ++
 drivers/gpu/drm/i915/intel_panel.c | 16 +
 4 files changed, 67 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47d022d48718..11efc6cb74c8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1727,6 +1727,7 @@ void intel_panel_enable_backlight(const struct 
intel_crtc_state *crtc_state,
  const struct drm_connector_state *conn_state);
 void intel_panel_disable_backlight(const struct drm_connector_state 
*old_conn_state);
 void intel_panel_destroy_backlight(struct drm_connector *connector);
+void intel_panel_set_orientation(struct intel_panel *panel, int orientation);
 enum drm_connector_status intel_panel_detect(struct drm_i915_private 
*dev_priv);
 extern struct drm_display_mode *intel_find_panel_downclock(
struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 83f15848098a..3e2f12db8d15 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1084,13 +1084,16 @@ static void bxt_dsi_get_pipe_config(struct 
intel_encoder *encoder,
struct drm_display_mode *adjusted_mode_sw;
struct intel_crtc *intel_crtc;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+   struct intel_panel *panel = &intel_dsi->attached_connector->panel;
unsigned int lane_count = intel_dsi->lane_count;
unsigned int bpp, fmt;
+   int orientation;
enum port port;
u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
u16 hfp_sw, hsync_sw, hbp_sw;
u16 crtc_htotal_sw, crtc_hsync_start_sw, crtc_hsync_end_sw,
crtc_hblank_start_sw, crtc_hblank_end_sw;
+   u32 val;
 
/* FIXME: hw readout should not depend on SW state */
intel_crtc = to_intel_crtc(encoder->base.crtc);
@@ -1234,6 +1237,49 @@ static void bxt_dsi_get_pipe_config(struct intel_encoder 
*encoder,
if (adjusted_mode->crtc_hblank_end == crtc_hblank_end_sw)
adjusted_mode->crtc_hblank_end =
adjusted_mode_sw->crtc_hblank_end;
+
+   if (!intel_dsi->got_panel_orientation) {
+   val = I915_READ(PLANE_CTL(intel_crtc->pipe, 0));
+   /* The rotation is used to correct for the panel orientation */
+   switch (val & PLANE_CTL_ROTATE_MASK) {
+   case PLANE_CTL_ROTATE_0:
+   orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
+   break;
+   case PLANE_CTL_ROTATE_90:
+   orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
+   break;
+   case PLANE_CTL_ROTATE_180:
+   orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
+   break;
+   case PLANE_CTL_ROTATE_270:
+   orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
+   break;
+   }
+   intel_panel_set_orientation(panel, orientation);
+   intel_dsi->got_panel_orientation = true;
+   }
+}
+
+static void vlv_dsi_get_pipe_config(struct intel_encoder *encoder)
+{
+   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+   struct intel_ds

[PATCH v5 0/7] drm/fbdev: Panel orientation connector property support

2017-11-06 Thread Hans de Goede
Hi All,

Here is v5 of my series to add a "panel orientation" property to
the drm-connector for the LCD panel to let userspace know about LCD
panels which are not mounted upright, as well as detecting upside-down
panels without needing quirks (like we do for 90 degree rotated screens).

New in v5:
-Add kernel-doc comment documenting drm_get_panel_orientation_quirk()
-drm_fb_helper: Only use hardware (crtc primary plane) rotation for
 180 degrees for now as 9-/270 degrees rotation requires special handling

New in v4:
-Fix drm_fb_helper code setting an invalid rotation value on the primary
 plane of disabled/unused crtcs (caught by Fi.CI)

New in v3:
-As requested by Daniel v3 moves the quirks over from the fbdev
 subsys to the drm subsys. I've done this by simpy starting with a copy of
 the quirk table and eventually removing the fbdev version.

The 1st patch in this series is a small fbdev/fbcon patch, patches 2-5
are all drm patches and patches 6-7 are fbdev/fbcon patches again. As
discussed previously the plan is to merge all 7 patches through the
drm tree.

Regards,

Hans
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] display: panel: Add Tianma tm070rvhg71 display support (800x480)

2017-11-06 Thread Lukasz Majewski
Hi Rob,

> Hi Rob,
> 
> > On Sat, Oct 21, 2017 at 12:10:03AM +0200, Lukasz Majewski wrote:  
> > > Signed-off-by: Lukasz Majewski 
> > > ---
> > >  .../bindings/display/panel/tianma,tm070rvhg71.txt  |  7 ++
> > >  drivers/gpu/drm/panel/panel-simple.c   | 27
> > > ++ 2 files changed, 34 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
> > > b/Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
> > > new file mode 100644 index 000..b84217f --- /dev/null
> > > +++
> > > b/Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
> > > @@ -0,0 +1,7 @@ +Tianma Micro-electronics TM070RVHG71 7.0" WXGA
> > > TFT LCD panel +
> > > +Required properties:
> > > +- compatible: should be "tianma,tm070rvhg71
> > > +
> > > +This binding is compatible with the simple-panel binding, which
> > > is specified +in simple-panel.txt in this directory.  
> > 
> > No supplies? Still need to list power-supply here if so, so it is
> > clear that this display has a single supply (or you need to list
> > multiple ones if not).  
> 
> I took the same approach as several other simple panels having
> description in Documentation/devicetree/bindings/display/panel/*
> directory (e.g. innolux,g121i1-l01.txt) .
> 
> In the description it is stated that this binding is compatible with
> one documented in the simple-panel.txt, which defines following
> properties:
> 
> Required properties:
> - power-supply: regulator to provide the supply voltage
> 
> Optional properties:
> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> - enable-gpios: GPIO pin to enable or disable the panel
> - backlight: phandle of the backlight device attached to the panel
> 
> 
> Do I need to do something more?

Gentle ping...

> 
> > 
> > Rob  
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpwFBEDRO1WX.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 1/7] fbcon: Add fbcon_rotate_hint to struct fb_info

2017-11-06 Thread Hans de Goede
On some hardware the LCD panel is not mounted upright in the casing,
but upside-down or rotated 90 degrees. In this case we want the console
to automatically be rotated to compensate.

The fbdev-driver may know about the need to rotate. Add a new
fbcon_rotate_hint field to struct fb_info, which gets initialized to -1.
If the fbdev-driver knows that some sort of rotation is necessary then
it can set this field to a FB_ROTATE_* value to tell the fbcon console
driver to rotate the console.

Signed-off-by: Hans de Goede 
---
 drivers/video/fbdev/core/fbcon.c   | 18 --
 drivers/video/fbdev/core/fbsysfs.c |  1 +
 include/linux/fb.h |  5 +
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 04612f938bab..fb317ed76b45 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -963,10 +963,13 @@ static const char *fbcon_startup(void)
ops->cur_rotate = -1;
ops->cur_blink_jiffies = HZ / 5;
info->fbcon_par = ops;
-   if (initial_rotation != -1)
-   p->con_rotate = initial_rotation;
-   else
+
+   p->con_rotate = initial_rotation;
+   if (p->con_rotate == -1)
+   p->con_rotate = info->fbcon_rotate_hint;
+   if (p->con_rotate == -1)
p->con_rotate = fbcon_platform_get_rotate(info);
+
set_blitting_type(vc, info);
 
if (info->fix.type != FB_TYPE_TEXT) {
@@ -1103,10 +1106,13 @@ static void fbcon_init(struct vc_data *vc, int init)
 
ops = info->fbcon_par;
ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-   if (initial_rotation != -1)
-   p->con_rotate = initial_rotation;
-   else
+
+   p->con_rotate = initial_rotation;
+   if (p->con_rotate == -1)
+   p->con_rotate = info->fbcon_rotate_hint;
+   if (p->con_rotate == -1)
p->con_rotate = fbcon_platform_get_rotate(info);
+
set_blitting_type(vc, info);
 
cols = vc->vc_cols;
diff --git a/drivers/video/fbdev/core/fbsysfs.c 
b/drivers/video/fbdev/core/fbsysfs.c
index 15755ce1d26c..e31a182b42bf 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -58,6 +58,7 @@ struct fb_info *framebuffer_alloc(size_t size, struct device 
*dev)
info->par = p + fb_info_size;
 
info->device = dev;
+   info->fbcon_rotate_hint = -1;
 
 #ifdef CONFIG_FB_BACKLIGHT
mutex_init(&info->bl_curve_mutex);
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f4386b0ccf40..10cf71cc5d13 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -464,6 +464,11 @@ struct fb_info {
atomic_t count;
int node;
int flags;
+   /*
+* -1 by default, set to a FB_ROTATE_* value by the driver, if it knows
+* a lcd is not mounted upright and fbcon should rotate to compensate.
+*/
+   int fbcon_rotate_hint;
struct mutex lock;  /* Lock for open/release/ioctl funcs */
struct mutex mm_lock;   /* Lock for fb_mmap and smem_* fields */
struct fb_var_screeninfo var;   /* Current var */
-- 
2.14.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 2/7] drm: Add panel orientation quirks

2017-11-06 Thread Hans de Goede
Some x86 clamshell design devices use portrait tablet screens and a display
engine which cannot rotate in hardware, so the firmware just leaves things
as is and we cannot figure out that the display is oriented non upright
from the hardware.

So at least on x86, we need a quirk table for this. This commit adds a DMI
based quirk table which is initially populated with 5 such devices: Asus
T100HA, GPD Pocket, GPD win, I.T.Works TW891 and the VIOS LTH17.

This quirk table will be used by the drm code to let userspace know that
the display is not mounted upright inside the devices case through a new
panel orientation drm-connector property, as well as to tell fbcon to
rotate the console so that it shows the right way up.

Signed-off-by: Hans de Goede 
---
Changes in v5:
-Add a kernel-doc comment documenting drm_get_panel_orientation_quirk()
-Remove board_* matches from the dmi-matches for the VIOS LTH17 laptop,
 keeping only the (identical) sys_vendor and product_name matches.
 This is necessary because an older version of the bios has
 board_vendor set to VOIS instead of VIOS
---
 drivers/gpu/drm/Kconfig|   3 +
 drivers/gpu/drm/Makefile   |   1 +
 drivers/gpu/drm/drm_panel_orientation_quirks.c | 174 +
 include/drm/drm_utils.h|  18 +++
 4 files changed, 196 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_panel_orientation_quirks.c
 create mode 100644 include/drm/drm_utils.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 4d9f21831741..9d005ac98c2b 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -26,6 +26,9 @@ config DRM_MIPI_DSI
bool
depends on DRM
 
+config DRM_PANEL_ORIENTATION_QUIRKS
+   tristate
+
 config DRM_DP_AUX_CHARDEV
bool "DRM DP AUX Interface"
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index a3fdc5a68dff..ffb621819bbf 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/
 
 obj-$(CONFIG_DRM)  += drm.o
 obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
+obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
 obj-$(CONFIG_DRM_ARM)  += arm/
 obj-$(CONFIG_DRM_TTM)  += ttm/
 obj-$(CONFIG_DRM_TDFX) += tdfx/
diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c 
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
new file mode 100644
index ..b8765e2ed1d6
--- /dev/null
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -0,0 +1,174 @@
+/*
+ *  drm_panel_orientation_quirks.c -- Quirks for non-normal panel orientation
+ *
+ * Copyright (C) 2017 Hans de Goede 
+ *
+ *  This file is subject to the terms and conditions of the GNU General Public
+ *  License.  See the file COPYING in the main directory of this archive for
+ *  more details.
+ */
+
+#include 
+#include 
+
+#ifdef CONFIG_DMI
+
+/*
+ * Some x86 clamshell design devices use portrait tablet screens and a display
+ * engine which cannot rotate in hardware, so we need to rotate the fbcon to
+ * compensate. Unfortunately these (cheap) devices also typically have quite
+ * generic DMI data, so we match on a combination of DMI data, screen 
resolution
+ * and a list of known BIOS dates to avoid false positives.
+ */
+
+struct drm_dmi_panel_orientation_data {
+   int width;
+   int height;
+   const char * const *bios_dates;
+   int orientation;
+};
+
+static const struct drm_dmi_panel_orientation_data asus_t100ha = {
+   .width = 800,
+   .height = 1280,
+   .orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP,
+};
+
+static const struct drm_dmi_panel_orientation_data gpd_pocket = {
+   .width = 1200,
+   .height = 1920,
+   .bios_dates = (const char * const []){ "05/26/2017", "06/28/2017",
+   "07/05/2017", "08/07/2017", NULL },
+   .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
+};
+
+static const struct drm_dmi_panel_orientation_data gpd_win = {
+   .width = 720,
+   .height = 1280,
+   .bios_dates = (const char * const []){
+   "10/25/2016", "11/18/2016", "12/23/2016", "12/26/2016",
+   "02/21/2017", "03/20/2017", "05/25/2017", NULL },
+   .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
+};
+
+static const struct drm_dmi_panel_orientation_data itworks_tw891 = {
+   .width = 800,
+   .height = 1280,
+   .bios_dates = (const char * const []){ "10/16/2015", NULL },
+   .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
+};
+
+static const struct drm_dmi_panel_orientation_data vios_lth17 = {
+   .width = 800,
+   .height = 1280,
+   .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
+};
+
+static const struct dmi_system_id orientation_data[] = {
+   {   /* Asus T100HA */
+   .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+   

[PATCH v5 7/7] fbcon: Remove dmi quirk table

2017-11-06 Thread Hans de Goede
This is now all handled in the drivers and communicated through
fb_info.fbcon_rotate_hint.

Signed-off-by: Hans de Goede 
---
 drivers/video/fbdev/core/Makefile   |   3 -
 drivers/video/fbdev/core/fbcon.c|   4 +-
 drivers/video/fbdev/core/fbcon.h|   6 --
 drivers/video/fbdev/core/fbcon_dmi_quirks.c | 145 
 4 files changed, 2 insertions(+), 156 deletions(-)
 delete mode 100644 drivers/video/fbdev/core/fbcon_dmi_quirks.c

diff --git a/drivers/video/fbdev/core/Makefile 
b/drivers/video/fbdev/core/Makefile
index 73493bbd7a15..0214b863ac3f 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -14,9 +14,6 @@ ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE_ROTATION),y)
 fb-y += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \
 fbcon_ccw.o
 endif
-ifeq ($(CONFIG_DMI),y)
-fb-y += fbcon_dmi_quirks.o
-endif
 endif
 fb-objs   := $(fb-y)
 
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index fb317ed76b45..157a40670a47 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -968,7 +968,7 @@ static const char *fbcon_startup(void)
if (p->con_rotate == -1)
p->con_rotate = info->fbcon_rotate_hint;
if (p->con_rotate == -1)
-   p->con_rotate = fbcon_platform_get_rotate(info);
+   p->con_rotate = FB_ROTATE_UR;
 
set_blitting_type(vc, info);
 
@@ -,7 +,7 @@ static void fbcon_init(struct vc_data *vc, int init)
if (p->con_rotate == -1)
p->con_rotate = info->fbcon_rotate_hint;
if (p->con_rotate == -1)
-   p->con_rotate = fbcon_platform_get_rotate(info);
+   p->con_rotate = FB_ROTATE_UR;
 
set_blitting_type(vc, info);
 
diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index 18f3ac144237..3746828356ed 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -261,10 +261,4 @@ extern void fbcon_set_rotate(struct fbcon_ops *ops);
 #define fbcon_set_rotate(x) do {} while(0)
 #endif /* CONFIG_FRAMEBUFFER_CONSOLE_ROTATION */
 
-#ifdef CONFIG_DMI
-int fbcon_platform_get_rotate(struct fb_info *info);
-#else
-#define fbcon_platform_get_rotate(i) FB_ROTATE_UR
-#endif /* CONFIG_DMI */
-
 #endif /* _VIDEO_FBCON_H */
diff --git a/drivers/video/fbdev/core/fbcon_dmi_quirks.c 
b/drivers/video/fbdev/core/fbcon_dmi_quirks.c
deleted file mode 100644
index 6904e47d1e51..
--- a/drivers/video/fbdev/core/fbcon_dmi_quirks.c
+++ /dev/null
@@ -1,145 +0,0 @@
-/*
- *  fbcon_dmi_quirks.c -- DMI based quirk detection for fbcon
- *
- * Copyright (C) 2017 Hans de Goede 
- *
- *  This file is subject to the terms and conditions of the GNU General Public
- *  License.  See the file COPYING in the main directory of this archive for
- *  more details.
- */
-
-#include 
-#include 
-#include 
-#include "fbcon.h"
-
-/*
- * Some x86 clamshell design devices use portrait tablet screens and a display
- * engine which cannot rotate in hardware, so we need to rotate the fbcon to
- * compensate. Unfortunately these (cheap) devices also typically have quite
- * generic DMI data, so we match on a combination of DMI data, screen 
resolution
- * and a list of known BIOS dates to avoid false positives.
- */
-
-struct fbcon_dmi_rotate_data {
-   int width;
-   int height;
-   const char * const *bios_dates;
-   int rotate;
-};
-
-static const struct fbcon_dmi_rotate_data rotate_data_asus_t100ha = {
-   .width = 800,
-   .height = 1280,
-   .rotate = FB_ROTATE_CCW,
-};
-
-static const struct fbcon_dmi_rotate_data rotate_data_gpd_pocket = {
-   .width = 1200,
-   .height = 1920,
-   .bios_dates = (const char * const []){ "05/26/2017", "06/28/2017",
-   "07/05/2017", "08/07/2017", NULL },
-   .rotate = FB_ROTATE_CW,
-};
-
-static const struct fbcon_dmi_rotate_data rotate_data_gpd_win = {
-   .width = 720,
-   .height = 1280,
-   .bios_dates = (const char * const []){
-   "10/25/2016", "11/18/2016", "12/23/2016", "12/26/2016",
-   "02/21/2017", "03/20/2017", "05/25/2017", NULL },
-   .rotate = FB_ROTATE_CW,
-};
-
-static const struct fbcon_dmi_rotate_data rotate_data_itworks_tw891 = {
-   .width = 800,
-   .height = 1280,
-   .bios_dates = (const char * const []){ "10/16/2015", NULL },
-   .rotate = FB_ROTATE_CW,
-};
-
-static const struct fbcon_dmi_rotate_data rotate_data_vios_lth17 = {
-   .width = 800,
-   .height = 1280,
-   .rotate = FB_ROTATE_CW,
-};
-
-static const struct dmi_system_id rotate_data[] = {
-   {   /* Asus T100HA */
-   .matches = {
- DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
- DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "T100H

[PATCH v8 3/5] Documentation: Add device tree binding for Goldfish FB driver

2017-11-06 Thread Aleksandar Markovic
From: Aleksandar Markovic 

Add documentation for DT binding of Goldfish FB driver. The compatible
string used by OS for binding the driver is "google,goldfish-fb".

Signed-off-by: Miodrag Dinic 
Signed-off-by: Goran Ferenc 
Signed-off-by: Aleksandar Markovic 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/display/google,goldfish-fb.txt  | 17 +
 1 file changed, 17 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/google,goldfish-fb.txt

diff --git a/Documentation/devicetree/bindings/display/google,goldfish-fb.txt 
b/Documentation/devicetree/bindings/display/google,goldfish-fb.txt
new file mode 100644
index 000..751fa9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/google,goldfish-fb.txt
@@ -0,0 +1,17 @@
+Android Goldfish framebuffer
+
+Android Goldfish framebuffer device used by Android emulator.
+
+Required properties:
+
+- compatible : should contain "google,goldfish-fb"
+- reg: 
+- interrupts : 
+
+Example:
+
+   display-controller@1f008000 {
+   compatible = "google,goldfish-fb";
+   interrupts = <0x10>;
+   reg = <0x1f008000 0x100>;
+   };
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/vc4: Convert timers to use timer_setup()

2017-11-06 Thread Daniel Vetter
On Fri, Nov 03, 2017 at 01:07:57PM -0700, Eric Anholt wrote:
> Kees Cook  writes:
> 
> > On Mon, Oct 30, 2017 at 4:49 PM, Eric Anholt  wrote:
> >> Kees Cook  writes:
> >>
> >>> In preparation for unconditionally passing the struct timer_list pointer 
> >>> to
> >>> all timer callbacks, switch to using the new timer_setup() and 
> >>> from_timer()
> >>> to pass the timer pointer explicitly.
> >>
> >> Reviewed and applied to drm-misc-next.  Thanks!
> >
> > Thanks!
> >
> > I happened to notice that this was in next-20171102, but missing in
> > next-20171103. Did it get removed, or am I misunderstanding something?
> 
> I don't know.  It's in drm-misc-next, though, so it'll flow upstream
> without my intervention.

I'll make 4.16. Again, our merge cutoff is -rc6, hence why they all
missed. I guess one more to cherry-pick for you, or time to give up?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 00/10] Allwinner H3/H5/A64(DE2) SimpleFB support

2017-11-06 Thread Daniel Vetter
On Thu, Nov 02, 2017 at 04:44:47PM +0100, Maxime Ripard wrote:
> On Thu, Nov 02, 2017 at 04:51:29PM +0800, Icenowy Zheng wrote:
> > 在 2017-10-27 23:06,Icenowy Zheng 写道:
> > > This patchset adds support for the SimpleFB on Allwinner SoCs with
> > > "Display Engine 2.0".
> > > 
> > > PATCH 1 to PATCH 3 are DE2 CCU fixes for H3/H5 SoCs.
> > > 
> > > PATCH 4 adds the pipeline strings for DE2 SimpleFB.
> > > 
> > > PATCH 5 to 7 adds necessary device tree nodes (DE2 CCU and SimpleFB)
> > > for H3/H5 SoCs.
> > > 
> > > PATCH 8 to 10 are for Allwinner A64 SoC to enable SimpleFB.
> > > 
> > > Icenowy Zheng (10):
> > >   dt-bindings: fix the binding of Allwinner DE2 CCU of A83T and H3
> > >   clk: sunxi-ng: add support for Allwinner H3 DE2 CCU
> > >   clk: sunxi-ng: fix the A64/H5 clock description of DE2 CCU
> > >   dt-bindings: simplefb-sunxi: add pipelines for DE2
> > >   ARM: sun8i: h3/h5: add DE2 CCU device node for H3
> > >   arm64: allwinner: h5: add compatible string for DE2 CCU
> > >   ARM: sunxi: h3/h5: add simplefb nodes
> > >   dt-bindings: add binding for A64 DE2 CCU SRAM
> > >   arm64: allwinner: a64: add DE2 CCU for A64 SoC
> > >   arm64: allwinner: a64: add simplefb for A64 SoC
> > 
> > Maxime, could you review and, if possible, apply the H3/5
> > part of this patchset?
> 
> This came a bit late, we're too close from the merge window
> now. Please resend them after -rc1 is out.

Just dropping here that drm-misc is open all the time, making for a much
better process for contributors :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 2/4] dma-buf/fence: Sparse wants __rcu on the object itself

2017-11-06 Thread Daniel Vetter
On Thu, Nov 02, 2017 at 10:03:34PM +0200, Ville Syrjala wrote:
> From: Chris Wilson 
> 
> In order to silent sparse in dma_fence_get_rcu_safe(), we need to mark

s/silent/silence/

On the series (assuming sparse is indeed happy now, I didn't check that):

Reviewed-by: Daniel Vetter 

> the incoming fence object as being RCU protected and not the pointer to
> the object.
> 
> Cc: Dave Airlie 
> Cc: Jason Ekstrand 
> Cc: linaro-mm-...@lists.linaro.org
> Cc: linux-me...@vger.kernel.org
> Cc: Alex Deucher 
> Cc: Christian König 
> Cc: Sumit Semwal 
> Signed-off-by: Chris Wilson 
> ---
>  include/linux/dma-fence.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index efdabbb64e3c..4c008170fe65 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -242,7 +242,7 @@ static inline struct dma_fence *dma_fence_get_rcu(struct 
> dma_fence *fence)
>   * The caller is required to hold the RCU read lock.
>   */
>  static inline struct dma_fence *
> -dma_fence_get_rcu_safe(struct dma_fence * __rcu *fencep)
> +dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep)
>  {
>   do {
>   struct dma_fence *fence;
> -- 
> 2.13.6
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/6] drm/probe-helper: Fix drm_kms_helper_poll_enable() docs

2017-11-06 Thread Daniel Vetter
On Thu, Nov 02, 2017 at 09:09:04PM +0100, Noralf Trønnes wrote:
> Fix docs to reflect code and drm_kms_helper_poll_disable() docs by saying
> that calling drm_kms_helper_poll_enable() is fine even if output polling
> is not enabled.
> 
> Signed-off-by: Noralf Trønnes 

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/drm_probe_helper.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> b/drivers/gpu/drm/drm_probe_helper.c
> index 5840aabbf24e..024a89bf0ba7 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -216,8 +216,7 @@ enum drm_mode_status drm_connector_mode_valid(struct 
> drm_connector *connector,
>   * suspend/resume.
>   *
>   * Drivers can call this helper from their device resume implementation. It 
> is
> - * an error to call this when the output polling support has not yet been set
> - * up.
> + * not an error to call this even when output polling isn't enabled.
>   *
>   * Note that calls to enable and disable polling must be strictly ordered, 
> which
>   * is automatically the case when they're only call from suspend/resume
> -- 
> 2.14.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 2/6] drm/modeset-helper: Add simple modeset suspend/resume helpers

2017-11-06 Thread Daniel Vetter
On Thu, Nov 02, 2017 at 09:09:05PM +0100, Noralf Trønnes wrote:
> Add drm_mode_config_helper_suspend/resume() which takes care of
> atomic modeset suspend/resume for simple use cases.
> The suspend state is stored in struct drm_mode_config.
> 
> Signed-off-by: Noralf Trønnes 

It'd be great if we could add a paragraph somewhere more prominent that
references these 2 helpers. As-is they'll be impossible to find.

But I didn't find a good spot, so

Reviewed-by: Daniel Vetter 

on this one. If you have an idea, follow-up would be great.

> ---
>  drivers/gpu/drm/drm_modeset_helper.c | 76 
> 
>  include/drm/drm_mode_config.h|  9 +
>  include/drm/drm_modeset_helper.h |  3 ++
>  3 files changed, 88 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c 
> b/drivers/gpu/drm/drm_modeset_helper.c
> index 9cb1eede0b4d..f1c24ab0ef09 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -20,6 +20,9 @@
>   * OF THIS SOFTWARE.
>   */
>  
> +#include 
> +#include 
> +#include 
>  #include 
>  #include 
>  
> @@ -156,3 +159,76 @@ int drm_crtc_init(struct drm_device *dev, struct 
> drm_crtc *crtc,
>NULL);
>  }
>  EXPORT_SYMBOL(drm_crtc_init);
> +
> +/**
> + * drm_mode_config_helper_suspend - Modeset suspend helper
> + * @dev: DRM device
> + *
> + * This helper function takes care of suspending the modeset side. It 
> disables
> + * output polling if initialized, suspends fbdev if used and finally calls
> + * drm_atomic_helper_suspend().

ocd nit: Either full new paragraph or merge the lines.
-Daniel

> + * If suspending fails, fbdev and polling is re-enabled.
> + *
> + * Returns:
> + * Zero on success, negative error code on error.
> + *
> + * See also:
> + * drm_kms_helper_poll_disable() and drm_fb_helper_set_suspend_unlocked().
> + */
> +int drm_mode_config_helper_suspend(struct drm_device *dev)
> +{
> + struct drm_atomic_state *state;
> +
> + if (!dev)
> + return 0;
> +
> + drm_kms_helper_poll_disable(dev);
> + drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 1);
> + state = drm_atomic_helper_suspend(dev);
> + if (IS_ERR(state)) {
> + drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
> + drm_kms_helper_poll_enable(dev);
> + return PTR_ERR(state);
> + }
> +
> + dev->mode_config.suspend_state = state;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_config_helper_suspend);
> +
> +/**
> + * drm_mode_config_helper_resume - Modeset resume helper
> + * @dev: DRM device
> + *
> + * This helper function takes care of resuming the modeset side. It calls
> + * drm_atomic_helper_resume(), resumes fbdev if used and enables output 
> polling
> + * if initiaized.
> + *
> + * Returns:
> + * Zero on success, negative error code on error.
> + *
> + * See also:
> + * drm_fb_helper_set_suspend_unlocked() and drm_kms_helper_poll_enable().
> + */
> +int drm_mode_config_helper_resume(struct drm_device *dev)
> +{
> + int ret;
> +
> + if (!dev)
> + return 0;
> +
> + if (WARN_ON(!dev->mode_config.suspend_state))
> + return -EINVAL;
> +
> + ret = drm_atomic_helper_resume(dev, dev->mode_config.suspend_state);
> + if (ret)
> + DRM_ERROR("Failed to resume (%d)\n", ret);
> + dev->mode_config.suspend_state = NULL;
> +
> + drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
> + drm_kms_helper_poll_enable(dev);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_mode_config_helper_resume);
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 1b37368416c8..5a872496b409 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -766,6 +766,15 @@ struct drm_mode_config {
>   /* cursor size */
>   uint32_t cursor_width, cursor_height;
>  
> + /**
> +  * @suspend_state:
> +  *
> +  * Atomic state when suspended.
> +  * Set by drm_mode_config_helper_suspend() and cleared by
> +  * drm_mode_config_helper_resume().
> +  */
> + struct drm_atomic_state *suspend_state;
> +
>   const struct drm_mode_config_helper_funcs *helper_private;
>  };
>  
> diff --git a/include/drm/drm_modeset_helper.h 
> b/include/drm/drm_modeset_helper.h
> index cb0ec92e11e6..efa337f03129 100644
> --- a/include/drm/drm_modeset_helper.h
> +++ b/include/drm/drm_modeset_helper.h
> @@ -34,4 +34,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device *dev,
>  int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
> const struct drm_crtc_funcs *funcs);
>  
> +int drm_mode_config_helper_suspend(struct drm_device *dev);
> +int drm_mode_config_helper_resume(struct drm_device *dev);
> +
>  #endif
> -- 
> 2.14.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list

Re: [PATCH 6/6] drm/docs: Add todo entry for simple modeset suspend/resume

2017-11-06 Thread Daniel Vetter
On Thu, Nov 02, 2017 at 09:09:09PM +0100, Noralf Trønnes wrote:
> Add entry for conversion of drivers to new helpers.
> 
> Signed-off-by: Noralf Trønnes 

Reviewed-by: Daniel Vetter 

I think you can merge this one as soon as the code has landed, no need to
wait for all the driver conversions to get acks.
-Daniel


> ---
>  Documentation/gpu/todo.rst | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index a44f379d2b25..6bce1beafabe 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -185,6 +185,15 @@ are better.
>  
>  Contact: Sean Paul, Maintainer of the driver you plan to convert
>  
> +Convert drivers to use simple modeset suspend/resume
> +
> +
> +Most drivers (except i915 and nouveau) that use
> +drm_atomic_helper_suspend/resume() can probably be converted to use
> +drm_mode_config_helper_suspend/resume().
> +
> +Contact: Maintainer of the driver you plan to convert
> +
>  Core refactorings
>  =
>  
> -- 
> 2.14.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] video: atmel_lcdfb: Use common error handling code in atmel_lcdfb_of_init()

2017-11-06 Thread Dan Carpenter
On Mon, Nov 06, 2017 at 09:40:19AM +0100, Nicolas Ferre wrote:
> If you want to lower the size of strings in this driver, you can do it,
> but not like this.

Just so we're clear, GCC already detects and combines it when you use
the same string constant twice.

regards,
dan carpenter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 01/22] drm/gem-fb-helper: drm_gem_fbdev_fb_create() make funcs optional

2017-11-06 Thread Daniel Vetter
On Sat, Nov 04, 2017 at 02:03:55PM +0100, Noralf Trønnes wrote:
> Make the drm_framebuffer_funcs argument optional for drivers that
> don't need to set the dirty callback.
> 
> Signed-off-by: Noralf Trønnes 

Looks like this patch ended up in the wrong patch series? Atm there's not
much users of this (I guess they're still waiting for acks), and somehow
I'd expect that there's some drivers which really want this (but I don't
see any in this patch series). Looks good, but I'm a bit confused.
-Daniel

> ---
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
> b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index aa8cb9bfa499..4d682a6e8bcb 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -272,7 +272,8 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
>   * @sizes: fbdev size description
>   * @pitch_align: Optional pitch alignment
>   * @obj: GEM object backing the framebuffer
> - * @funcs: vtable to be used for the new framebuffer object
> + * @funcs: Optional vtable to be used for the new framebuffer object when the
> + * dirty callback is needed.
>   *
>   * This function creates a framebuffer from a &drm_fb_helper_surface_size
>   * description for use in the &drm_fb_helper_funcs.fb_probe callback.
> @@ -300,6 +301,9 @@ drm_gem_fbdev_fb_create(struct drm_device *dev,
>   if (obj->size < mode_cmd.pitches[0] * mode_cmd.height)
>   return ERR_PTR(-EINVAL);
>  
> + if (!funcs)
> + funcs = &drm_gem_fb_funcs;
> +
>   return drm_gem_fb_alloc(dev, &mode_cmd, &obj, 1, funcs);
>  }
>  EXPORT_SYMBOL(drm_gem_fbdev_fb_create);
> -- 
> 2.14.2
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/amd/display: small cleanup in destruct()

2017-11-06 Thread Christian König

Am 06.11.2017 um 08:07 schrieb Dan Carpenter:

Static analysis tools get annoyed that we don't indent this if
statement.  Actually, the if statement isn't required because kfree()
can handle NULL pointers just fine.  The DCE110STRENC_FROM_STRENC()
macro is a wrapper around container_of() but it's basically a no-op or a
cast.  Anyway, it's not really appropriate here so it should be removed
as well.

Signed-off-by: Dan Carpenter 


Acked-by: Christian König 


---
v2: in v1 I just added a tab

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index d911590d08bc..4c4bd72d4e40 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -725,10 +725,8 @@ static void destruct(struct dcn10_resource_pool *pool)
}
}
  
-	for (i = 0; i < pool->base.stream_enc_count; i++) {

-   if (pool->base.stream_enc[i] != NULL)
-   kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
-   }
+   for (i = 0; i < pool->base.stream_enc_count; i++)
+   kfree(pool->base.stream_enc[i]);
  
  	for (i = 0; i < pool->base.audio_count; i++) {

if (pool->base.audios[i])



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: video: atmel_lcdfb: Use common error handling code in atmel_lcdfb_of_init()

2017-11-06 Thread SF Markus Elfring
>> If you want to lower the size of strings in this driver, you can do it,
>> but not like this.
> 
> Just so we're clear, GCC already detects and combines it when you use
> the same string constant twice.

Do you distinguish between merging of constants and the combination
of statements for such an use case?

Regards,
Markus
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 02/22] drm/cma-helper: Add drm_fb_cma_fbdev_init/fini()

2017-11-06 Thread Daniel Vetter
On Sat, Nov 04, 2017 at 02:03:56PM +0100, Noralf Trønnes wrote:
> Add functions drm_fb_cma_fbdev_init(), drm_fb_cma_fbdev_fini() and
> drm_fb_cma_fbdev_init_with_funcs(). These functions relies on the fact
> that the drm_fb_helper struct is stored in dev->drm_fb_helper_private
> so drivers don't need to store it.
> 
> Cc: Laurent Pinchart 
> Signed-off-by: Noralf Trønnes 

I guess you've proven me wrong, since at the end of this patch series we
do have some neat functions that do the same thing an earlier patch series
did, expect there's a _cma_ in the name. But the function itself is 100%
generic.

Sry for all the meandering, I guess this once again shows that any problem
in software can indeed be solved by adding another abstraction layer on
top. You'll probably be slightly mad, but I'd vote for the patch to drop
the _cma_ infix again (once everything has landed, no need to respin all
the patches again).


> ---
>  drivers/gpu/drm/drm_fb_cma_helper.c | 116 
> +++-
>  include/drm/drm_fb_cma_helper.h |   7 +++
>  2 files changed, 121 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 0e3c14174d08..267c04216281 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #define DEFAULT_FBDEFIO_DELAY_MS 50
> @@ -42,7 +43,7 @@ struct drm_fbdev_cma {
>   * callback function to create a cma backed framebuffer.
>   *
>   * An fbdev framebuffer backed by cma is also available by calling
> - * drm_fbdev_cma_init(). drm_fbdev_cma_fini() tears it down.
> + * drm_fb_cma_fbdev_init(). drm_fb_cma_fbdev_fini() tears it down.
>   * If the &drm_framebuffer_funcs.dirty callback is set, fb_deferred_io will 
> be
>   * set up automatically. &drm_framebuffer_funcs.dirty is called by
>   * drm_fb_helper_deferred_io() in process context (&struct delayed_work).
> @@ -68,7 +69,7 @@ struct drm_fbdev_cma {
>   *
>   * Initialize::
>   *
> - * fbdev = drm_fbdev_cma_init_with_funcs(dev, 16,
> + * fbdev = drm_fb_cma_fbdev_init_with_funcs(dev, 16,
>   *   dev->mode_config.num_crtc,
>   *   dev->mode_config.num_connector,
>   *   &driver_fb_funcs);
> @@ -314,6 +315,117 @@ static const struct drm_fb_helper_funcs 
> drm_fb_cma_helper_funcs = {
>   .fb_probe = drm_fbdev_cma_create,
>  };
>  
> +/**
> + * drm_fb_cma_fbdev_init_with_funcs() - Allocate and initialize fbdev 
> emulation
> + * @dev: DRM device
> + * @preferred_bpp: Preferred bits per pixel for the device.
> + * @dev->mode_config.preferred_depth is used if this is zero.
> + * @max_conn_count: Maximum number of connectors.
> + *  @dev->mode_config.num_connector is used if this is zero.
> + * @funcs: Framebuffer functions, in particular a custom dirty() callback.

With the previous patch this is allowed to be NULL, right? Please
document.

And since that also explains the patch 1, on both patches:

Reviewed-by: Daniel Vetter 

But please get an ack from Laurent too.
-Daniel

> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
> + unsigned int preferred_bpp, unsigned int max_conn_count,
> + const struct drm_framebuffer_funcs *funcs)
> +{
> + struct drm_fbdev_cma *fbdev_cma;
> + struct drm_fb_helper *fb_helper;
> + int ret;
> +
> + if (!preferred_bpp)
> + preferred_bpp = dev->mode_config.preferred_depth;
> + if (!preferred_bpp)
> + preferred_bpp = 32;
> +
> + if (!max_conn_count)
> + max_conn_count = dev->mode_config.num_connector;
> +
> + fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
> + if (!fbdev_cma)
> + return -ENOMEM;
> +
> + fbdev_cma->fb_funcs = funcs;
> + fb_helper = &fbdev_cma->fb_helper;
> +
> + drm_fb_helper_prepare(dev, fb_helper, &drm_fb_cma_helper_funcs);
> +
> + ret = drm_fb_helper_init(dev, fb_helper, max_conn_count);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev->dev, "Failed to initialize fbdev helper.\n");
> + goto err_free;
> + }
> +
> + ret = drm_fb_helper_single_add_all_connectors(fb_helper);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev->dev, "Failed to add connectors.\n");
> + goto err_drm_fb_helper_fini;
> + }
> +
> + ret = drm_fb_helper_initial_config(fb_helper, preferred_bpp);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev->dev, "Failed to set fbdev configuration.\n");
> + goto err_drm_fb_helper_fini;
> + }
> +
> + return 0;
> +
> +err_drm_fb_helper_fini:
> + drm_fb_helper_fini(fb_helper);
> +err_free:
> + kfree(fbdev_cma);
> +
> + return ret;
> +}

Re: [PATCH 03/22] drm/arc: Use drm_fb_cma_fbdev_init/fini()

2017-11-06 Thread Daniel Vetter
On Sat, Nov 04, 2017 at 02:03:57PM +0100, Noralf Trønnes wrote:
> Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
> the fact that drm_device holds a pointer to the drm_fb_helper structure.
> This means that the driver doesn't have to keep track of that.
> Also use the drm_fb_helper functions directly.
> Remove unused function prototype arcpgu_fbdev_cma_init().
> 
> Cc: Alexey Brodkin 
> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/arc/arcpgu.h |  4 
>  drivers/gpu/drm/arc/arcpgu_drv.c | 36 +++-
>  2 files changed, 7 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
> index e8fcf3ab1d9a..90ef76b19f8a 100644
> --- a/drivers/gpu/drm/arc/arcpgu.h
> +++ b/drivers/gpu/drm/arc/arcpgu.h
> @@ -20,7 +20,6 @@
>  struct arcpgu_drm_private {
>   void __iomem*regs;
>   struct clk  *clk;
> - struct drm_fbdev_cma*fbdev;
>   struct drm_framebuffer  *fb;
>   struct drm_crtc crtc;
>   struct drm_plane*plane;
> @@ -43,8 +42,5 @@ static inline u32 arc_pgu_read(struct arcpgu_drm_private 
> *arcpgu,
>  int arc_pgu_setup_crtc(struct drm_device *dev);
>  int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np);
>  int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np);
> -struct drm_fbdev_cma *arcpgu_fbdev_cma_init(struct drm_device *dev,
> - unsigned int preferred_bpp, unsigned int num_crtc,
> - unsigned int max_conn_count);
>  
>  #endif
> diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c 
> b/drivers/gpu/drm/arc/arcpgu_drv.c
> index 074fd4ea7ece..f54481ee834c 100644
> --- a/drivers/gpu/drm/arc/arcpgu_drv.c
> +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
> @@ -16,6 +16,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -25,16 +26,9 @@
>  #include "arcpgu.h"
>  #include "arcpgu_regs.h"
>  
> -static void arcpgu_fb_output_poll_changed(struct drm_device *dev)
> -{
> - struct arcpgu_drm_private *arcpgu = dev->dev_private;
> -
> - drm_fbdev_cma_hotplug_event(arcpgu->fbdev);
> -}
> -
>  static const struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
>   .fb_create  = drm_gem_fb_create,
> - .output_poll_changed = arcpgu_fb_output_poll_changed,
> + .output_poll_changed = drm_fb_helper_output_poll_changed,
>   .atomic_check = drm_atomic_helper_check,
>   .atomic_commit = drm_atomic_helper_commit,
>  };
> @@ -51,13 +45,6 @@ static void arcpgu_setup_mode_config(struct drm_device 
> *drm)
>  
>  DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
>  
> -static void arcpgu_lastclose(struct drm_device *drm)
> -{
> - struct arcpgu_drm_private *arcpgu = drm->dev_private;
> -
> - drm_fbdev_cma_restore_mode(arcpgu->fbdev);
> -}
> -
>  static int arcpgu_load(struct drm_device *drm)
>  {
>   struct platform_device *pdev = to_platform_device(drm->dev);
> @@ -113,13 +100,9 @@ static int arcpgu_load(struct drm_device *drm)
>   drm_mode_config_reset(drm);
>   drm_kms_helper_poll_init(drm);
>  
> - arcpgu->fbdev = drm_fbdev_cma_init(drm, 16,
> -drm->mode_config.num_connector);
> - if (IS_ERR(arcpgu->fbdev)) {
> - ret = PTR_ERR(arcpgu->fbdev);
> - arcpgu->fbdev = NULL;
> - return -ENODEV;
> - }
> + ret = drm_fb_cma_fbdev_init(drm, 16, 0);

s/0/NULL/

Probably applies to other patches too.

Also, why did you merge this new change into the old patch with the
output_poll/lastclose change? It took me a while to figure out what you're
doing, because the patches looked familiar, but the cover letter didn't
mention how stuff evolved, nor the patches here.

Also, you've lost the bunch of acks/r-bs you've gathered already. If
there's no technical reason to merge the 2 patches I'm kinda leaning
towards merging them separately, at least the ones that have acks already.
Just to avoid the pitfall of the continually evolving patch series that
never lands.
-Daniel

> + if (ret)
> + return ret;
>  
>   platform_set_drvdata(pdev, drm);
>   return 0;
> @@ -127,12 +110,7 @@ static int arcpgu_load(struct drm_device *drm)
>  
>  static int arcpgu_unload(struct drm_device *drm)
>  {
> - struct arcpgu_drm_private *arcpgu = drm->dev_private;
> -
> - if (arcpgu->fbdev) {
> - drm_fbdev_cma_fini(arcpgu->fbdev);
> - arcpgu->fbdev = NULL;
> - }
> + drm_fb_cma_fbdev_fini(drm);
>   drm_kms_helper_poll_fini(drm);
>   drm_mode_config_cleanup(drm);
>  
> @@ -168,7 +146,7 @@ static int arcpgu_debugfs_init(struct drm_minor *minor)
>  static struct drm_driver arcpgu_drm_driver = {
>   .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
>  DRIVER_ATOMIC,
> - .lastclose = arcpgu_lastclose,
> + .lastclose = drm_fb_helper_lastclose,
>   .name = "arcpgu",
>   .desc = "

Re: [PATCH 22/22] drm/cma-helper: Remove drm_fbdev_cma* functions

2017-11-06 Thread Daniel Vetter
On Sat, Nov 04, 2017 at 02:04:16PM +0100, Noralf Trønnes wrote:
> Remove the unused struct drm_fbdev_cma functions.
> 
> Cc: Laurent Pinchart 
> Signed-off-by: Noralf Trønnes 

On patches 22&21:

Acked-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_fb_cma_helper.c | 158 
> 
>  include/drm/drm_fb_cma_helper.h |  28 +--
>  2 files changed, 3 insertions(+), 183 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 267c04216281..0c73957d7aad 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -425,161 +425,3 @@ void drm_fb_cma_fbdev_fini(struct drm_device *dev)
>   kfree(to_fbdev_cma(fb_helper));
>  }
>  EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_fini);
> -
> -/**
> - * drm_fbdev_cma_init_with_funcs() - Allocate and initializes a 
> drm_fbdev_cma struct
> - * @dev: DRM device
> - * @preferred_bpp: Preferred bits per pixel for the device
> - * @max_conn_count: Maximum number of connectors
> - * @funcs: fb helper functions, in particular a custom dirty() callback
> - *
> - * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR.
> - */
> -struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
> - unsigned int preferred_bpp, unsigned int max_conn_count,
> - const struct drm_framebuffer_funcs *funcs)
> -{
> - struct drm_fbdev_cma *fbdev_cma;
> - struct drm_fb_helper *helper;
> - int ret;
> -
> - fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
> - if (!fbdev_cma) {
> - dev_err(dev->dev, "Failed to allocate drm fbdev.\n");
> - return ERR_PTR(-ENOMEM);
> - }
> - fbdev_cma->fb_funcs = funcs;
> -
> - helper = &fbdev_cma->fb_helper;
> -
> - drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs);
> -
> - ret = drm_fb_helper_init(dev, helper, max_conn_count);
> - if (ret < 0) {
> - dev_err(dev->dev, "Failed to initialize drm fb helper.\n");
> - goto err_free;
> - }
> -
> - ret = drm_fb_helper_single_add_all_connectors(helper);
> - if (ret < 0) {
> - dev_err(dev->dev, "Failed to add connectors.\n");
> - goto err_drm_fb_helper_fini;
> -
> - }
> -
> - ret = drm_fb_helper_initial_config(helper, preferred_bpp);
> - if (ret < 0) {
> - dev_err(dev->dev, "Failed to set initial hw configuration.\n");
> - goto err_drm_fb_helper_fini;
> - }
> -
> - return fbdev_cma;
> -
> -err_drm_fb_helper_fini:
> - drm_fb_helper_fini(helper);
> -err_free:
> - kfree(fbdev_cma);
> -
> - return ERR_PTR(ret);
> -}
> -EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs);
> -
> -static const struct drm_framebuffer_funcs drm_fb_cma_funcs = {
> - .destroy= drm_gem_fb_destroy,
> - .create_handle  = drm_gem_fb_create_handle,
> -};
> -
> -/**
> - * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct
> - * @dev: DRM device
> - * @preferred_bpp: Preferred bits per pixel for the device
> - * @max_conn_count: Maximum number of connectors
> - *
> - * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR.
> - */
> -struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
> - unsigned int preferred_bpp, unsigned int max_conn_count)
> -{
> - return drm_fbdev_cma_init_with_funcs(dev, preferred_bpp,
> -  max_conn_count,
> -  &drm_fb_cma_funcs);
> -}
> -EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
> -
> -/**
> - * drm_fbdev_cma_fini() - Free drm_fbdev_cma struct
> - * @fbdev_cma: The drm_fbdev_cma struct
> - */
> -void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
> -{
> - drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper);
> - if (fbdev_cma->fb_helper.fbdev)
> - drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
> -
> - if (fbdev_cma->fb_helper.fb)
> - drm_framebuffer_remove(fbdev_cma->fb_helper.fb);
> -
> - drm_fb_helper_fini(&fbdev_cma->fb_helper);
> - kfree(fbdev_cma);
> -}
> -EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini);
> -
> -/**
> - * drm_fbdev_cma_restore_mode() - Restores initial framebuffer mode
> - * @fbdev_cma: The drm_fbdev_cma struct, may be NULL
> - *
> - * This function is usually called from the &drm_driver.lastclose callback.
> - */
> -void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma)
> -{
> - if (fbdev_cma)
> - 
> drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev_cma->fb_helper);
> -}
> -EXPORT_SYMBOL_GPL(drm_fbdev_cma_restore_mode);
> -
> -/**
> - * drm_fbdev_cma_hotplug_event() - Poll for hotpulug events
> - * @fbdev_cma: The drm_fbdev_cma struct, may be NULL
> - *
> - * This function is usually called from the 
> &drm_mode_config.output_poll_changed
> - * callback.
> - */
> -void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma)
> -{
> - if (fbdev_cma)
> -  

Re: [Intel-gfx] [PATCH v5 2/7] drm: Add panel orientation quirks

2017-11-06 Thread Daniel Vetter
On Sat, Nov 04, 2017 at 03:08:23PM +0100, Hans de Goede wrote:
> Some x86 clamshell design devices use portrait tablet screens and a display
> engine which cannot rotate in hardware, so the firmware just leaves things
> as is and we cannot figure out that the display is oriented non upright
> from the hardware.
> 
> So at least on x86, we need a quirk table for this. This commit adds a DMI
> based quirk table which is initially populated with 5 such devices: Asus
> T100HA, GPD Pocket, GPD win, I.T.Works TW891 and the VIOS LTH17.
> 
> This quirk table will be used by the drm code to let userspace know that
> the display is not mounted upright inside the devices case through a new
> panel orientation drm-connector property, as well as to tell fbcon to
> rotate the console so that it shows the right way up.
> 
> Signed-off-by: Hans de Goede 
> ---
> Changes in v5:
> -Add a kernel-doc comment documenting drm_get_panel_orientation_quirk()

The kerneldoc isn't much use without pulling it into the overall
structure. Documentation/gpu/drm-kms-helper.rst seems like a suitable
place. Pls also check it builds correctly and looks pretty using

$ make htmldocs

With that fixed:

Reviewed-by: Daniel Vetter 

> -Remove board_* matches from the dmi-matches for the VIOS LTH17 laptop,
>  keeping only the (identical) sys_vendor and product_name matches.
>  This is necessary because an older version of the bios has
>  board_vendor set to VOIS instead of VIOS
> ---
>  drivers/gpu/drm/Kconfig|   3 +
>  drivers/gpu/drm/Makefile   |   1 +
>  drivers/gpu/drm/drm_panel_orientation_quirks.c | 174 
> +
>  include/drm/drm_utils.h|  18 +++
>  4 files changed, 196 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_panel_orientation_quirks.c
>  create mode 100644 include/drm/drm_utils.h
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 4d9f21831741..9d005ac98c2b 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -26,6 +26,9 @@ config DRM_MIPI_DSI
>   bool
>   depends on DRM
>  
> +config DRM_PANEL_ORIENTATION_QUIRKS
> + tristate
> +
>  config DRM_DP_AUX_CHARDEV
>   bool "DRM DP AUX Interface"
>   depends on DRM
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index a3fdc5a68dff..ffb621819bbf 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -46,6 +46,7 @@ obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/
>  
>  obj-$(CONFIG_DRM)+= drm.o
>  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> +obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
>  obj-$(CONFIG_DRM_ARM)+= arm/
>  obj-$(CONFIG_DRM_TTM)+= ttm/
>  obj-$(CONFIG_DRM_TDFX)   += tdfx/
> diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c 
> b/drivers/gpu/drm/drm_panel_orientation_quirks.c
> new file mode 100644
> index ..b8765e2ed1d6
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
> @@ -0,0 +1,174 @@
> +/*
> + *  drm_panel_orientation_quirks.c -- Quirks for non-normal panel orientation
> + *
> + *   Copyright (C) 2017 Hans de Goede 
> + *
> + *  This file is subject to the terms and conditions of the GNU General 
> Public
> + *  License.  See the file COPYING in the main directory of this archive for
> + *  more details.
> + */
> +
> +#include 
> +#include 
> +
> +#ifdef CONFIG_DMI
> +
> +/*
> + * Some x86 clamshell design devices use portrait tablet screens and a 
> display
> + * engine which cannot rotate in hardware, so we need to rotate the fbcon to
> + * compensate. Unfortunately these (cheap) devices also typically have quite
> + * generic DMI data, so we match on a combination of DMI data, screen 
> resolution
> + * and a list of known BIOS dates to avoid false positives.
> + */
> +
> +struct drm_dmi_panel_orientation_data {
> + int width;
> + int height;
> + const char * const *bios_dates;
> + int orientation;
> +};
> +
> +static const struct drm_dmi_panel_orientation_data asus_t100ha = {
> + .width = 800,
> + .height = 1280,
> + .orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP,
> +};
> +
> +static const struct drm_dmi_panel_orientation_data gpd_pocket = {
> + .width = 1200,
> + .height = 1920,
> + .bios_dates = (const char * const []){ "05/26/2017", "06/28/2017",
> + "07/05/2017", "08/07/2017", NULL },
> + .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
> +};
> +
> +static const struct drm_dmi_panel_orientation_data gpd_win = {
> + .width = 720,
> + .height = 1280,
> + .bios_dates = (const char * const []){
> + "10/25/2016", "11/18/2016", "12/23/2016", "12/26/2016",
> + "02/21/2017", "03/20/2017", "05/25/2017", NULL },
> + .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
> +};
> +
> +static const struct drm_dmi_panel_orientation_data itworks_tw891 = {
> + .width = 800,

Re: [Intel-gfx] [PATCH v5 3/7] drm: Add support for a panel-orientation connector property

2017-11-06 Thread Daniel Vetter
On Sat, Nov 04, 2017 at 03:08:24PM +0100, Hans de Goede wrote:
> On some devices the LCD panel is mounted in the casing in such a way that
> the up/top side of the panel does not match with the top side of the
> device (e.g. it is mounted upside-down).
> 
> This commit adds the necessary infra for lcd-panel drm_connector-s to
> have a "panel orientation" property to communicate how the panel is
> orientated vs the casing.
> 
> Userspace can use this property to check for non-normal orientation and
> then adjust the displayed image accordingly by rotating it to compensate.
> 
> Signed-off-by: Hans de Goede 
> ---

btw in drm we'd like to record the per-patch changelog, pls put them above
the ---. I know everyone's different, it's silly.

> Changes in v2:
> -Rebased on 4.14-rc1
> -Store panel_orientation in drm_display_info, so that drm_fb_helper.c can
>  access it easily
> -Have a single drm_connector_init_panel_orientation_property rather then
>  create and attach functions. The caller is expected to set
>  drm_display_info.panel_orientation before calling this, then this will
>  check for platform specific quirks overriding the panel_orientation and if
>  the panel_orientation is set after this then it will attach the property.
> ---
>  drivers/gpu/drm/Kconfig |  1 +
>  drivers/gpu/drm/drm_connector.c | 73 
> +
>  include/drm/drm_connector.h | 11 +++
>  include/drm/drm_mode_config.h   |  7 
>  include/uapi/drm/drm_mode.h |  7 
>  5 files changed, 99 insertions(+)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 9d005ac98c2b..0b166e626eb6 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -7,6 +7,7 @@
>  menuconfig DRM
>   tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI 
> support)"
>   depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA
> + select DRM_PANEL_ORIENTATION_QUIRKS
>   select HDMI
>   select FB_CMDLINE
>   select I2C
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 704fc8934616..129c83a84320 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "drm_crtc_internal.h"
>  #include "drm_internal.h"
> @@ -212,6 +213,8 @@ int drm_connector_init(struct drm_device *dev,
>   mutex_init(&connector->mutex);
>   connector->edid_blob_ptr = NULL;
>   connector->status = connector_status_unknown;
> + connector->display_info.panel_orientation =
> + DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
>  
>   drm_connector_get_cmdline_mode(connector);
>  
> @@ -664,6 +667,13 @@ static const struct drm_prop_enum_list 
> drm_aspect_ratio_enum_list[] = {
>   { DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
>  };
>  
> +static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = {
> + { DRM_MODE_PANEL_ORIENTATION_NORMAL,"Normal"},
> + { DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down"   },
> + { DRM_MODE_PANEL_ORIENTATION_LEFT_UP,   "Left Side Up"  },
> + { DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,  "Right Side Up" },
> +};
> +
>  static const struct drm_prop_enum_list drm_dvi_i_select_enum_list[] = {
>   { DRM_MODE_SUBCONNECTOR_Automatic, "Automatic" }, /* DVI-I and TV-out */
>   { DRM_MODE_SUBCONNECTOR_DVID,  "DVI-D" }, /* DVI-I  */
> @@ -768,6 +778,18 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name,
>   *
>   * CRTC_ID:
>   *   Mode object ID of the &drm_crtc this connector should be connected to.
> + *
> + * Connectors for LCD panels may also have one standardized property:
> + *
> + * panel orientation:
> + *   On some devices the LCD panel is mounted in the casing in such a way
> + *   that the up/top side of the panel does not match with the top side of
> + *   the device. Userspace can use this property to check for this.
> + *   Note that input coordinates from touchscreens (input devices with
> + *   INPUT_PROP_DIRECT) will still map 1:1 to the actual LCD panel
> + *   coordinates, so if userspace rotates the picture to adjust for
> + *   the orientation it must also apply the same transformation to the
> + *   touchscreen input coordinates.
>   */
>  
>  int drm_connector_create_standard_properties(struct drm_device *dev)
> @@ -1234,6 +1256,57 @@ void 
> drm_mode_connector_set_link_status_property(struct drm_connector *connector
>  }
>  EXPORT_SYMBOL(drm_mode_connector_set_link_status_property);
>  
> +/**
> + * drm_connector_init_panel_orientation_property -
> + *   initialize the connecters panel_orientation property
> + * @connector: connector for which to init the panel-orientation property.
> + * @width: width in pixels of the panel, used for panel quirk detection
> + * @height: height in pixels of the panel, used for panel quirk detection
> + *
> + * This function should only be called for built-in panels, a

Re: [Intel-gfx] [PATCH v5 4/7] drm/fb-helper: Apply panel orientation connector prop to the primary plane

2017-11-06 Thread Daniel Vetter
On Sat, Nov 04, 2017 at 03:08:25PM +0100, Hans de Goede wrote:
> Apply the "panel orientation" drm connector prop to the primary plane so
> that fbcon and fbdev using userspace programs display the right way up.
> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=94894
> Signed-off-by: Hans de Goede 
> ---
> Changes in v2:
> -New patch in v2 of this patch-set
> 
> Changes in v3:
> -Use a rotation member in struct drm_fb_helper_crtc and set that from
>  drm_setup_crtcs instead of looping over all crtc's to find the right one
>  later
> -Since we now no longer look at rotation quirks directly in the fbcon
>  code, set fb_info.fbcon_rotate_hint when the panel is not mounted upright
>  and we cannot use hardware rotation
> 
> Changes in v4:
> -Make drm_fb_helper_init() init drm_fb_helper_crtc.rotation to
>  DRM_MODE_ROTATE_0 for all crtcs, so that we do not end up setting the
>  plane_state's rotation to an invalid value for disabled crtcs
>  (caught by Fi.CI)
> 
> Changes in v5:
> -Only use hardware (crtc primary plane) rotation for DRM_ROTATE_180,
>  90 / 270 degree rotation requires special handling which we lack atm
> -Add a TODO comment for 90 / 270 degree hardware rotation
> -Add some comments to better document the default case when mapping
>  sw_rotations to fbcon_rotate_hints

Yeah the above stuff really needs to be in the commit message. Or you need
to explain in prose, but just copying the changelog is easier.

People forgetting to update the patch justification is the no1 reason we
want to record the changelogs ...

Otherwise lgtm

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 90 
> -
>  include/drm/drm_fb_helper.h |  8 
>  2 files changed, 96 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 116d1f1337c7..5a557488bff4 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -41,6 +41,7 @@
>  #include 
>  #include 
>  
> +#include "drm_crtc_internal.h"
>  #include "drm_crtc_helper_internal.h"
>  
>  static bool drm_fbdev_emulation = true;
> @@ -350,6 +351,7 @@ EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>  static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool 
> active)
>  {
>   struct drm_device *dev = fb_helper->dev;
> + struct drm_plane_state *plane_state;
>   struct drm_plane *plane;
>   struct drm_atomic_state *state;
>   int i, ret;
> @@ -368,8 +370,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper 
> *fb_helper, bool activ
>  retry:
>   plane_mask = 0;
>   drm_for_each_plane(plane, dev) {
> - struct drm_plane_state *plane_state;
> -
>   plane_state = drm_atomic_get_plane_state(state, plane);
>   if (IS_ERR(plane_state)) {
>   ret = PTR_ERR(plane_state);
> @@ -392,6 +392,11 @@ static int restore_fbdev_mode_atomic(struct 
> drm_fb_helper *fb_helper, bool activ
>  
>   for (i = 0; i < fb_helper->crtc_count; i++) {
>   struct drm_mode_set *mode_set = 
> &fb_helper->crtc_info[i].mode_set;
> + struct drm_plane *primary = mode_set->crtc->primary;
> +
> + /* Cannot fail as we've already gotten the plane state above */
> + plane_state = drm_atomic_get_new_plane_state(state, primary);
> + plane_state->rotation = fb_helper->crtc_info[i].rotation;
>  
>   ret = __drm_atomic_helper_set_config(mode_set, state);
>   if (ret != 0)
> @@ -821,6 +826,7 @@ int drm_fb_helper_init(struct drm_device *dev,
>   if (!fb_helper->crtc_info[i].mode_set.connectors)
>   goto out_free;
>   fb_helper->crtc_info[i].mode_set.num_connectors = 0;
> + fb_helper->crtc_info[i].rotation = DRM_MODE_ROTATE_0;
>   }
>  
>   i = 0;
> @@ -2334,6 +2340,62 @@ static int drm_pick_crtcs(struct drm_fb_helper 
> *fb_helper,
>   return best_score;
>  }
>  
> +/*
> + * This function checks if rotation is necessary because of panel orientation
> + * and if it is, if it is supported.
> + * If rotation is necessary and supported, its gets set in fb_crtc.rotation.
> + * If rotation is necessary but not supported, a DRM_MODE_ROTATE_* flag gets
> + * or-ed into fb_helper->sw_rotations. In drm_setup_crtcs_fb() we check if 
> only
> + * one bit is set and then we set fb_info.fbcon_rotate_hint to make fbcon do
> + * the unsupported rotation.
> + */
> +static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
> + struct drm_fb_helper_crtc *fb_crtc,
> + struct drm_connector *connector)
> +{
> + struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
> + uint64_t valid_mask = 0;
> + int i, rotation;
> +
> + fb_crtc->rotation = DRM_MODE_ROTATE_0;
> +
> + switch (connector->display_info.panel_orientation) {
> +  

Re: video: atmel_lcdfb: Use common error handling code in atmel_lcdfb_of_init()

2017-11-06 Thread SF Markus Elfring
> Sorry but NACK: the message was malformed and resulted in the
> duplication of the error log that you spotted.
> 
> The proper way to fix this is to modify the second occurrence of this message.

* Would you like to achieve that a corresponding message will mention
  anything around a property “atmel,dmacon” (instead of “bits-per-pixel”)?

* Can it make sense to adjust the used message format then?

Regards,
Markus
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: video: atmel_lcdfb: Use common error handling code in atmel_lcdfb_of_init()

2017-11-06 Thread Dan Carpenter
On Mon, Nov 06, 2017 at 10:00:25AM +0100, SF Markus Elfring wrote:
> >> If you want to lower the size of strings in this driver, you can do it,
> >> but not like this.
> > 
> > Just so we're clear, GCC already detects and combines it when you use
> > the same string constant twice.
> 
> Do you distinguish between merging of constants and the combination
> of statements for such an use case?

I would have rejected the patch even if GCC didn't combine the strings
because the most important thing is that the code is readable.

regards,
dan carpenter

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 07/22] drm/imx: Use drm_fb_cma_fbdev_init/fini()

2017-11-06 Thread Philipp Zabel
On Sat, 2017-11-04 at 14:04 +0100, Noralf Trønnes wrote:
> Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
> the fact that drm_device holds a pointer to the drm_fb_helper structure.
> This means that the driver doesn't have to keep track of that.
> Also use the drm_fb_helper functions directly.
> 
> Cc: Philipp Zabel 
> Signed-off-by: Noralf Trønnes 

Acked-by: Philipp Zabel 

regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 99843] Geometry Shader - Incorrect Output

2017-11-06 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=99843

--- Comment #14 from d...@jerber.co.uk ---
The 4890 has a RV790 GPU.

I've run the trace again and also see an error similar to the one Gert
mentioned:

1639 Message api issue 1: FBO incomplete: no attachments and default width or
height is 0 [-1].

There are also more errors reported around glDrawArrays as follows:

major api error 2: GL_INVALID_ENUM in
glGetIntegerV(pname=GL_TEXTURE_BINDING_CUBE_MAP_ARRAY)

warning: dumping of normalized attribute (vertexCoord) not yet supported

warning: dumping of normalized attribute (textureCoord) not yet supported

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 05/22] drm/atmel-hlcdc: Use drm_fb_cma_fbdev_init/fini()

2017-11-06 Thread Boris Brezillon
On Sat,  4 Nov 2017 14:03:59 +0100
Noralf Trønnes  wrote:

> Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
> the fact that drm_device holds a pointer to the drm_fb_helper structure.
> This means that the driver doesn't have to keep track of that.
> Also use the drm_fb_helper functions directly.
> 
> Cc: Boris Brezillon 

Acked-by: Boris Brezillon 

> Signed-off-by: Noralf Trønnes 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 26 --
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h |  2 +-
>  2 files changed, 5 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index c6e8061ffcfc..c1ea5c36b006 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -461,13 +461,6 @@ static struct drm_framebuffer 
> *atmel_hlcdc_fb_create(struct drm_device *dev,
>   return drm_gem_fb_create(dev, file_priv, mode_cmd);
>  }
>  
> -static void atmel_hlcdc_fb_output_poll_changed(struct drm_device *dev)
> -{
> - struct atmel_hlcdc_dc *dc = dev->dev_private;
> -
> - drm_fbdev_cma_hotplug_event(dc->fbdev);
> -}
> -
>  struct atmel_hlcdc_dc_commit {
>   struct work_struct work;
>   struct drm_device *dev;
> @@ -563,7 +556,7 @@ static int atmel_hlcdc_dc_atomic_commit(struct drm_device 
> *dev,
>  
>  static const struct drm_mode_config_funcs mode_config_funcs = {
>   .fb_create = atmel_hlcdc_fb_create,
> - .output_poll_changed = atmel_hlcdc_fb_output_poll_changed,
> + .output_poll_changed = drm_fb_helper_output_poll_changed,
>   .atomic_check = drm_atomic_helper_check,
>   .atomic_commit = atmel_hlcdc_dc_atomic_commit,
>  };
> @@ -665,10 +658,7 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>  
>   platform_set_drvdata(pdev, dev);
>  
> - dc->fbdev = drm_fbdev_cma_init(dev, 24,
> - dev->mode_config.num_connector);
> - if (IS_ERR(dc->fbdev))
> - dc->fbdev = NULL;
> + drm_fb_cma_fbdev_init(dev, 24, 0);
>  
>   drm_kms_helper_poll_init(dev);
>  
> @@ -688,8 +678,7 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
>  {
>   struct atmel_hlcdc_dc *dc = dev->dev_private;
>  
> - if (dc->fbdev)
> - drm_fbdev_cma_fini(dc->fbdev);
> + drm_fb_cma_fbdev_fini(dev);
>   flush_workqueue(dc->wq);
>   drm_kms_helper_poll_fini(dev);
>   drm_mode_config_cleanup(dev);
> @@ -705,13 +694,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
>   destroy_workqueue(dc->wq);
>  }
>  
> -static void atmel_hlcdc_dc_lastclose(struct drm_device *dev)
> -{
> - struct atmel_hlcdc_dc *dc = dev->dev_private;
> -
> - drm_fbdev_cma_restore_mode(dc->fbdev);
> -}
> -
>  static int atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev)
>  {
>   struct atmel_hlcdc_dc *dc = dev->dev_private;
> @@ -744,7 +726,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>   .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
>  DRIVER_MODESET | DRIVER_PRIME |
>  DRIVER_ATOMIC,
> - .lastclose = atmel_hlcdc_dc_lastclose,
> + .lastclose = drm_fb_helper_lastclose,
>   .irq_handler = atmel_hlcdc_dc_irq_handler,
>   .irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>   .irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index 6833ee253cfa..ab32d5b268d2 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -32,6 +32,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -374,7 +375,6 @@ struct atmel_hlcdc_dc {
>   const struct atmel_hlcdc_dc_desc *desc;
>   struct dma_pool *dscrpool;
>   struct atmel_hlcdc *hlcdc;
> - struct drm_fbdev_cma *fbdev;
>   struct drm_crtc *crtc;
>   struct atmel_hlcdc_layer *layers[ATMEL_HLCDC_MAX_LAYERS];
>   struct workqueue_struct *wq;

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Intel-gfx] [PATCH v5 5/7] drm/i915: Add "panel orientation" property to the panel connector

2017-11-06 Thread Daniel Vetter
On Sat, Nov 04, 2017 at 03:08:26PM +0100, Hans de Goede wrote:
> Ideally we could use the VBT for this, that would be simple, in
> intel_dsi_init() check dev_priv->vbt.dsi.config->rotation, set
> connector->display_info.panel_orientation accordingly and call
> drm_connector_init_panel_orientation_property(), done.
> 
> Unfortunately vbt.dsi.config->rotation is always 0 even on tablets
> with an upside down LCD and where the GOP is properly rotating the
> EFI fb in hardware.
> 
> So instead we end up reading the rotation from the primary plane.
> To read the info from the primary plane, we need to know which crtc
> the panel is hooked up to, so we do this the first time the panel
> encoder's get_config function get called, as by then the encoder
> crtc routing has been set up.
> 
> This commit only implements the panel orientation property for DSI
> panels on BYT / CHT / BXT hardware, as all known non normal oriented
> panels are only found on this hardware.
> 
> Signed-off-by: Hans de Goede 
> ---
> Changes in v2:
> -Read back the rotation applied by the GOP from the primary plane
>  instead of relying on dev_priv->vbt.dsi.config->rotation, because it
>  seems that the VBT rotation filed is always 0 even on devices where the
>  GOP does apply a rotation
> 
> Changes in v3:
> -Rewrite the code to read back the orientation from the primary
>  plane to contain all of this in intel_dsi.c instead of poking a bunch
>  of holes between all the different layers
> ---
>  drivers/gpu/drm/i915/intel_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_dsi.c   | 48 
> ++
>  drivers/gpu/drm/i915/intel_dsi.h   |  2 ++
>  drivers/gpu/drm/i915/intel_panel.c | 16 +
>  4 files changed, 67 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 47d022d48718..11efc6cb74c8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1727,6 +1727,7 @@ void intel_panel_enable_backlight(const struct 
> intel_crtc_state *crtc_state,
> const struct drm_connector_state *conn_state);
>  void intel_panel_disable_backlight(const struct drm_connector_state 
> *old_conn_state);
>  void intel_panel_destroy_backlight(struct drm_connector *connector);
> +void intel_panel_set_orientation(struct intel_panel *panel, int orientation);
>  enum drm_connector_status intel_panel_detect(struct drm_i915_private 
> *dev_priv);
>  extern struct drm_display_mode *intel_find_panel_downclock(
>   struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c 
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 83f15848098a..3e2f12db8d15 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1084,13 +1084,16 @@ static void bxt_dsi_get_pipe_config(struct 
> intel_encoder *encoder,
>   struct drm_display_mode *adjusted_mode_sw;
>   struct intel_crtc *intel_crtc;
>   struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> + struct intel_panel *panel = &intel_dsi->attached_connector->panel;
>   unsigned int lane_count = intel_dsi->lane_count;
>   unsigned int bpp, fmt;
> + int orientation;
>   enum port port;
>   u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
>   u16 hfp_sw, hsync_sw, hbp_sw;
>   u16 crtc_htotal_sw, crtc_hsync_start_sw, crtc_hsync_end_sw,
>   crtc_hblank_start_sw, crtc_hblank_end_sw;
> + u32 val;
>  
>   /* FIXME: hw readout should not depend on SW state */
>   intel_crtc = to_intel_crtc(encoder->base.crtc);
> @@ -1234,6 +1237,49 @@ static void bxt_dsi_get_pipe_config(struct 
> intel_encoder *encoder,
>   if (adjusted_mode->crtc_hblank_end == crtc_hblank_end_sw)
>   adjusted_mode->crtc_hblank_end =
>   adjusted_mode_sw->crtc_hblank_end;
> +
> + if (!intel_dsi->got_panel_orientation) {
> + val = I915_READ(PLANE_CTL(intel_crtc->pipe, 0));
> + /* The rotation is used to correct for the panel orientation */
> + switch (val & PLANE_CTL_ROTATE_MASK) {
> + case PLANE_CTL_ROTATE_0:
> + orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
> + break;
> + case PLANE_CTL_ROTATE_90:
> + orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
> + break;
> + case PLANE_CTL_ROTATE_180:
> + orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> + break;
> + case PLANE_CTL_ROTATE_270:
> + orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
> + break;
> + }
> + intel_panel_set_orientation(panel, orientation);
> + intel_dsi->got_panel_orientation = true;
> + }
> +}
> +
> +static void vlv_dsi_get_pipe_config(stru

Re: [PATCHv1 00/14] omapdrm: DSI command mode panel support

2017-11-06 Thread Pavel Machek
Hi!

> > P.S.: I got asked by different people about the status of this
> > patchset, which is required for display support on N9, N950 and
> > Droid 4. It's not just me and Tony, that are interested in this :)
> 
> Ok. If you agree to help with the DSI part in the future, I have no
> problems applying these (after review and testing, of course).
> 
> My worry is not only with complicating Laurent's work towards common DRM
> drivers, but also with the maintenance burden this brings. Keeping DSI
> working in the future may be challenging due to the lack of users and
> (easy to use) boards. I do have a N950 and OMAP4 Blaze Tablet, though,
> so I'm able to test both command and video modes. But getting those boot
> up is not always trivial, especially for the blaze tablet.

Well... Yes, they are challenging to boot, because without working
display, debugging them is hard/impossible. Very few people have
serial cables for those, but at least the N9 is pretty common.

PostmarketOS is interested in using it as a target. I'm interested in
keeping it going. With working display, I'm pretty sure I can prevent
regressions... and there could be even some improvements :-).

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH hwc] Android: add CleanSpec.mk

2017-11-06 Thread Robert Foss
Hey Rob,

On Fri, 2017-11-03 at 07:35 -0500, Rob Herring wrote:
> On Thu, Nov 2, 2017 at 11:45 PM, Chih-Wei Huang  org> wrote:
> > The file contains rules that are executed on incremental builds.
> > Since commit 4f7dc9b6 the library was moved to /vendor so
> > the old file must be removed.
> > 
> > Signed-off-by: Chih-Wei Huang 
> > ---
> >  CleanSpec.mk | 1 +
> >  1 file changed, 1 insertion(+)
> >  create mode 100644 CleanSpec.mk
> > 
> > diff --git a/CleanSpec.mk b/CleanSpec.mk
> > new file mode 100644
> > index 000..99dcecd
> > --- /dev/null
> > +++ b/CleanSpec.mk
> > @@ -0,0 +1 @@
> > +$(call add-clean-step, rm -rf
> > $(TARGET_OUT)/lib*/hw/hwcomposer.drm.so)
> 
> Seems a bit silly to add forever an explicit file to clean for a
> transient problem.
> 
While transient, if maintaining multuple Android versions it may still
stick around for quite some time.

That being said, I don't have much of an opinion about this.


Rob.

signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


AMD, please run Smatch on your driver

2017-11-06 Thread Dan Carpenter
Linux-next was offline for the last month and the AMD drm driver went
through major changes.  Anyway, I'm a bit overwhelmed by the number of
warnings and I'm not going to be able to go through them all so I'm just
sending them to you unfiltered.

Part of the problem is that I'm not running the released version of
Smatch myself.  That has two effects.  1) The released version is
crappier than I had imagined.  2) I get *way* more warnings than you see
which is overwhelming...  So this is mostly my fault and I will try to
do better.

Here are the current warnings from Friday's linux-next, lightly edited.
I know that everyone hates a big dump of static checker warnings...
Speaking of being ignored, I sent a fix for this one back in August but
never heard back:

  drivers/gpu/drm/amd/amdgpu/ci_dpm.c:4553 ci_set_mc_special_registers()
  error: buffer overflow 'table->mc_reg_address' 16 <= 16

https://lists.freedesktop.org/archives/amd-gfx/2017-August/012333.html

So this is partly your fault as well because if you cleaned up static
checker warnings little by little, then they wouldn't pile up like this.
Eventually, everyone is going to have to start running Smatch for
themselves because it scales better than relying on me to do it.

regards,
dan carpenter

drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:2224 amdgpu_device_init() warn: 
'adev->rio_mem' was not released on error
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:2395 amdgpu_device_init() warn: 
'adev->rio_mem' was not released on error
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3373 amdgpu_debugfs_regs_write() 
warn: 'mutex:&adev->pm.mutex' is sometimes locked here and sometimes unlocked.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3377 amdgpu_debugfs_regs_write() 
warn: 'mutex:&adev->pm.mutex' is sometimes locked here and sometimes unlocked.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3771 amdgpu_debugfs_gpr_read() 
error: buffer overflow 'data' 1024 <= 4095
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:155 amdgpu_driver_load_kms() warn: we 
tested 'r' before and it was 'false'
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c:689 amdgpu_gem_op_ioctl() warn: should 
'robj->tbo.mem.page_alignment << 12' be a 64 bit type?
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:196 amdgpu_cs_parser_init() warn: 
'mutex:&p->ctx->lock' is sometimes locked here and sometimes unlocked.
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:674 amdgpu_cs_parser_bos() warn: we 
tested 'r' before and it was 'false'
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:755 amdgpu_cs_parser_fini() warn: 
'mutex:&parser->ctx->lock' is sometimes locked here and sometimes unlocked.
drivers/gpu/drm/amd/amdgpu/atombios_i2c.c:72 
amdgpu_atombios_i2c_process_i2c_ch() warn: impossible condition '(num > 255) => 
(0-255 > 255)'
drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c:217 amdgpu_queue_mgr_map() warn: 
variable dereferenced before check 'mgr' (see line 215)
drivers/gpu/drm/amd/amdgpu/kv_dpm.c:1618 kv_get_acp_boot_level() warn: always 
true condition '(table->entries[i]->clk >= 0) => (0-u32max >= 0)'
drivers/gpu/drm/amd/amdgpu/ci_dpm.c:4560 ci_set_mc_special_registers() error: 
buffer overflow 'table->mc_reg_address' 16 <= 16
drivers/gpu/drm/amd/amdgpu/ci_dpm.c:5065 
ci_request_link_speed_change_before_state_change() warn: missing break? 
reassigning 'pi->force_pcie_gen'
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:5256 gfx_v7_0_get_cu_info() error: buffer 
overflow 'cu_info->bitmap' 4 <= 4
drivers/gpu/drm/amd/amdgpu/si.c:1288 si_common_early_init() warn: inconsistent 
indenting
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c:3026 dce_v6_0_pageflip_irq() warn: 
inconsistent indenting
drivers/gpu/drm/amd/amdgpu/si_dpm.c:6242 
si_request_link_speed_change_before_state_change() warn: missing break? 
reassigning 'si_pi->force_pcie_gen'
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:5222 gfx_v8_0_pre_soft_reset() warn: 
inconsistent indenting
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:7105 gfx_v8_0_get_cu_info() error: buffer 
overflow 'cu_info->bitmap' 4 <= 4
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:3077 gfx_v9_0_soft_reset() warn: we 
tested 'grbm_soft_reset' before and it was 'true'
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:3644 gfx_v9_0_ring_emit_ib_gfx() warn: 
inconsistent indenting
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:4457 gfx_v9_0_get_cu_info() error: buffer 
overflow 'cu_info->bitmap' 4 <= 4
drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c:605 amdgpu_cgs_lock_grbm_idx() warn: 
'mutex:&adev->grbm_idx_mutex' is sometimes locked here and sometimes unlocked.
drivers/gpu/drm/amd/amdgpu/../scheduler/gpu_scheduler.c:696 amd_sched_init() 
warn: call of 'kthread_create_on_node' with non-constant format argument
drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/tonga_smumgr.c:3128 
tonga_set_mc_special_registers() error: buffer overflow 'table->mc_reg_address' 
16 <= 16
drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/polaris10_smumgr.c:916 
polaris10_calculate_sclk_params() warn: should 'clock << 
table->SclkFcwRangeTable[sclk_setting->PllRange].postdiv' be a 64 bit type?
drivers/gpu/drm/amd/amdgp

Re: [PATCH] backlight: ili922x: remove redundant variable len

2017-11-06 Thread Daniel Thompson

On 05/11/17 11:53, Colin King wrote:

From: Colin Ian King 

The variable len is assigned but never read, therefore it is redundant
and can be removed. Cleans up clang warning:

drivers/video/backlight/ili922x.c:276:2: warning: Value stored to 'len'
is never read

Signed-off-by: Colin Ian King 


Acked-by: Daniel Thompson 


---
  drivers/video/backlight/ili922x.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/video/backlight/ili922x.c 
b/drivers/video/backlight/ili922x.c
index a9e9cef..2b6c6aa 100644
--- a/drivers/video/backlight/ili922x.c
+++ b/drivers/video/backlight/ili922x.c
@@ -251,7 +251,7 @@ static int ili922x_write(struct spi_device *spi, u8 reg, 
u16 value)
struct spi_transfer xfer_regindex, xfer_regvalue;
unsigned char tbuf[CMD_BUFSIZE];
unsigned char rbuf[CMD_BUFSIZE];
-   int ret, len = 0;
+   int ret;
  
  	memset(&xfer_regindex, 0, sizeof(struct spi_transfer));

memset(&xfer_regvalue, 0, sizeof(struct spi_transfer));
@@ -273,7 +273,6 @@ static int ili922x_write(struct spi_device *spi, u8 reg, 
u16 value)
ret = spi_sync(spi, &msg);
  
  	spi_message_init(&msg);

-   len = 0;
tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
 START_RW_WRITE));
tbuf[1] = set_tx_byte((value & 0xFF00) >> 8);



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/amd/display: checking for NULL instead of IS_ERR()

2017-11-06 Thread Dan Carpenter
backlight_device_register() never returns NULL, it returns error
pointers on error so the check here is wrong.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 33a15a1d818c..75f9383f5b9b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1296,7 +1296,7 @@ amdgpu_dm_register_backlight_device(struct 
amdgpu_display_manager *dm)
&amdgpu_dm_backlight_ops,
&props);
 
-   if (NULL == dm->backlight_dev)
+   if (IS_ERR(dm->backlight_dev))
DRM_ERROR("DM: Backlight registration failed!\n");
else
DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n", 
bl_name);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/amd/display: remove some unneeded code

2017-11-06 Thread Dan Carpenter
We assign "v_init = asic_blank_start;" a few lines earlier so there is
no need to do it again inside the if statements.  Also "v_init" is
unsigned so it can't be less than zero.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
index 1994865d4351..c7333cdf1802 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
@@ -236,13 +236,10 @@ static void tgn10_program_timing(
if (tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT ||
tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
tg->dlg_otg_param.signal == SIGNAL_TYPE_EDP) {
-   v_init = asic_blank_start;
start_point = 1;
if (patched_crtc_timing.flags.INTERLACE == 1)
field_num = 1;
}
-   if (v_init < 0)
-   v_init = 0;
v_fp2 = 0;
if (tg->dlg_otg_param.vstartup_start > asic_blank_end)
v_fp2 = tg->dlg_otg_param.vstartup_start > asic_blank_end;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 101483] A10-8780P (Carrizo) + R7 M260/M265 does not boot without any "workaround" parameter.

2017-11-06 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=101483

--- Comment #22 from FFAB  ---
Upstream kernel test - kernel 4.14-rc8

Installation, on which upstream kernel was installed:

ubuntu 16.04.3, kernel4.10.0-38, updated 2017-11-06

1. booting without any workaround parameters
2. booting with kernel parameter iommu=soft

boot-time 45sec (message sound),
black screen
remote-login (ssh) o.k.

attachments: dmesg_4.14-rc8.text, dmesg-iommu_soft_4.14-rc8.text

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 101483] A10-8780P (Carrizo) + R7 M260/M265 does not boot without any "workaround" parameter.

2017-11-06 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=101483

--- Comment #23 from FFAB  ---
Created attachment 135255
  --> https://bugs.freedesktop.org/attachment.cgi?id=135255&action=edit
dmesg upstream kernel 4.14-rc8, 2017-11-06

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 101483] A10-8780P (Carrizo) + R7 M260/M265 does not boot without any "workaround" parameter.

2017-11-06 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=101483

FFAB  changed:

   What|Removed |Added

 Attachment #135182|0   |1
is obsolete||
 Attachment #135183|0   |1
is obsolete||

--- Comment #24 from FFAB  ---
Created attachment 135256
  --> https://bugs.freedesktop.org/attachment.cgi?id=135256&action=edit
dmesg upstream kernel 4.14-rc8, 2017-11-06

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/amd/display: remove some unneeded code

2017-11-06 Thread Christian König

Am 06.11.2017 um 12:44 schrieb Dan Carpenter:

We assign "v_init = asic_blank_start;" a few lines earlier so there is
no need to do it again inside the if statements.  Also "v_init" is
unsigned so it can't be less than zero.

Signed-off-by: Dan Carpenter 


Acked-by: Christian König 



diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
index 1994865d4351..c7333cdf1802 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
@@ -236,13 +236,10 @@ static void tgn10_program_timing(
if (tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT ||
tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
tg->dlg_otg_param.signal == SIGNAL_TYPE_EDP) {
-   v_init = asic_blank_start;
start_point = 1;
if (patched_crtc_timing.flags.INTERLACE == 1)
field_num = 1;
}
-   if (v_init < 0)
-   v_init = 0;
v_fp2 = 0;
if (tg->dlg_otg_param.vstartup_start > asic_blank_end)
v_fp2 = tg->dlg_otg_param.vstartup_start > asic_blank_end;
___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/amd/display: checking for NULL instead of IS_ERR()

2017-11-06 Thread Christian König

Am 06.11.2017 um 12:43 schrieb Dan Carpenter:

backlight_device_register() never returns NULL, it returns error
pointers on error so the check here is wrong.

Signed-off-by: Dan Carpenter 


Acked-by: Christian König 



diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 33a15a1d818c..75f9383f5b9b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1296,7 +1296,7 @@ amdgpu_dm_register_backlight_device(struct 
amdgpu_display_manager *dm)
&amdgpu_dm_backlight_ops,
&props);
  
-	if (NULL == dm->backlight_dev)

+   if (IS_ERR(dm->backlight_dev))
DRM_ERROR("DM: Backlight registration failed!\n");
else
DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n", 
bl_name);



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 3/6] drm/arm/mali: Use drm_mode_config_helper_suspend/resume()

2017-11-06 Thread Liviu Dudau
On Thu, Nov 02, 2017 at 09:09:06PM +0100, Noralf Trønnes wrote:
> These helpers take care of output polling, fbdev and atomic state.

Hmm, not much useful info here, tbh. Maybe something like:

"Replace driver's code with the generic helpers that do the same thing" ?

> 
> Cc: Liviu Dudau 
> Cc: Brian Starkey 
> Signed-off-by: Noralf Trønnes 

Otherwise, looks good to me. Thanks!

Reviewed-by: Liviu Dudau 

> ---
>  drivers/gpu/drm/arm/malidp_drv.c | 24 +++-
>  drivers/gpu/drm/arm/malidp_drv.h |  1 -
>  2 files changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c 
> b/drivers/gpu/drm/arm/malidp_drv.c
> index b8944666a18f..75f0bce33941 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "malidp_drv.h"
> @@ -749,34 +750,15 @@ static int malidp_platform_remove(struct 
> platform_device *pdev)
>  static int __maybe_unused malidp_pm_suspend(struct device *dev)
>  {
>   struct drm_device *drm = dev_get_drvdata(dev);
> - struct malidp_drm *malidp = drm->dev_private;
>  
> - drm_kms_helper_poll_disable(drm);
> - console_lock();
> - drm_fbdev_cma_set_suspend(malidp->fbdev, 1);
> - console_unlock();
> - malidp->pm_state = drm_atomic_helper_suspend(drm);
> - if (IS_ERR(malidp->pm_state)) {
> - console_lock();
> - drm_fbdev_cma_set_suspend(malidp->fbdev, 0);
> - console_unlock();
> - drm_kms_helper_poll_enable(drm);
> - return PTR_ERR(malidp->pm_state);
> - }
> -
> - return 0;
> + return drm_mode_config_helper_suspend(drm);
>  }
>  
>  static int __maybe_unused malidp_pm_resume(struct device *dev)
>  {
>   struct drm_device *drm = dev_get_drvdata(dev);
> - struct malidp_drm *malidp = drm->dev_private;
>  
> - drm_atomic_helper_resume(drm, malidp->pm_state);
> - console_lock();
> - drm_fbdev_cma_set_suspend(malidp->fbdev, 0);
> - console_unlock();
> - drm_kms_helper_poll_enable(drm);
> + drm_mode_config_helper_resume(drm);
>  
>   return 0;
>  }
> diff --git a/drivers/gpu/drm/arm/malidp_drv.h 
> b/drivers/gpu/drm/arm/malidp_drv.h
> index 2e2033140efc..70ed6aeccf05 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.h
> +++ b/drivers/gpu/drm/arm/malidp_drv.h
> @@ -24,7 +24,6 @@ struct malidp_drm {
>   struct drm_crtc crtc;
>   wait_queue_head_t wq;
>   atomic_t config_valid;
> - struct drm_atomic_state *pm_state;
>   u32 core_id;
>  };
>  
> -- 
> 2.14.2
> 

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 01/22] drm/gem-fb-helper: drm_gem_fbdev_fb_create() make funcs optional

2017-11-06 Thread Noralf Trønnes


Den 06.11.2017 09.54, skrev Daniel Vetter:

On Sat, Nov 04, 2017 at 02:03:55PM +0100, Noralf Trønnes wrote:

Make the drm_framebuffer_funcs argument optional for drivers that
don't need to set the dirty callback.

Signed-off-by: Noralf Trønnes 

Looks like this patch ended up in the wrong patch series? Atm there's not
much users of this (I guess they're still waiting for acks), and somehow
I'd expect that there's some drivers which really want this (but I don't
see any in this patch series). Looks good, but I'm a bit confused.


The purpose of this change is so the driver doesn't have to provide a
fb vtable if it doesn't need to set the dirty calback.

So if I didn't rework the cma helper to get rid of struct drm_fbdev_cma,
I would have followed up with this change:

-static const struct drm_framebuffer_funcs drm_fb_cma_funcs = {
-    .destroy    = drm_gem_fb_destroy,
-    .create_handle    = drm_gem_fb_create_handle,
-};
-
 /**
  * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct
  * @dev: DRM device
  * @preferred_bpp: Preferred bits per pixel for the device
  * @max_conn_count: Maximum number of connectors
  *
  * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR.
  */
 struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
 unsigned int preferred_bpp, unsigned int max_conn_count)
 {
 return drm_fbdev_cma_init_with_funcs(dev, preferred_bpp,
                  max_conn_count,
 -                     &drm_fb_cma_funcs);
 +                     NULL);
 }
 EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);

Noralf.


-Daniel


---
  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index aa8cb9bfa499..4d682a6e8bcb 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -272,7 +272,8 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
   * @sizes: fbdev size description
   * @pitch_align: Optional pitch alignment
   * @obj: GEM object backing the framebuffer
- * @funcs: vtable to be used for the new framebuffer object
+ * @funcs: Optional vtable to be used for the new framebuffer object when the
+ * dirty callback is needed.
   *
   * This function creates a framebuffer from a &drm_fb_helper_surface_size
   * description for use in the &drm_fb_helper_funcs.fb_probe callback.
@@ -300,6 +301,9 @@ drm_gem_fbdev_fb_create(struct drm_device *dev,
if (obj->size < mode_cmd.pitches[0] * mode_cmd.height)
return ERR_PTR(-EINVAL);
  
+	if (!funcs)

+   funcs = &drm_gem_fb_funcs;
+
return drm_gem_fb_alloc(dev, &mode_cmd, &obj, 1, funcs);
  }
  EXPORT_SYMBOL(drm_gem_fbdev_fb_create);
--
2.14.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 02/22] drm/cma-helper: Add drm_fb_cma_fbdev_init/fini()

2017-11-06 Thread Noralf Trønnes


Den 06.11.2017 10.04, skrev Daniel Vetter:

On Sat, Nov 04, 2017 at 02:03:56PM +0100, Noralf Trønnes wrote:

Add functions drm_fb_cma_fbdev_init(), drm_fb_cma_fbdev_fini() and
drm_fb_cma_fbdev_init_with_funcs(). These functions relies on the fact
that the drm_fb_helper struct is stored in dev->drm_fb_helper_private
so drivers don't need to store it.

Cc: Laurent Pinchart 
Signed-off-by: Noralf Trønnes 

I guess you've proven me wrong, since at the end of this patch series we
do have some neat functions that do the same thing an earlier patch series
did, expect there's a _cma_ in the name. But the function itself is 100%
generic.

Sry for all the meandering, I guess this once again shows that any problem
in software can indeed be solved by adding another abstraction layer on
top. You'll probably be slightly mad, but I'd vote for the patch to drop
the _cma_ infix again (once everything has landed, no need to respin all
the patches again).



The purpose here is not to add a layer, but to change an existing one.
I want to get rid of struct drm_fbdev_cma and I have 2 options:
- One patch that touches all drivers
- Add new functions, switch over drivers one patch at a time

In effect this is what I do in this patchset:

include/drm/drm_fb_cma_helper.h:

+int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
+    unsigned int preferred_bpp, unsigned int max_conn_count,
+    const struct drm_framebuffer_funcs *funcs);
+int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int 
preferred_bpp,

+              unsigned int max_conn_count);
+void drm_fb_cma_fbdev_fini(struct drm_device *dev);
+

-struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
-    unsigned int preferred_bpp, unsigned int max_conn_count,
-    const struct drm_framebuffer_funcs *funcs);
-struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
-    unsigned int preferred_bpp, unsigned int max_conn_count);
-void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma);
-

How about this commit message:

drm/cma-helper: Add drm_fb_cma_fbdev_init/fini()

There is no need for drivers to carry a pointer to the the fbdev
emulation structure now that drm_device has one. Currently drivers has
a pointer to struct drm_fbdev_cma in their driver structure. This
pointer is returned by drm_fbdev_cma_init().

In order to get rid of struct drm_fbdev_cma, add functions
drm_fb_cma_fbdev_init(), drm_fb_cma_fbdev_fini() and
drm_fb_cma_fbdev_init_with_funcs(). These functions relies on the fact
that the drm_fb_helper struct is stored in drm_device->fb_helper so
drivers don't need to store it. When drivers have been moved over to
these functions, the old init/fini functions can be removed.

Cc: Laurent Pinchart 
Signed-off-by: Noralf Trønnes 


Noralf.


---
  drivers/gpu/drm/drm_fb_cma_helper.c | 116 +++-
  include/drm/drm_fb_cma_helper.h |   7 +++
  2 files changed, 121 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
b/drivers/gpu/drm/drm_fb_cma_helper.c
index 0e3c14174d08..267c04216281 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -23,6 +23,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  
  #define DEFAULT_FBDEFIO_DELAY_MS 50

@@ -42,7 +43,7 @@ struct drm_fbdev_cma {
   * callback function to create a cma backed framebuffer.
   *
   * An fbdev framebuffer backed by cma is also available by calling
- * drm_fbdev_cma_init(). drm_fbdev_cma_fini() tears it down.
+ * drm_fb_cma_fbdev_init(). drm_fb_cma_fbdev_fini() tears it down.
   * If the &drm_framebuffer_funcs.dirty callback is set, fb_deferred_io will be
   * set up automatically. &drm_framebuffer_funcs.dirty is called by
   * drm_fb_helper_deferred_io() in process context (&struct delayed_work).
@@ -68,7 +69,7 @@ struct drm_fbdev_cma {
   *
   * Initialize::
   *
- * fbdev = drm_fbdev_cma_init_with_funcs(dev, 16,
+ * fbdev = drm_fb_cma_fbdev_init_with_funcs(dev, 16,
   *   dev->mode_config.num_crtc,
   *   dev->mode_config.num_connector,
   *   &driver_fb_funcs);
@@ -314,6 +315,117 @@ static const struct drm_fb_helper_funcs 
drm_fb_cma_helper_funcs = {
.fb_probe = drm_fbdev_cma_create,
  };
  
+/**

+ * drm_fb_cma_fbdev_init_with_funcs() - Allocate and initialize fbdev emulation
+ * @dev: DRM device
+ * @preferred_bpp: Preferred bits per pixel for the device.
+ * @dev->mode_config.preferred_depth is used if this is zero.
+ * @max_conn_count: Maximum number of connectors.
+ *  @dev->mode_config.num_connector is used if this is zero.
+ * @funcs: Framebuffer functions, in particular a custom dirty() callback.

With the previous patch this is allowed to be NULL, right? Please
document.

And since that also explains the patch 1, on both patches:

Reviewed-by: Daniel Vetter 

But ple

Re: [PATCH 03/22] drm/arc: Use drm_fb_cma_fbdev_init/fini()

2017-11-06 Thread Noralf Trønnes


Den 06.11.2017 10.08, skrev Daniel Vetter:

On Sat, Nov 04, 2017 at 02:03:57PM +0100, Noralf Trønnes wrote:

Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
the fact that drm_device holds a pointer to the drm_fb_helper structure.
This means that the driver doesn't have to keep track of that.
Also use the drm_fb_helper functions directly.
Remove unused function prototype arcpgu_fbdev_cma_init().

Cc: Alexey Brodkin 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/arc/arcpgu.h |  4 
  drivers/gpu/drm/arc/arcpgu_drv.c | 36 +++-
  2 files changed, 7 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index e8fcf3ab1d9a..90ef76b19f8a 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -20,7 +20,6 @@
  struct arcpgu_drm_private {
void __iomem*regs;
struct clk  *clk;
-   struct drm_fbdev_cma*fbdev;
struct drm_framebuffer  *fb;
struct drm_crtc crtc;
struct drm_plane*plane;
@@ -43,8 +42,5 @@ static inline u32 arc_pgu_read(struct arcpgu_drm_private 
*arcpgu,
  int arc_pgu_setup_crtc(struct drm_device *dev);
  int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np);
  int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np);
-struct drm_fbdev_cma *arcpgu_fbdev_cma_init(struct drm_device *dev,
-   unsigned int preferred_bpp, unsigned int num_crtc,
-   unsigned int max_conn_count);
  
  #endif

diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 074fd4ea7ece..f54481ee834c 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -16,6 +16,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -25,16 +26,9 @@
  #include "arcpgu.h"
  #include "arcpgu_regs.h"
  
-static void arcpgu_fb_output_poll_changed(struct drm_device *dev)

-{
-   struct arcpgu_drm_private *arcpgu = dev->dev_private;
-
-   drm_fbdev_cma_hotplug_event(arcpgu->fbdev);
-}
-
  static const struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
.fb_create  = drm_gem_fb_create,
-   .output_poll_changed = arcpgu_fb_output_poll_changed,
+   .output_poll_changed = drm_fb_helper_output_poll_changed,
.atomic_check = drm_atomic_helper_check,
.atomic_commit = drm_atomic_helper_commit,
  };
@@ -51,13 +45,6 @@ static void arcpgu_setup_mode_config(struct drm_device *drm)
  
  DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
  
-static void arcpgu_lastclose(struct drm_device *drm)

-{
-   struct arcpgu_drm_private *arcpgu = drm->dev_private;
-
-   drm_fbdev_cma_restore_mode(arcpgu->fbdev);
-}
-
  static int arcpgu_load(struct drm_device *drm)
  {
struct platform_device *pdev = to_platform_device(drm->dev);
@@ -113,13 +100,9 @@ static int arcpgu_load(struct drm_device *drm)
drm_mode_config_reset(drm);
drm_kms_helper_poll_init(drm);
  
-	arcpgu->fbdev = drm_fbdev_cma_init(drm, 16,

-  drm->mode_config.num_connector);
-   if (IS_ERR(arcpgu->fbdev)) {
-   ret = PTR_ERR(arcpgu->fbdev);
-   arcpgu->fbdev = NULL;
-   return -ENODEV;
-   }
+   ret = drm_fb_cma_fbdev_init(drm, 16, 0);

s/0/NULL/

Probably applies to other patches too.

Also, why did you merge this new change into the old patch with the
output_poll/lastclose change? It took me a while to figure out what you're
doing, because the patches looked familiar, but the cover letter didn't
mention how stuff evolved, nor the patches here.

Also, you've lost the bunch of acks/r-bs you've gathered already. If
there's no technical reason to merge the 2 patches I'm kinda leaning
towards merging them separately, at least the ones that have acks already.
Just to avoid the pitfall of the continually evolving patch series that
never lands.


Ah, now I understand some of the confusion.

I have one patchset that converts the drivers that don't use the cma helper:
drm/fb-helper: Add .last_close and .output_poll_changed helpers

And in this patchset I remove the use of struct drm_fbdev_cma which means
I move away from the all the drm_fbdev_cma* functions. This means switching
to the .last_close and .output_poll_changed helpers from the previous
patchset.

Not sure how I can un-confuse this.

Noralf.


-Daniel


+   if (ret)
+   return ret;
  
  	platform_set_drvdata(pdev, drm);

return 0;
@@ -127,12 +110,7 @@ static int arcpgu_load(struct drm_device *drm)
  
  static int arcpgu_unload(struct drm_device *drm)

  {
-   struct arcpgu_drm_private *arcpgu = drm->dev_private;
-
-   if (arcpgu->fbdev) {
-   drm_fbdev_cma_fini(arcpgu->fbdev);
-   arcpgu->fbdev = NULL;
-   }
+   drm_fb_cma_fbdev_fini(drm);
drm_kms_helper_poll_fini(drm);
drm_mod

[PATCH] video: fbdev: intelfb: remove redundant variables

2017-11-06 Thread Colin King
From: Colin Ian King 

Variables err_max, err_target and f_best are being assigned values but
these are never read, hence they are redundant variables and can be
removed. Cleans up clang warnings:

drivers/video/fbdev/intelfb/intelfbhw.c:946:2: warning: Value stored to
'err_max' is never read
drivers/video/fbdev/intelfb/intelfbhw.c:947:2: warning: Value stored to
'err_target' is never read
drivers/video/fbdev/intelfb/intelfbhw.c:995:6: warning: Value stored to
'f_best' is never read

Signed-off-by: Colin Ian King 
---
 drivers/video/fbdev/intelfb/intelfbhw.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/intelfb/intelfbhw.c 
b/drivers/video/fbdev/intelfb/intelfbhw.c
index d31ed4e2c46f..83fec573cceb 100644
--- a/drivers/video/fbdev/intelfb/intelfbhw.c
+++ b/drivers/video/fbdev/intelfb/intelfbhw.c
@@ -937,15 +937,11 @@ static int calc_pll_params(int index, int clock, u32 
*retm1, u32 *retm2,
 {
u32 m1, m2, n, p1, p2, n1, testm;
u32 f_vco, p, p_best = 0, m, f_out = 0;
-   u32 err_max, err_target, err_best = 1000;
-   u32 n_best = 0, m_best = 0, f_best, f_err;
+   u32 err_best = 1000;
+   u32 n_best = 0, m_best = 0, f_err;
u32 p_min, p_max, p_inc, div_max;
struct pll_min_max *pll = &plls[index];
 
-   /* Accept 0.5% difference, but aim for 0.1% */
-   err_max = 5 * clock / 1000;
-   err_target = clock / 1000;
-
DBG_MSG("Clock is %d\n", clock);
 
div_max = pll->max_vco / clock;
@@ -992,7 +988,6 @@ static int calc_pll_params(int index, int clock, u32 
*retm1, u32 *retm2,
m_best = testm;
n_best = n;
p_best = p;
-   f_best = f_out;
err_best = f_err;
}
}
-- 
2.14.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 10/22] drm/pl111: Use drm_fb_cma_fbdev_init/fini()

2017-11-06 Thread Linus Walleij
On Sat, Nov 4, 2017 at 2:04 PM, Noralf Trønnes  wrote:

> Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
> the fact that drm_device holds a pointer to the drm_fb_helper structure.
> This means that the driver doesn't have to keep track of that.
> Also use the drm_fb_helper functions directly.
>
> Cc: Eric Anholt 
> Signed-off-by: Noralf Trønnes 

Looks reasonable.

Reviewed-by: Linus Walleij 

Yours,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 16/22] drm/tve200: Use drm_fb_cma_fbdev_init/fini()

2017-11-06 Thread Linus Walleij
On Sat, Nov 4, 2017 at 2:04 PM, Noralf Trønnes  wrote:

> Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
> the fact that drm_device holds a pointer to the drm_fb_helper structure.
> This means that the driver doesn't have to keep track of that.
> Also use the drm_fb_helper functions directly.
>
> Cc: Linus Walleij 
> Signed-off-by: Noralf Trønnes 

Reviewed-by: Linus Walleij 

Yours,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH hwc] Android: add CleanSpec.mk

2017-11-06 Thread Rob Herring
On Mon, Nov 6, 2017 at 5:16 AM, Robert Foss  wrote:
> Hey Rob,
>
> On Fri, 2017-11-03 at 07:35 -0500, Rob Herring wrote:
>> On Thu, Nov 2, 2017 at 11:45 PM, Chih-Wei Huang > org> wrote:
>> > The file contains rules that are executed on incremental builds.
>> > Since commit 4f7dc9b6 the library was moved to /vendor so
>> > the old file must be removed.
>> >
>> > Signed-off-by: Chih-Wei Huang 
>> > ---
>> >  CleanSpec.mk | 1 +
>> >  1 file changed, 1 insertion(+)
>> >  create mode 100644 CleanSpec.mk
>> >
>> > diff --git a/CleanSpec.mk b/CleanSpec.mk
>> > new file mode 100644
>> > index 000..99dcecd
>> > --- /dev/null
>> > +++ b/CleanSpec.mk
>> > @@ -0,0 +1 @@
>> > +$(call add-clean-step, rm -rf
>> > $(TARGET_OUT)/lib*/hw/hwcomposer.drm.so)
>>
>> Seems a bit silly to add forever an explicit file to clean for a
>> transient problem.
>>
> While transient, if maintaining multuple Android versions it may still
> stick around for quite some time.

That's not the issue. Once you do a clean build, the problem is gone
regardless of version.

Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/rockchip: add CONFIG_OF dependency for lvds

2017-11-06 Thread Arnd Bergmann
Build-testing on randconfig kernels revealed a dependency in the
newly added lvds sub-driver:

drivers/gpu/drm/rockchip/rockchip_lvds.c: In function 'rockchip_lvds_bind':
drivers/gpu/drm/rockchip/rockchip_lvds.c:380:24: error: 'struct drm_bridge' has 
no member named 'of_node'
   remote = lvds->bridge->of_node;

We could work around that in the code, adding a Kconfig dependency
seems easier.

Fixes: 34cc0aa25456 ("drm/rockchip: Add support for Rockchip Soc LVDS")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/rockchip/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 3c70c6224bd2..0ccc76217ee4 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -60,7 +60,7 @@ config ROCKCHIP_INNO_HDMI
 config ROCKCHIP_LVDS
bool "Rockchip LVDS support"
depends on DRM_ROCKCHIP
-   depends on PINCTRL
+   depends on PINCTRL && OF
help
  Choose this option to enable support for Rockchip LVDS controllers.
  Rockchip rk3288 SoC has LVDS TX Controller can be used, and it
-- 
2.9.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Intel-gfx] [PATCH] drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, v2.

2017-11-06 Thread Ville Syrjälä
On Thu, Nov 02, 2017 at 09:55:40AM +0100, Maarten Lankhorst wrote:
> Op 01-11-17 om 18:00 schreef Ville Syrjälä:
> > On Wed, Nov 01, 2017 at 04:55:06PM +0100, Maarten Lankhorst wrote:
> >> Op 01-11-17 om 16:29 schreef Ville Syrjälä:
> >>> On Wed, Nov 01, 2017 at 04:04:33PM +0100, Maarten Lankhorst wrote:
>  This introduces a slight behavioral change to rmfb. Instead of
>  disabling a crtc when the primary plane is disabled, we try to
>  preserve it.
> 
>  Apart from old versions of the vmwgfx xorg driver, there is
>  nothing depending on rmfb disabling a crtc.
> 
>  Vmwgfx' and simple kms helper atomic implementation rejects CRTC
>  enabled without plane, so we can do this safely.
> > The code for those seems a bit inconsistent. The crtc check requires
> > that the crtc state and plane state match. But the plane check allows
> > the plane to be enabled w/o the crtc being enabled. I guess it doesn't
> > matter really since you can't enable the plane without a crtc, and the
> > crtc check would then catch the case where the crtc would be disabled.
> 
> Exactly. :-) Only when a plane is unbound and stays unbound, the crtc check 
> won't
> be invoked. Hence it's the most accurate way of making sure that
> crtc enabled <=> primary plane bound.
> 
> If the check was done in the primary plane, an atomic modeset could enable
> the crtc without enabling the primary plane, which shouldn't be allowed but
> a plane check won't catch it.

> 
> This has been a bug in simple-kms-helper, fixed in the below commit:
> 
> commit 765831dc27ab141b3a0be1ab55b922b012427902
> Author: Maarten Lankhorst 
> Date:   Wed Jul 12 10:13:29 2017 +0200
> 
> drm/simple-kms-helper: Fix the check for the mismatch between plane and 
> CRTC enabled.

Hmm. OK that part looks OK. What does seem a bit inconsistent is the
fact that we pass can_update_disabled=true, but later on we reject the
update when visible==false. The old code would have accepted that
because it didn't even call drm_plane_helper_check_state() when the
crtc (and thus also the plane) was disabled.

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Intel-gfx] [PATCH] drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, v2.

2017-11-06 Thread Ville Syrjälä
On Mon, Nov 06, 2017 at 04:01:20PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 02, 2017 at 09:55:40AM +0100, Maarten Lankhorst wrote:
> > Op 01-11-17 om 18:00 schreef Ville Syrjälä:
> > > On Wed, Nov 01, 2017 at 04:55:06PM +0100, Maarten Lankhorst wrote:
> > >> Op 01-11-17 om 16:29 schreef Ville Syrjälä:
> > >>> On Wed, Nov 01, 2017 at 04:04:33PM +0100, Maarten Lankhorst wrote:
> >  This introduces a slight behavioral change to rmfb. Instead of
> >  disabling a crtc when the primary plane is disabled, we try to
> >  preserve it.
> > 
> >  Apart from old versions of the vmwgfx xorg driver, there is
> >  nothing depending on rmfb disabling a crtc.
> > 
> >  Vmwgfx' and simple kms helper atomic implementation rejects CRTC
> >  enabled without plane, so we can do this safely.
> > > The code for those seems a bit inconsistent. The crtc check requires
> > > that the crtc state and plane state match. But the plane check allows
> > > the plane to be enabled w/o the crtc being enabled. I guess it doesn't
> > > matter really since you can't enable the plane without a crtc, and the
> > > crtc check would then catch the case where the crtc would be disabled.
> > 
> > Exactly. :-) Only when a plane is unbound and stays unbound, the crtc check 
> > won't
> > be invoked. Hence it's the most accurate way of making sure that
> > crtc enabled <=> primary plane bound.
> > 
> > If the check was done in the primary plane, an atomic modeset could enable
> > the crtc without enabling the primary plane, which shouldn't be allowed but
> > a plane check won't catch it.
> 
> > 
> > This has been a bug in simple-kms-helper, fixed in the below commit:
> > 
> > commit 765831dc27ab141b3a0be1ab55b922b012427902
> > Author: Maarten Lankhorst 
> > Date:   Wed Jul 12 10:13:29 2017 +0200
> > 
> > drm/simple-kms-helper: Fix the check for the mismatch between plane and 
> > CRTC enabled.
> 
> Hmm. OK that part looks OK. What does seem a bit inconsistent is the
> fact that we pass can_update_disabled=true, but later on we reject the
> update when visible==false. The old code would have accepted that
> because it didn't even call drm_plane_helper_check_state() when the
> crtc (and thus also the plane) was disabled.

Actually how does this work at all? If you turn off both the crtc and
plane, then the plane check will always return -EINVAL on account of
visible==false?

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3] display: panel: Add Mitsubishi aa070mc01 display support (800x480)

2017-11-06 Thread Rob Herring
On Fri, Oct 20, 2017 at 5:18 PM, Lukasz Majewski  wrote:
> This commit adds support for Mitsubishi aa070mc01 TFT panel working
> with 8 bit ISP mode (pin 19 "mode" HIGH for 20 pin TFT connector).
>
> Signed-off-by: Lukasz Majewski 
> ---
> Changes for v2:
> - Place the code sorted alphabetically
> - Add missing ./Documentation/devicetree/binding/display properties
>   description
>
> Changes for v3:
> - Fix typo in display name (in property description)
>
> ---
>  .../display/panel/mitsubishi,aa070mc01.txt |  7 +
>  drivers/gpu/drm/panel/panel-simple.c   | 35 
> ++
>  2 files changed, 42 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/mitsubishi,aa070mc01.txt

Acked-by: Rob Herring 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] display: panel: Add Tianma tm070rvhg71 display support (800x480)

2017-11-06 Thread Rob Herring
On Fri, Oct 27, 2017 at 3:18 AM, Lukasz Majewski  wrote:
> Hi Rob,
>
>> On Sat, Oct 21, 2017 at 12:10:03AM +0200, Lukasz Majewski wrote:
>> > Signed-off-by: Lukasz Majewski 
>> > ---
>> >  .../bindings/display/panel/tianma,tm070rvhg71.txt  |  7 ++
>> >  drivers/gpu/drm/panel/panel-simple.c   | 27
>> > ++ 2 files changed, 34 insertions(+)
>> >  create mode 100644
>> > Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
>> >
>> > diff --git
>> > a/Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
>> > b/Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
>> > new file mode 100644 index 000..b84217f --- /dev/null
>> > +++
>> > b/Documentation/devicetree/bindings/display/panel/tianma,tm070rvhg71.txt
>> > @@ -0,0 +1,7 @@ +Tianma Micro-electronics TM070RVHG71 7.0" WXGA TFT
>> > LCD panel +
>> > +Required properties:
>> > +- compatible: should be "tianma,tm070rvhg71
>> > +
>> > +This binding is compatible with the simple-panel binding, which is
>> > specified +in simple-panel.txt in this directory.
>>
>> No supplies? Still need to list power-supply here if so, so it is
>> clear that this display has a single supply (or you need to list
>> multiple ones if not).
>
> I took the same approach as several other simple panels having
> description in Documentation/devicetree/bindings/display/panel/*
> directory (e.g. innolux,g121i1-l01.txt) .
>
> In the description it is stated that this binding is compatible with
> one documented in the simple-panel.txt, which defines following
> properties:
>
> Required properties:
> - power-supply: regulator to provide the supply voltage
>
> Optional properties:
> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> - enable-gpios: GPIO pin to enable or disable the panel
> - backlight: phandle of the backlight device attached to the panel
>
>
> Do I need to do something more?

Yes. How do I know this panel has a single power supply vs. it has
multiple supplies and you haven't thought about supplies yet or they
aren't s/w controlled on some boards. The latter is frequently the
case when I go read the datasheets.

And yes there are examples that don't do this, but we're stricter now.

Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

2017-11-06 Thread Benjamin Gaignard
2017-11-02 12:10 GMT+01:00 Mark Brown :
> On Thu, Nov 02, 2017 at 11:44:07AM +0100, Greg KH wrote:
>> On Tue, Oct 31, 2017 at 07:11:53PM +, Mark Brown wrote:
>
>> > There was a discussion a while ago in the context of I2C/SPI MFDs
>> > which concluded that if you need a bus and it's going to be effectively
>> > noop then you should just use the platform bus as anything else will
>> > consist almost entirely of cut'n'paste from the platform bus with some
>> > light sed usage and code duplication is bad.  It's not super lovely as
>> > it's not actually a memory mapped device but it's the best idea we've
>> > got.
>
>> Ugh, I hate that.  What's wrong with using a "virtual" device instead?
>
> It was the duplication, initially everyone was making buses.
>
>> I can create a "virtual" bus for things like this if they really want a
>> "simple" bus, abusing platform for this is the major reason I hate the
>> platform bus code...
>
> In the MFD case they're physical devices, they're just usually on the
> wrong side of an I2C or SPI link.  Plus MFD already handles platform
> devices for things that are memory mapped so it's a bit of a more
> natural fit there.

What I can do is to register an ion bus (like cec one for example),
add one ion parent device so heaps will appear in /sys/bus/ion/ion*
and /sys/devices/ion/ion*

Does that could sound good enough ?

Benjamin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

2017-11-06 Thread Greg KH
On Mon, Nov 06, 2017 at 03:42:04PM +0100, Benjamin Gaignard wrote:
> 2017-11-02 12:10 GMT+01:00 Mark Brown :
> > On Thu, Nov 02, 2017 at 11:44:07AM +0100, Greg KH wrote:
> >> On Tue, Oct 31, 2017 at 07:11:53PM +, Mark Brown wrote:
> >
> >> > There was a discussion a while ago in the context of I2C/SPI MFDs
> >> > which concluded that if you need a bus and it's going to be effectively
> >> > noop then you should just use the platform bus as anything else will
> >> > consist almost entirely of cut'n'paste from the platform bus with some
> >> > light sed usage and code duplication is bad.  It's not super lovely as
> >> > it's not actually a memory mapped device but it's the best idea we've
> >> > got.
> >
> >> Ugh, I hate that.  What's wrong with using a "virtual" device instead?
> >
> > It was the duplication, initially everyone was making buses.
> >
> >> I can create a "virtual" bus for things like this if they really want a
> >> "simple" bus, abusing platform for this is the major reason I hate the
> >> platform bus code...
> >
> > In the MFD case they're physical devices, they're just usually on the
> > wrong side of an I2C or SPI link.  Plus MFD already handles platform
> > devices for things that are memory mapped so it's a bit of a more
> > natural fit there.
> 
> What I can do is to register an ion bus (like cec one for example),
> add one ion parent device so heaps will appear in /sys/bus/ion/ion*
> and /sys/devices/ion/ion*
> 
> Does that could sound good enough ?

I would like to see that...

thanks,

greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 0/7] drm/edid and drivers: ELD refactoring

2017-11-06 Thread Thierry Reding
On Wed, Nov 01, 2017 at 04:20:56PM +0200, Jani Nikula wrote:
> We were recently bitten by drm_edid_to_eld() clearing the connector
> type, and us failing to set it back for DP. Here's a few ELD related
> patches to try to unify ELD handling and make it a bit simpler for
> drivers to get it right.
> 
> Apologies for the massive Cc list; it's the maintainers of all drivers
> that call drm_edid_to_eld().
> 
> I'm open to splitting up patch 6 to driver specific patches as needed,
> but I'd think it would be just fine to merge via drm-misc as-is too.
> 
> BR,
> Jani.
> 
> Cc: Alex Deucher 
> Cc: Christian König 
> Cc: Archit Taneja 
> Cc: Andrzej Hajda 
> Cc: Russell King 
> Cc: CK Hu 
> Cc: Philipp Zabel 
> Cc: Ben Skeggs 
> Cc: Mark Yao 
> Cc: Benjamin Gaignard 
> Cc: Vincent Abriou 
> Cc: Thierry Reding 
> Cc: Eric Anholt 
> 
> 
> Jani Nikula (7):
>   drm/edid: use macros for ELD offsets and values
>   drm/edid: set ELD connector type in drm_edid_to_eld()
>   drm/i915: remove redundant ELD connector type update
>   drm/edid: abstract connector ELD clearing
>   drm/edid: build ELD in drm_add_edid_modes()
>   drm/drivers: drop redundant drm_edid_to_eld() calls
>   drm/edid: make drm_edid_to_eld() static
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c |  1 -
>  drivers/gpu/drm/bridge/analogix-anx78xx.c  |  2 -
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c  |  2 -
>  drivers/gpu/drm/drm_edid.c | 70 
> +++---
>  drivers/gpu/drm/i2c/tda998x_drv.c  |  1 -
>  drivers/gpu/drm/i915/intel_dp.c|  1 -
>  drivers/gpu/drm/i915/intel_modes.c | 18 ---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c|  1 -
>  drivers/gpu/drm/nouveau/nv50_display.c |  5 +-
>  drivers/gpu/drm/radeon/radeon_connectors.c |  1 -
>  drivers/gpu/drm/radeon/radeon_dp_mst.c |  1 -
>  drivers/gpu/drm/rockchip/cdn-dp-core.c |  4 +-
>  drivers/gpu/drm/sti/sti_hdmi.c |  1 -
>  drivers/gpu/drm/tegra/output.c |  1 -
>  drivers/gpu/drm/vc4/vc4_hdmi.c |  1 -
>  include/drm/drm_edid.h |  1 -
>  include/drm/drm_modeset_helper_vtables.h   |  3 --
>  17 files changed, 44 insertions(+), 70 deletions(-)

The series:

Acked-by: Thierry Reding 


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


i915: pipe B vblank wait timed out

2017-11-06 Thread Borislav Petkov
Hi,

I see this on an 32-bit acer atom mini-laptop with -rc8+tip:

[2.399416] pipe B vblank wait timed out
[2.399506] [ cut here ]
[2.399533] WARNING: CPU: 1 PID: 22 at 
/mnt/kernel/kernel/linux-2.6/drivers/gpu/drm/i915/intel_display.c:12176 
intel_atomic_commit_tail+0xe0d/0xe20
[2.399545] Modules linked in:
[2.399574] CPU: 1 PID: 22 Comm: kworker/1:1 Not tainted 4.14.0-rc8+ #2
[2.399583] Hardware name: Acer AOA150/, BIOS v0.3309 10/06/2008
[2.399597] Workqueue: events output_poll_execute
[2.399613] task: f65a3780 task.stack: f65aa000
[2.399625] EIP: intel_atomic_commit_tail+0xe0d/0xe20
[2.399634] EFLAGS: 00210286 CPU: 1
[2.399643] EAX: 001c EBX:  ECX: 00200046 EDX: c18f6926
[2.399652] ESI: 01a8 EDI: 0001 EBP: f65abdc0 ESP: f65abd40
[2.399662]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[2.399671] CR0: 80050033 CR2:  CR3: 01c81000 CR4: 06f0
[2.399680] Call Trace:
[2.399699]  ? trace_hardirqs_on_caller+0xdc/0x1c0
[2.399713]  ? wait_woken+0x80/0x80
[2.399725]  ? wait_woken+0x80/0x80
[2.399739]  intel_atomic_commit+0x18f/0x250
[2.399751]  ? intel_atomic_commit+0x18f/0x250
[2.399766]  drm_atomic_commit+0x3a/0x50
[2.399782]  restore_fbdev_mode_atomic+0x15b/0x1a0
[2.399798]  ? intel_atomic_commit_tail+0xe20/0xe20
[2.399811]  restore_fbdev_mode+0x2c/0x140
[2.399824]  drm_fb_helper_restore_fbdev_mode_unlocked.part.19+0x23/0x70
[2.399836]  drm_fb_helper_set_par+0x45/0x80
[2.399848]  drm_fb_helper_hotplug_event.part.18+0x86/0xb0
[2.399861]  drm_fb_helper_hotplug_event+0x20/0x30
[2.399874]  intel_fbdev_output_poll_changed+0x17/0x20
[2.399885]  drm_kms_helper_hotplug_event+0x21/0x30
[2.399897]  output_poll_execute+0x8c/0x190
[2.399912]  process_one_work+0x1c5/0x5c0
[2.399927]  worker_thread+0x39/0x3c0
[2.399939]  ? preempt_count_sub+0x98/0xf0
[2.399952]  ? process_one_work+0x5c0/0x5c0
[2.399964]  kthread+0x10b/0x140
[2.399975]  ? process_one_work+0x5c0/0x5c0
[2.399987]  ? kthread_create_on_node+0x20/0x20
[2.38]  ? umh_complete+0x40/0x40
[2.400233]  ret_from_fork+0x19/0x24
[2.400246] Code: 4d 94 8d 55 c4 8b 81 30 02 00 00 01 f0 83 c0 04 e8 f9 a6 
c0 ff 85 db 0f 85 e2 fb ff ff 8d 47 41 50 68 08 28 95 c1 e8 90 de c2 ff <0f> ff 
58 5a e9 cb fb ff ff 8d 76 00 8d bc 27 00 00 00 00 3e 8d
[2.400722] ---[ end trace b8984f1a73a52375 ]---


-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/rockchip: add CONFIG_OF dependency for lvds

2017-11-06 Thread Sean Paul
On Mon, Nov 06, 2017 at 02:58:43PM +0100, Arnd Bergmann wrote:
> Build-testing on randconfig kernels revealed a dependency in the
> newly added lvds sub-driver:
> 
> drivers/gpu/drm/rockchip/rockchip_lvds.c: In function 'rockchip_lvds_bind':
> drivers/gpu/drm/rockchip/rockchip_lvds.c:380:24: error: 'struct drm_bridge' 
> has no member named 'of_node'
>remote = lvds->bridge->of_node;
> 
> We could work around that in the code, adding a Kconfig dependency
> seems easier.
> 
> Fixes: 34cc0aa25456 ("drm/rockchip: Add support for Rockchip Soc LVDS")
> Signed-off-by: Arnd Bergmann 

Thanks for the patch Arnd, I've pushed to drm-misc-next-fixes.

Sean

> ---
>  drivers/gpu/drm/rockchip/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig 
> b/drivers/gpu/drm/rockchip/Kconfig
> index 3c70c6224bd2..0ccc76217ee4 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -60,7 +60,7 @@ config ROCKCHIP_INNO_HDMI
>  config ROCKCHIP_LVDS
>   bool "Rockchip LVDS support"
>   depends on DRM_ROCKCHIP
> - depends on PINCTRL
> + depends on PINCTRL && OF
>   help
> Choose this option to enable support for Rockchip LVDS controllers.
> Rockchip rk3288 SoC has LVDS TX Controller can be used, and it
> -- 
> 2.9.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Intel-gfx] [PATCH] drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, v2.

2017-11-06 Thread Maarten Lankhorst
Op 06-11-17 om 15:06 schreef Ville Syrjälä:
> On Mon, Nov 06, 2017 at 04:01:20PM +0200, Ville Syrjälä wrote:
>> On Thu, Nov 02, 2017 at 09:55:40AM +0100, Maarten Lankhorst wrote:
>>> Op 01-11-17 om 18:00 schreef Ville Syrjälä:
 On Wed, Nov 01, 2017 at 04:55:06PM +0100, Maarten Lankhorst wrote:
> Op 01-11-17 om 16:29 schreef Ville Syrjälä:
>> On Wed, Nov 01, 2017 at 04:04:33PM +0100, Maarten Lankhorst wrote:
>>> This introduces a slight behavioral change to rmfb. Instead of
>>> disabling a crtc when the primary plane is disabled, we try to
>>> preserve it.
>>>
>>> Apart from old versions of the vmwgfx xorg driver, there is
>>> nothing depending on rmfb disabling a crtc.
>>>
>>> Vmwgfx' and simple kms helper atomic implementation rejects CRTC
>>> enabled without plane, so we can do this safely.
 The code for those seems a bit inconsistent. The crtc check requires
 that the crtc state and plane state match. But the plane check allows
 the plane to be enabled w/o the crtc being enabled. I guess it doesn't
 matter really since you can't enable the plane without a crtc, and the
 crtc check would then catch the case where the crtc would be disabled.
>>> Exactly. :-) Only when a plane is unbound and stays unbound, the crtc check 
>>> won't
>>> be invoked. Hence it's the most accurate way of making sure that
>>> crtc enabled <=> primary plane bound.
>>>
>>> If the check was done in the primary plane, an atomic modeset could enable
>>> the crtc without enabling the primary plane, which shouldn't be allowed but
>>> a plane check won't catch it.
>>> This has been a bug in simple-kms-helper, fixed in the below commit:
>>>
>>> commit 765831dc27ab141b3a0be1ab55b922b012427902
>>> Author: Maarten Lankhorst 
>>> Date:   Wed Jul 12 10:13:29 2017 +0200
>>>
>>> drm/simple-kms-helper: Fix the check for the mismatch between plane and 
>>> CRTC enabled.
>> Hmm. OK that part looks OK. What does seem a bit inconsistent is the
>> fact that we pass can_update_disabled=true, but later on we reject the
>> update when visible==false. The old code would have accepted that
>> because it didn't even call drm_plane_helper_check_state() when the
>> crtc (and thus also the plane) was disabled.
> Actually how does this work at all? If you turn off both the crtc and
> plane, then the plane check will always return -EINVAL on account of
> visible==false?
>
If the crtc is turned off, enabled = false and the primary plane is not in 
crtc_state->plane_mask. 




  

Visibility is ignored in this check, that part is handled in plane check that 
the framebuffer has to span the entire crtc if enabled.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/3] ASoC: amd: Report accurate hw_ptr during dma

2017-11-06 Thread Mark Brown
On Fri, Nov 03, 2017 at 04:35:43PM -0400, Alex Deucher wrote:

> Signed-off-by: Vijendar Mukunda 
> Signed-off-by: Akshu Agrawal 
> Reviewed-on: https://chromium-review.googlesource.com/659699
> Commit-Ready: Akshu Agrawal 
> Tested-by: Akshu Agrawal 
> Reviewed-by: Jason Clinton 
> Reviewed-on: https://chromium-review.googlesource.com/676627

These two URLs are different, what was being reviewed here?  What is
Commit-Ready supposed to mean?


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v6 1/2] staging: ion: reorder include

2017-11-06 Thread Benjamin Gaignard
Put include in alphabetic order

Signed-off-by: Benjamin Gaignard 
---
 drivers/staging/android/ion/ion.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index a7d9b0e..fda9756 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -15,28 +15,28 @@
  *
  */
 
+#include 
+#include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
+#include 
 #include 
 #include 
-#include 
-#include 
-#include 
-#include 
 
 #include "ion.h"
 
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v6 0/2] staging: ion: get one device per heap

2017-11-06 Thread Benjamin Gaignard
version 6:
- add an ION bus so heap are show as devices in /sys/bus/ion/
  instead of platform bus.
- split the patch in two: one for include reordering and one
  for per heap device change
- rebased on top of next-2017110 tag

version 5:
- create a configuration flag to keep legacy Ion misc device

version 4:
- add a configuration flag to switch between legacy Ion misc device
  and one device per heap version.
  This change has been suggested after Laura talks at XDC 2017.

version 3:
- change ion_device_add_heap prototype to return a possible error.

version 2:
- simplify ioctl check like propose by Dan
- make sure that we don't register more than ION_DEV_MAX heaps.

Until now all ion heaps are addressing using the same device "/dev/ion".
This way of working doesn't allow to give access rights (for example with
SElinux rules) per heap.
This series propose to have one device "/dev/ionX" per heap.
Query heaps informations will be possible on each device node but
allocation request will only be possible if heap_mask_id match with device 
minor number.

Using legacy Ion misc device is still by setting ION_LEGACY_DEVICE_API
configuration flag.


Benjamin Gaignard (2):
  staging: ion: reorder include
  staging: ion: create one device entry per heap

 drivers/staging/android/TODO|  1 -
 drivers/staging/android/ion/Kconfig |  7 +++
 drivers/staging/android/ion/ion-ioctl.c | 18 +++-
 drivers/staging/android/ion/ion.c   | 76 +++--
 drivers/staging/android/ion/ion.h   | 15 ++-
 5 files changed, 98 insertions(+), 19 deletions(-)

-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v6 2/2] staging: ion: create one device entry per heap

2017-11-06 Thread Benjamin Gaignard
Instead a getting only one common device "/dev/ion" for
all the heaps this patch allow to create one device
entry ("/dev/ionX") per heap.
Getting an entry per heap could allow to set security rules
per heap and global ones for all heaps.

Allocation requests will be only allowed if the mask_id
match with device minor.
Query request could be done on any of the devices.

Signed-off-by: Benjamin Gaignard 
---
 drivers/staging/android/TODO|  1 -
 drivers/staging/android/ion/Kconfig |  7 
 drivers/staging/android/ion/ion-ioctl.c | 18 --
 drivers/staging/android/ion/ion.c   | 62 +
 drivers/staging/android/ion/ion.h   | 15 ++--
 5 files changed, 91 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 687e0ea..8a11931 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -8,7 +8,6 @@ TODO:
 ion/
  - Add dt-bindings for remaining heaps (chunk and carveout heaps). This would
involve putting appropriate bindings in a memory node for Ion to find.
- - Split /dev/ion up into multiple nodes (e.g. /dev/ion/heap0)
  - Better test framework (integration with VGEM was suggested)
 
 Please send patches to Greg Kroah-Hartman  and Cc:
diff --git a/drivers/staging/android/ion/Kconfig 
b/drivers/staging/android/ion/Kconfig
index a517b2d..cb4666e 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -10,6 +10,13 @@ menuconfig ION
  If you're not using Android its probably safe to
  say N here.
 
+config ION_LEGACY_DEVICE_API
+   bool "Keep using Ion legacy misc device API"
+   depends on ION
+   help
+ Choose this option to keep using Ion legacy misc device API
+ i.e. /dev/ion
+
 config ION_SYSTEM_HEAP
bool "Ion system heap"
depends on ION
diff --git a/drivers/staging/android/ion/ion-ioctl.c 
b/drivers/staging/android/ion/ion-ioctl.c
index e26b786..bb5c77b 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -25,7 +25,8 @@ union ion_ioctl_arg {
struct ion_heap_query query;
 };
 
-static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
+static int validate_ioctl_arg(struct file *filp,
+ unsigned int cmd, union ion_ioctl_arg *arg)
 {
switch (cmd) {
case ION_IOC_HEAP_QUERY:
@@ -34,6 +35,19 @@ static int validate_ioctl_arg(unsigned int cmd, union 
ion_ioctl_arg *arg)
arg->query.reserved2 )
return -EINVAL;
break;
+
+   case ION_IOC_ALLOC:
+   {
+   int mask = 1 << iminor(filp->f_inode);
+
+#ifdef CONFIG_ION_LEGACY_DEVICE_API
+   if (imajor(filp->f_inode) == MISC_MAJOR)
+   return 0;
+#endif
+   if (!(arg->allocation.heap_id_mask & mask))
+   return -EINVAL;
+   break;
+   }
default:
break;
}
@@ -69,7 +83,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned 
long arg)
if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
return -EFAULT;
 
-   ret = validate_ioctl_arg(cmd, &data);
+   ret = validate_ioctl_arg(filp, cmd, &data);
if (WARN_ON_ONCE(ret))
return ret;
 
diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index fda9756..2c2568b 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -40,6 +40,9 @@
 
 #include "ion.h"
 
+#define ION_DEV_MAX 32
+#define ION_NAME "ion"
+
 static struct ion_device *internal_dev;
 static int heap_id;
 
@@ -535,15 +538,38 @@ static int debug_shrink_get(void *data, u64 *val)
 DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
debug_shrink_set, "%llu\n");
 
-void ion_device_add_heap(struct ion_heap *heap)
+static struct device ion_bus = {
+   .init_name = ION_NAME,
+};
+
+static struct bus_type ion_bus_type = {
+   .name = ION_NAME,
+};
+
+int ion_device_add_heap(struct ion_heap *heap)
 {
struct dentry *debug_file;
struct ion_device *dev = internal_dev;
+   int ret = 0;
 
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
   __func__);
 
+   if (heap_id >= ION_DEV_MAX)
+   return -EBUSY;
+
+   heap->ddev.parent = &ion_bus;
+   heap->ddev.bus = &ion_bus_type;
+   heap->ddev.devt = MKDEV(MAJOR(internal_dev->devt), heap_id);
+   dev_set_name(&heap->ddev, ION_NAME"%d", heap_id);
+   device_initialize(&heap->ddev);
+   cdev_init(&heap->chrdev, &ion_fops);
+   heap->chrdev.owner = THIS_MODULE;
+   ret = cdev_device_add(&heap->chrdev, &heap->ddev);
+   if (ret < 0)
+   return ret;
+
  

[Bug 102358] WarThunder freezes at start, with activated vsync (vblank_mode=2)

2017-11-06 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=102358

--- Comment #37 from har...@gmx.de ---
Ok, thanks for clarification. 
I prefer not to add such tag, because this is my anonymous email address,
dedicated to things like to games.

/Jens

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: radeon_dp_aux_transfer_native: 74 callbacks suppressed

2017-11-06 Thread Lyude Paul
The main reason I added this was because the radeon driver's hotplugging paths
for DP do a ton of unnessecary probing, and because the driver usually also
checks all connectors every time there's a hotplug (there isn't much of a good
reason for this, it's just an old driver) it's not at all difficult to get
well more then 70 callbacks from this that end up filling the kernel log with
useless information (all the messages mean is that for some reason or another,
a DP aux transaction fails which usually just means the port is disconnected).

Ideally: We should be printing the first error code that the i2c handlers for
radeon return in addition to the suppressed messages (and this itself should
not be suppressed iirc), since that's almost always the only piece of
information that matters.

I wouldn't drop the message entirely though unless we're printing something
from DRM. Additionally, we should also just fix this ratelimit macro anyway
since it's intended purpose is not to print anything when debugging isn't
enabled. What do you think Alex?

On Mon, 2017-11-06 at 10:21 +0100, Jean Delvare wrote:
> Hi all,
> 
> commit 92c177b7947d9c889ea7b024871445015ea74221
> Author: Lyude
> Date:   Wed Feb 22 16:34:53 2017 -0500
> 
> drm/radeon/dp_auxch: Ratelimit aux transfer debug messages
> 
> does more harm than good in my opinion. Since this commit, I see
> several occurrences of the following message in my kernel log daily:
> 
> radeon_dp_aux_transfer_native: 74 callbacks suppressed
> 
> I never got to see the "callback" in question though, not even once, as
> this is a debug message which is off by default. Before the change, I
> would not get any such message in the kernel log (as I would expect
> when everything works as intended.)
> 
> Does this debug message really have any practical value? If not, the
> easiest solution would be to simply drop it:
> 
> --- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
> @@ -168,8 +168,10 @@ radeon_dp_aux_transfer_native(struct drm
>   goto done;
>   }
>   if (tmp & AUX_RX_ERROR_FLAGS) {
> - DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero:
> %08x\n",
> -   tmp);
> + /*
> +  * aux transfers always fail with non-zero status flags
> when
> +  * there's nothing connected on the port
> +  */
>   ret = -EIO;
>   goto done;
>   }
> 
> I can resend this as a formal patch if you agree with this solution.
> 
> The actual cause of the problem is that ___ratelimit() prints its
> message at KERN_WARNING level regardless of the level of the message
> being suppressed. This makes ratelimiting debug, info or notice
> messages awkward. Looks like a design overlook to me, maybe it should
> be fixed, but that's a much bigger and intrusive change than the
> proposal above.
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/2] dma-buf: keep only not signaled fence in reservation_object_add_shared_replace v2

2017-11-06 Thread Chris Wilson
Quoting Christian König (2017-10-30 14:59:03)
> From: Christian König 
> 
> The amdgpu issue to also need signaled fences in the reservation objects
> should be fixed by now.
> 
> Optimize the list by keeping only the not signaled yet fences around.
> 
> v2: temporary put the signaled fences at the end of the new container
> 
> Signed-off-by: Christian König 
> ---
>  drivers/dma-buf/reservation.c | 36 ++--
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index b44d9d7db347..6fc794576997 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -145,8 +145,8 @@ reservation_object_add_shared_replace(struct 
> reservation_object *obj,
>   struct reservation_object_list *fobj,
>   struct dma_fence *fence)
>  {
> -   unsigned i;
> struct dma_fence *old_fence = NULL;
> +   unsigned i, j, k;
>  
> dma_fence_get(fence);
>  
> @@ -162,9 +162,7 @@ reservation_object_add_shared_replace(struct 
> reservation_object *obj,
>  * references from the old struct are carried over to
>  * the new.
>  */
> -   fobj->shared_count = old->shared_count;
> -
> -   for (i = 0; i < old->shared_count; ++i) {
> +   for (i = 0, j = 0, k = fobj->shared_max; i < old->shared_count; ++i) {
> struct dma_fence *check;
>  
> check = rcu_dereference_protected(old->shared[i],
> @@ -172,10 +170,18 @@ reservation_object_add_shared_replace(struct 
> reservation_object *obj,
>  
> if (!old_fence && check->context == fence->context) {
> old_fence = check;
> -   RCU_INIT_POINTER(fobj->shared[i], fence);
> -   } else
> -   RCU_INIT_POINTER(fobj->shared[i], check);
> +   RCU_INIT_POINTER(fobj->shared[j++], fence);
> +   } else if (!dma_fence_is_signaled(check)) {
> +   RCU_INIT_POINTER(fobj->shared[j++], check);
> +   } else {
> +   /*
> +* Temporary save the signaled fences at the end of 
> the
> +* new container
> +*/
> +   RCU_INIT_POINTER(fobj->shared[--k], check);
> +   }
> }
> +   fobj->shared_count = j;
> if (!old_fence) {
> RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
> fobj->shared_count++;
> @@ -192,10 +198,20 @@ reservation_object_add_shared_replace(struct 
> reservation_object *obj,
> write_seqcount_end(&obj->seq);
> preempt_enable();
>  
> -   if (old)
> -   kfree_rcu(old, rcu);
> -
> dma_fence_put(old_fence);

We don't need to keep old_fence (local var) around any more in this
scheme. By construction, the new shared_list is large enough to hold all
the old_fences+1, so you can use a loop like

j = 0;
k = fobj->shared_max;
RCU_INIT_POINTER(fobj->shared[j++], fence);
for (i = 0; i < old->shared_count; ++i) {
struct dma_fence *check;

check = rcu_dereference_protected(old->shared[i],
  reservation_object_held(obj));

if (check->context == fence->context ||
dma_fence_is_signaled(check))
RCU_INIT_POINTER(fobj->shared[--k], check);
else
RCU_INIT_POINTER(fobj->shared[j++], check);
}
fobj->shared_count = j;

> +
> +   if (!old)
> +   return;
> +
> +   /* Drop the references to the signaled fences */
> +   for (i = k; i < fobj->shared_max; ++i) {
> +   struct dma_fence *f;
> +
> +   f = rcu_dereference_protected(fobj->shared[i],
> + reservation_object_held(obj));
> +   dma_fence_put(f);
> +   }
> +   kfree_rcu(old, rcu);
>  }

Fwiw, this fixes a big problem with the lack of pruning of the
reservation_object, so \o/
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 17/22] drm/vc4: Use drm_fb_cma_fbdev_init/fini()

2017-11-06 Thread Eric Anholt
Noralf Trønnes  writes:

> Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
> the fact that drm_device holds a pointer to the drm_fb_helper structure.
> This means that the driver doesn't have to keep track of that.
> Also use the drm_fb_helper functions directly.

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Intel-gfx] [PATCH] drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, v2.

2017-11-06 Thread Ville Syrjälä
On Mon, Nov 06, 2017 at 04:43:17PM +0100, Maarten Lankhorst wrote:
> Op 06-11-17 om 15:06 schreef Ville Syrjälä:
> > On Mon, Nov 06, 2017 at 04:01:20PM +0200, Ville Syrjälä wrote:
> >> On Thu, Nov 02, 2017 at 09:55:40AM +0100, Maarten Lankhorst wrote:
> >>> Op 01-11-17 om 18:00 schreef Ville Syrjälä:
>  On Wed, Nov 01, 2017 at 04:55:06PM +0100, Maarten Lankhorst wrote:
> > Op 01-11-17 om 16:29 schreef Ville Syrjälä:
> >> On Wed, Nov 01, 2017 at 04:04:33PM +0100, Maarten Lankhorst wrote:
> >>> This introduces a slight behavioral change to rmfb. Instead of
> >>> disabling a crtc when the primary plane is disabled, we try to
> >>> preserve it.
> >>>
> >>> Apart from old versions of the vmwgfx xorg driver, there is
> >>> nothing depending on rmfb disabling a crtc.
> >>>
> >>> Vmwgfx' and simple kms helper atomic implementation rejects CRTC
> >>> enabled without plane, so we can do this safely.
>  The code for those seems a bit inconsistent. The crtc check requires
>  that the crtc state and plane state match. But the plane check allows
>  the plane to be enabled w/o the crtc being enabled. I guess it doesn't
>  matter really since you can't enable the plane without a crtc, and the
>  crtc check would then catch the case where the crtc would be disabled.
> >>> Exactly. :-) Only when a plane is unbound and stays unbound, the crtc 
> >>> check won't
> >>> be invoked. Hence it's the most accurate way of making sure that
> >>> crtc enabled <=> primary plane bound.
> >>>
> >>> If the check was done in the primary plane, an atomic modeset could enable
> >>> the crtc without enabling the primary plane, which shouldn't be allowed 
> >>> but
> >>> a plane check won't catch it.
> >>> This has been a bug in simple-kms-helper, fixed in the below commit:
> >>>
> >>> commit 765831dc27ab141b3a0be1ab55b922b012427902
> >>> Author: Maarten Lankhorst 
> >>> Date:   Wed Jul 12 10:13:29 2017 +0200
> >>>
> >>> drm/simple-kms-helper: Fix the check for the mismatch between plane 
> >>> and CRTC enabled.
> >> Hmm. OK that part looks OK. What does seem a bit inconsistent is the
> >> fact that we pass can_update_disabled=true, but later on we reject the
> >> update when visible==false. The old code would have accepted that
> >> because it didn't even call drm_plane_helper_check_state() when the
> >> crtc (and thus also the plane) was disabled.
> > Actually how does this work at all? If you turn off both the crtc and
> > plane, then the plane check will always return -EINVAL on account of
> > visible==false?
> >
> If the crtc is turned off, enabled = false and the primary plane is not in 
> crtc_state->plane_mask.
>
> Visibility is ignored in this check, that part is handled in plane check that 
> the framebuffer has to span the entire crtc if enabled.

Hmm. I thought the !crtc->enabled check disappeared from
drm_simple_kms_plane_atomic_check() but looks like I was wrong and it's
still there. OK, just totally ignore what I wrote before. I guess one
shouldn't try to figure out what the code is going to be doing while in
the middle of an unrelated bisect.

Though for some extra consistency we might want to change that to check
to look for !fb after calling drm_plane_helper_check_state(). That way
we wouldn't have to change drivers/simple_kms_helper if come up with
something that has to be checked even for disabled planes
in drm_plane_helper_check_state().

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 2/2] dma-buf: try to replace a signaled fence in reservation_object_add_shared_inplace

2017-11-06 Thread Chris Wilson
Quoting Christian König (2017-10-30 14:59:04)
> From: Christian König 
> 
> The amdgpu issue to also need signaled fences in the reservation objects 
> should
> be fixed by now.
> 
> Optimize the handling by replacing a signaled fence when adding a new
> shared one.
> 
> Signed-off-by: Christian König 
> ---
>  drivers/dma-buf/reservation.c | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 6fc794576997..a3928ce9f311 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -104,7 +104,8 @@ reservation_object_add_shared_inplace(struct 
> reservation_object *obj,
>   struct reservation_object_list *fobj,
>   struct dma_fence *fence)
>  {
> -   u32 i;
> +   struct dma_fence *signaled = NULL;
> +   u32 i, signaled_idx;
>  
> dma_fence_get(fence);
>  
> @@ -126,17 +127,28 @@ reservation_object_add_shared_inplace(struct 
> reservation_object *obj,
> dma_fence_put(old_fence);
> return;
> }
> +
> +   if (!signaled && dma_fence_is_signaled(old_fence)) {
> +   signaled = old_fence;
> +   signaled_idx = i;
> +   }

How much do we care about only keeping one fence per-ctx here? You could
rearrange this to break on old_fence->context == fence->context ||
dma_fence_is_signaled(old_fence) then everyone can use the final block.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 3/3] ASoC: rt5645: Wait for 400msec before concluding on value of RT5645_VENDOR_ID2

2017-11-06 Thread Mark Brown
On Fri, Nov 03, 2017 at 04:35:45PM -0400, Alex Deucher wrote:

> Minimum time required between power On of codec and read
> of RT5645_VENDOR_ID2 is 400msec. We should wait and attempt
> before erroring out.

So the description says we have to wait 400ms before attempting a
read...

> BUG=b:66978383

What does this mean?

> @@ -3786,6 +3789,15 @@ static int rt5645_i2c_probe(struct i2c_client *i2c,
>   }
>   regmap_read(regmap, RT5645_VENDOR_ID2, &val);
>  
> + /*
> +  * Read for 400msec, as it is the interval required between
> +  * read and power On.
> +  */
> + while (val != RT5645_DEVICE_ID && val != RT5650_DEVICE_ID && --timeout) 
> {
> + msleep(1);
> + regmap_read(regmap, RT5645_VENDOR_ID2, &val);
> + }
> +

...but what we actually do is try to read up to 400 times starting well
before that 400ms is up.  This directly contradicts what the commit
message said we needed, may take a lot longer if the chip misbehaves on
the I2C bus while it's not ready (which wouldn't be that much of a
surprise), might lead to us reporting before the chip is really stable
(if the read happens to work while the chip isn't yet stable) and could
cause lots of noise on the console if the I2C controller gets upset.
What are we actually waiting for here?

If we really need 400ms of dead reckoning time (which is a lot for a
modern chip, that feels more like a VMID ramp) then it's going to be
safer to just do that.


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [gabbayo:amdkfd-next 5/8] drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c:540:3: error: dereferencing pointer to incomplete type 'struct cik_sdma_rlc_registers'

2017-11-06 Thread Felix Kuehling
I messed up while rebasing patches and didn't test every intermediate
patch as I should have. The next patch in the series fixes this.

If anyone wants to fix this before merging further upstream, remove the
offending line in this commit and reintroduce it in the next commit in
the series. At this point in the series GFX8 KFD SDMA isn't supposed to
work anyway.

Regards,
  Felix


On 2017-11-05 11:03 AM, kbuild test robot wrote:
> tree:   git://people.freedesktop.org/~gabbayo/linux amdkfd-next
> head:   c0307884529de823406fb17daf477f6af34ca5e5
> commit: 88bd109688cf71dac908e5a06915996ee79fcef6 [5/8] drm/amdgpu: Add 
> support for resuming SDMA queues w/o HWS
> config: i386-randconfig-s0-201745 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> git checkout 88bd109688cf71dac908e5a06915996ee79fcef6
> # save the attached .config to linux build tree
> make ARCH=i386 
>
> Note: the gabbayo/amdkfd-next HEAD c0307884529de823406fb17daf477f6af34ca5e5 
> builds fine.
>   It only hurts bisectibility.
>
> All errors (new ones prefixed by >>):
>
>drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c: In function 
> 'kgd_hqd_sdma_destroy':
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c:540:3: error: 
>>> dereferencing pointer to incomplete type 'struct cik_sdma_rlc_registers'
>  m->sdmax_rlcx_rb_rptr = RREG32(sdma_base_addr + mmSDMA0_RLC0_RB_RPTR);
>   ^~
>
> vim +540 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
>
>509
>510static int kgd_hqd_sdma_destroy(struct kgd_dev *kgd, void *mqd,
>511unsigned int utimeout)
>512{
>513struct amdgpu_device *adev = get_amdgpu_device(kgd);
>514struct cik_sdma_rlc_registers *m;
>515uint32_t sdma_base_addr;
>516uint32_t temp;
>517unsigned long end_jiffies = (utimeout * HZ / 1000) + 
> jiffies;
>518
>519m = get_sdma_mqd(mqd);
>520sdma_base_addr = get_sdma_base_addr(m);
>521
>522temp = RREG32(sdma_base_addr + mmSDMA0_RLC0_RB_CNTL);
>523temp = temp & ~SDMA0_RLC0_RB_CNTL__RB_ENABLE_MASK;
>524WREG32(sdma_base_addr + mmSDMA0_RLC0_RB_CNTL, temp);
>525
>526while (true) {
>527temp = RREG32(sdma_base_addr + 
> mmSDMA0_RLC0_CONTEXT_STATUS);
>528if (temp & SDMA0_STATUS_REG__RB_CMD_IDLE__SHIFT)
>529break;
>530if (time_after(jiffies, end_jiffies))
>531return -ETIME;
>532usleep_range(500, 1000);
>533}
>534
>535WREG32(sdma_base_addr + mmSDMA0_RLC0_DOORBELL, 0);
>536WREG32(sdma_base_addr + mmSDMA0_RLC0_RB_RPTR, 0);
>537WREG32(sdma_base_addr + mmSDMA0_RLC0_RB_WPTR, 0);
>538WREG32(sdma_base_addr + mmSDMA0_RLC0_RB_BASE, 0);
>539
>  > 540m->sdmax_rlcx_rb_rptr = RREG32(sdma_base_addr + 
> mmSDMA0_RLC0_RB_RPTR);
>541
>542return 0;
>543}
>544
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: linux-next: Tree for Nov 6 (amdgpu_virt.c)

2017-11-06 Thread Alex Deucher
On Mon, Nov 6, 2017 at 12:20 PM, Randy Dunlap  wrote:
> On 11/05/2017 11:30 PM, Stephen Rothwell wrote:
>> Hi all,
>>
>> Changes since 20171103:
>>
>
> on i386, when CONFIG_MODULES is not set:
>
>   CC  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o
> In file included from ../arch/x86/include/asm/atomic.h:5:0,
>  from ../include/linux/atomic.h:5,
>  from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:31,
>  from ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:24:
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c: In function 
> 'amdgpu_virt_init_data_exchange':
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing 
> pointer to incomplete type
>  if (THIS_MODULE->version != NULL)
> ^
> ../include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
>   if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
>   ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of 
> macro 'if'
>  if (THIS_MODULE->version != NULL)
>  ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing 
> pointer to incomplete type
>  if (THIS_MODULE->version != NULL)
> ^
> ../include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
>   if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
>   ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of 
> macro 'if'
>  if (THIS_MODULE->version != NULL)
>  ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing 
> pointer to incomplete type
>  if (THIS_MODULE->version != NULL)
> ^
> ../include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
>__r = !!(cond); \
> ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of 
> macro 'if'
>  if (THIS_MODULE->version != NULL)
>  ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:332:29: error: dereferencing 
> pointer to incomplete type
>   strcpy(str, THIS_MODULE->version);
>  ^
> ../scripts/Makefile.build:314: recipe for target 
> 'drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o' failed
> make[5]: *** [drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o] Error 1
>
>
>
> Reported-by: Randy Dunlap 

Thanks.  Fixed in:
https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next&id=e477e940dad1836c6f6d23353e424665b9316b6e

Alex
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 13/22] drm/stm: Use drm_fb_cma_fbdev_init/fini()

2017-11-06 Thread Philippe CORNU
Hi Noralf,

On 11/04/2017 02:04 PM, Noralf Trønnes wrote:
> Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
> the fact that drm_device holds a pointer to the drm_fb_helper structure.
> This means that the driver doesn't have to keep track of that.
> Also use the drm_fb_helper functions directly.
> 
> Cc: Yannick Fertre 
> Cc: Philippe Cornu 
> Cc: Benjamin Gaignard 
> Cc: Vincent Abriou 
> Signed-off-by: Noralf Trønnes 

Many thanks for this nice cleanup.

I will have to push a small fix to move drm_kms_helper_poll_init() after 
drm_fb_cma_fbdev_init() in order to remove a warning when booting (+ a 
small cleanup as "ldev = ddev->dev_private;" is not used anymore before 
drm_fb_cma_fbdev_init().

Anyway, regarding your patchset for stm:

Acked-by: Philippe Cornu 
Tested-by: Philippe Cornu 

Thank you,
Philippe :-)

> ---
>   drivers/gpu/drm/stm/drv.c  | 37 ++---
>   drivers/gpu/drm/stm/ltdc.h |  1 -
>   2 files changed, 6 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> index c857663eafc2..9a66f5671d4f 100644
> --- a/drivers/gpu/drm/stm/drv.c
> +++ b/drivers/gpu/drm/stm/drv.c
> @@ -15,6 +15,7 @@
>   #include 
>   #include 
>   #include 
> +#include 
>   #include 
>   #include 
>   #include 
> @@ -24,35 +25,19 @@
>   #define STM_MAX_FB_WIDTH2048
>   #define STM_MAX_FB_HEIGHT   2048 /* same as width to handle orientation */
>   
> -static void drv_output_poll_changed(struct drm_device *ddev)
> -{
> - struct ltdc_device *ldev = ddev->dev_private;
> -
> - drm_fbdev_cma_hotplug_event(ldev->fbdev);
> -}
> -
>   static const struct drm_mode_config_funcs drv_mode_config_funcs = {
>   .fb_create = drm_gem_fb_create,
> - .output_poll_changed = drv_output_poll_changed,
> + .output_poll_changed = drm_fb_helper_output_poll_changed,
>   .atomic_check = drm_atomic_helper_check,
>   .atomic_commit = drm_atomic_helper_commit,
>   };
>   
> -static void drv_lastclose(struct drm_device *ddev)
> -{
> - struct ltdc_device *ldev = ddev->dev_private;
> -
> - DRM_DEBUG("%s\n", __func__);
> -
> - drm_fbdev_cma_restore_mode(ldev->fbdev);
> -}
> -
>   DEFINE_DRM_GEM_CMA_FOPS(drv_driver_fops);
>   
>   static struct drm_driver drv_driver = {
>   .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
>  DRIVER_ATOMIC,
> - .lastclose = drv_lastclose,
> + .lastclose = drm_fb_helper_lastclose,
>   .name = "stm",
>   .desc = "STMicroelectronics SoC DRM",
>   .date = "20170330",
> @@ -79,7 +64,6 @@ static struct drm_driver drv_driver = {
>   static int drv_load(struct drm_device *ddev)
>   {
>   struct platform_device *pdev = to_platform_device(ddev->dev);
> - struct drm_fbdev_cma *fbdev;
>   struct ltdc_device *ldev;
>   int ret;
>   
> @@ -113,13 +97,9 @@ static int drv_load(struct drm_device *ddev)
>   
>   if (ddev->mode_config.num_connector) {
>   ldev = ddev->dev_private;
> - fbdev = drm_fbdev_cma_init(ddev, 16,
> -ddev->mode_config.num_connector);
> - if (IS_ERR(fbdev)) {
> + ret = drm_fb_cma_fbdev_init(ddev, 16, 0);
> + if (ret)
>   DRM_DEBUG("Warning: fails to create fbdev\n");
> - fbdev = NULL;
> - }
> - ldev->fbdev = fbdev;
>   }
>   
>   platform_set_drvdata(pdev, ddev);
> @@ -132,14 +112,9 @@ static int drv_load(struct drm_device *ddev)
>   
>   static void drv_unload(struct drm_device *ddev)
>   {
> - struct ltdc_device *ldev = ddev->dev_private;
> -
>   DRM_DEBUG("%s\n", __func__);
>   
> - if (ldev->fbdev) {
> - drm_fbdev_cma_fini(ldev->fbdev);
> - ldev->fbdev = NULL;
> - }
> + drm_fb_cma_fbdev_fini(ddev);
>   drm_kms_helper_poll_fini(ddev);
>   ltdc_unload(ddev);
>   drm_mode_config_cleanup(ddev);
> diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
> index ae437557d715..27ac217406be 100644
> --- a/drivers/gpu/drm/stm/ltdc.h
> +++ b/drivers/gpu/drm/stm/ltdc.h
> @@ -21,7 +21,6 @@ struct ltdc_caps {
>   };
>   
>   struct ltdc_device {
> - struct drm_fbdev_cma *fbdev;
>   void __iomem *regs;
>   struct clk *pixel_clk;  /* lcd pixel clock */
>   struct mutex err_lock;  /* protecting error_status */
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Intel-gfx] [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping

2017-11-06 Thread Ville Syrjälä
On Thu, Nov 02, 2017 at 03:19:30PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 02, 2017 at 11:12:09AM +0100, Daniel Vetter wrote:
> > On Wed, Nov 01, 2017 at 08:29:18PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä 
> > > 
> > > Try to fix the code to actually clip the plane to the crtc bounds
> > > instead of the user provided crtc coordinates (which would be a no-op
> > > since those are exactly the coordinates before clipping).
> > > 
> > > Cc: VMware Graphics 
> > > Cc: Sinclair Yeh 
> > > Cc: Thomas Hellstrom 
> > > Signed-off-by: Ville Syrjälä 
> > 
> > I kinda wonder whether we shouldn't push this down into the helper ...
> 
> Would require quite going through all drivers making sure they are
> happy with using the adjusted_mode.crtc_ timings. I think most drivers
> use the other adjusted_mode timings currently, and some even use the
> user mode timings (which could be an actual bug).

Let me take that back. What we want is to clip to the user mode
timings. Stereo 3D needs the special treatment from
drm_mode_get_hv_timing(). I'm getting confused by all these
different timings we have all over the place.

I think for i915 all we would need is to change the double wide/dual
link adjustent for pipe_src_w to simply reject odd widths instead.
That would guarantee that the user mode timings match the pipe_src_w/h
100%.

For the other driver we'd need to figure out why they're using
adjusted_mode timings for clipping. I guess that's just a mistake,
which I repeated here with vmwgfx after getting confused by looking
at the other drivers.

I guess I just volunteered myself to do this. Just needs plenty of
acks from driver maintainers I suppose.

> 
> > 
> > Either way, Reviewed-by: Daniel Vetter 
> > 
> > > ---
> > >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 +--
> > >  1 file changed, 13 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
> > > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > index 515b67783a41..efa41c086198 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > @@ -441,20 +441,23 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane 
> > > *plane,
> > >  int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
> > > struct drm_plane_state *state)
> > >  {
> > > + struct drm_crtc_state *crtc_state = NULL;
> > >   struct drm_framebuffer *new_fb = state->fb;
> > > - struct drm_rect clip = {
> > > - .x1 = state->crtc_x,
> > > - .y1 = state->crtc_y,
> > > - .x2 = state->crtc_x + state->crtc_w,
> > > - .y2 = state->crtc_y + state->crtc_h,
> > > - };
> > > + struct drm_rect clip = {};
> > >   int ret;
> > >  
> > > - ret = drm_plane_helper_check_state(state, &clip,
> > > - DRM_PLANE_HELPER_NO_SCALING,
> > > - DRM_PLANE_HELPER_NO_SCALING,
> > > - false, true);
> > > + if (state->crtc)
> > > + crtc_state = drm_atomic_get_new_crtc_state(state->state, 
> > > state->crtc);
> > >  
> > > + if (crtc_state && crtc_state->enable) {
> > > + clip.x2 = crtc_state->adjusted_mode.hdisplay;
> > > + clip.y2 = crtc_state->adjusted_mode.vdisplay;
> > > + }
> > > +
> > > + ret = drm_plane_helper_check_state(state, &clip,
> > > +DRM_PLANE_HELPER_NO_SCALING,
> > > +DRM_PLANE_HELPER_NO_SCALING,
> > > +false, true);
> > >  
> > >   if (!ret && new_fb) {
> > >   struct drm_crtc *crtc = state->crtc;
> > > -- 
> > > 2.13.6
> > > 
> > > ___
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2] video: atmel_lcdfb: Use unique error messages in atmel_lcdfb_of_init()

2017-11-06 Thread SF Markus Elfring
From: Markus Elfring 
Date: Mon, 6 Nov 2017 19:00:58 +0100

A duplicate error message was used so far in this function implementation.
Thus use a consistent message format instead together with property names
where constant merging can be applied by the compiler in four cases.

This issue was detected by using the Coccinelle software.


This software update supersedes the suggestion "video: atmel_lcdfb:
Use common error handling code in atmel_lcdfb_of_init()" from 2017-11-05.
https://patchwork.kernel.org/patch/10042131/

Fixes: b985172b328abc6de495a87ab998b267c5a09c16 ("video: atmel_lcdfb: add 
device tree suport")

Signed-off-by: Markus Elfring 
---
 drivers/video/fbdev/atmel_lcdfb.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/atmel_lcdfb.c 
b/drivers/video/fbdev/atmel_lcdfb.c
index e06358da4b99..44146aec3596 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -1021,6 +1021,8 @@ static void atmel_lcdfb_power_control_gpio(struct 
atmel_lcdfb_pdata *pdata, int
gpio_set_value(og->gpio, on);
 }
 
+static char const property_failure[] = "failed to get property %s\n";
+
 static int atmel_lcdfb_of_init(struct atmel_lcdfb_info *sinfo)
 {
struct fb_info *info = sinfo->info;
@@ -1048,25 +1050,25 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info 
*sinfo)
 
ret = of_property_read_u32(display_np, "bits-per-pixel", 
&var->bits_per_pixel);
if (ret < 0) {
-   dev_err(dev, "failed to get property bits-per-pixel\n");
+   dev_err(dev, property_failure, "bits-per-pixel");
goto put_display_node;
}
 
ret = of_property_read_u32(display_np, "atmel,guard-time", 
&pdata->guard_time);
if (ret < 0) {
-   dev_err(dev, "failed to get property atmel,guard-time\n");
+   dev_err(dev, property_failure, "atmel,guard-time");
goto put_display_node;
}
 
ret = of_property_read_u32(display_np, "atmel,lcdcon2", 
&pdata->default_lcdcon2);
if (ret < 0) {
-   dev_err(dev, "failed to get property atmel,lcdcon2\n");
+   dev_err(dev, property_failure, "atmel,lcdcon2");
goto put_display_node;
}
 
ret = of_property_read_u32(display_np, "atmel,dmacon", 
&pdata->default_dmacon);
if (ret < 0) {
-   dev_err(dev, "failed to get property bits-per-pixel\n");
+   dev_err(dev, property_failure, "atmel,dmacon");
goto put_display_node;
}
 
-- 
2.15.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: radeon_dp_aux_transfer_native: 74 callbacks suppressed

2017-11-06 Thread Christian König

Additionally, we should also just fix this ratelimit macro anyway
since it's intended purpose is not to print anything when debugging isn't
enabled. What do you think Alex?
Well, at least I think that the described behavior of the macro is a bug 
which should be fixed.


But I think that is independent of the problem that radeon is a bit 
flaky about DP aux transfers.


Regards,
Christian.

Am 06.11.2017 um 17:17 schrieb Lyude Paul:

The main reason I added this was because the radeon driver's hotplugging paths
for DP do a ton of unnessecary probing, and because the driver usually also
checks all connectors every time there's a hotplug (there isn't much of a good
reason for this, it's just an old driver) it's not at all difficult to get
well more then 70 callbacks from this that end up filling the kernel log with
useless information (all the messages mean is that for some reason or another,
a DP aux transaction fails which usually just means the port is disconnected).

Ideally: We should be printing the first error code that the i2c handlers for
radeon return in addition to the suppressed messages (and this itself should
not be suppressed iirc), since that's almost always the only piece of
information that matters.

I wouldn't drop the message entirely though unless we're printing something
from DRM. Additionally, we should also just fix this ratelimit macro anyway
since it's intended purpose is not to print anything when debugging isn't
enabled. What do you think Alex?

On Mon, 2017-11-06 at 10:21 +0100, Jean Delvare wrote:

Hi all,

commit 92c177b7947d9c889ea7b024871445015ea74221
Author: Lyude
Date:   Wed Feb 22 16:34:53 2017 -0500

 drm/radeon/dp_auxch: Ratelimit aux transfer debug messages

does more harm than good in my opinion. Since this commit, I see
several occurrences of the following message in my kernel log daily:

radeon_dp_aux_transfer_native: 74 callbacks suppressed

I never got to see the "callback" in question though, not even once, as
this is a debug message which is off by default. Before the change, I
would not get any such message in the kernel log (as I would expect
when everything works as intended.)

Does this debug message really have any practical value? If not, the
easiest solution would be to simply drop it:

--- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
@@ -168,8 +168,10 @@ radeon_dp_aux_transfer_native(struct drm
goto done;
}
if (tmp & AUX_RX_ERROR_FLAGS) {
-   DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero:
%08x\n",
- tmp);
+   /*
+* aux transfers always fail with non-zero status flags
when
+* there's nothing connected on the port
+*/
ret = -EIO;
goto done;
}

I can resend this as a formal patch if you agree with this solution.

The actual cause of the problem is that ___ratelimit() prints its
message at KERN_WARNING level regardless of the level of the message
being suppressed. This makes ratelimiting debug, info or notice
messages awkward. Looks like a design overlook to me, maybe it should
be fixed, but that's a much bigger and intrusive change than the
proposal above.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2] video: atmel_lcdfb: Use unique error messages in atmel_lcdfb_of_init()

2017-11-06 Thread Joe Perches
On Mon, 2017-11-06 at 19:14 +0100, SF Markus Elfring wrote:
> A duplicate error message was used so far in this function implementation.
> Thus use a consistent message format instead together with property names
> where constant merging can be applied by the compiler in four cases.
[]
> diff --git a/drivers/video/fbdev/atmel_lcdfb.c 
> b/drivers/video/fbdev/atmel_lcdfb.c
[]
> @@ -1021,6 +1021,8 @@ static void atmel_lcdfb_power_control_gpio(struct 
> atmel_lcdfb_pdata *pdata, int
>   gpio_set_value(og->gpio, on);
>  }
>  
> +static char const property_failure[] = "failed to get property %s\n";

Yuck.  This makes it harder to grep the sources.
Just use the normal format string in each place.

> +
>  static int atmel_lcdfb_of_init(struct atmel_lcdfb_info *sinfo)
>  {
>   struct fb_info *info = sinfo->info;
> @@ -1048,25 +1050,25 @@ static int atmel_lcdfb_of_init(struct 
> atmel_lcdfb_info *sinfo)
>  
>   ret = of_property_read_u32(display_np, "bits-per-pixel", 
> &var->bits_per_pixel);
>   if (ret < 0) {
> - dev_err(dev, "failed to get property bits-per-pixel\n");
> + dev_err(dev, property_failure, "bits-per-pixel");

This likely doesn't even save any code size.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 21/22] drm/tinydrm: Use drm_fb_cma_fbdev_init_with_funcs/fini()

2017-11-06 Thread David Lechner

On 11/04/2017 08:04 AM, Noralf Trønnes wrote:

Use drm_fb_cma_fbdev_init_with_funcs() and drm_fb_cma_fbdev_fini() which
relies on the fact that drm_device holds a pointer to the drm_fb_helper
structure. This means that the driver doesn't have to keep track of that.
Also use the drm_fb_helper functions directly.
Remove todo entry.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---


Acked-by: David Lechner 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 3/6] drm/arm/mali: Use drm_mode_config_helper_suspend/resume()

2017-11-06 Thread Noralf Trønnes
Replace driver's code with the generic helpers that do the same thing.

Cc: Liviu Dudau 
Cc: Brian Starkey 
Signed-off-by: Noralf Trønnes 
Reviewed-by: Liviu Dudau 
---
 drivers/gpu/drm/arm/malidp_drv.c | 24 +++-
 drivers/gpu/drm/arm/malidp_drv.h |  1 -
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index b8944666a18f..75f0bce33941 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "malidp_drv.h"
@@ -749,34 +750,15 @@ static int malidp_platform_remove(struct platform_device 
*pdev)
 static int __maybe_unused malidp_pm_suspend(struct device *dev)
 {
struct drm_device *drm = dev_get_drvdata(dev);
-   struct malidp_drm *malidp = drm->dev_private;
 
-   drm_kms_helper_poll_disable(drm);
-   console_lock();
-   drm_fbdev_cma_set_suspend(malidp->fbdev, 1);
-   console_unlock();
-   malidp->pm_state = drm_atomic_helper_suspend(drm);
-   if (IS_ERR(malidp->pm_state)) {
-   console_lock();
-   drm_fbdev_cma_set_suspend(malidp->fbdev, 0);
-   console_unlock();
-   drm_kms_helper_poll_enable(drm);
-   return PTR_ERR(malidp->pm_state);
-   }
-
-   return 0;
+   return drm_mode_config_helper_suspend(drm);
 }
 
 static int __maybe_unused malidp_pm_resume(struct device *dev)
 {
struct drm_device *drm = dev_get_drvdata(dev);
-   struct malidp_drm *malidp = drm->dev_private;
 
-   drm_atomic_helper_resume(drm, malidp->pm_state);
-   console_lock();
-   drm_fbdev_cma_set_suspend(malidp->fbdev, 0);
-   console_unlock();
-   drm_kms_helper_poll_enable(drm);
+   drm_mode_config_helper_resume(drm);
 
return 0;
 }
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index 2e2033140efc..70ed6aeccf05 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -24,7 +24,6 @@ struct malidp_drm {
struct drm_crtc crtc;
wait_queue_head_t wq;
atomic_t config_valid;
-   struct drm_atomic_state *pm_state;
u32 core_id;
 };
 
-- 
2.14.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 6/6] drm/docs: Add todo entry for simple modeset suspend/resume

2017-11-06 Thread Noralf Trønnes
Add entry for conversion of drivers to new helpers.

Signed-off-by: Noralf Trønnes 
Reviewed-by: Daniel Vetter 
---
 Documentation/gpu/todo.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index a44f379d2b25..6bce1beafabe 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -185,6 +185,15 @@ are better.
 
 Contact: Sean Paul, Maintainer of the driver you plan to convert
 
+Convert drivers to use simple modeset suspend/resume
+
+
+Most drivers (except i915 and nouveau) that use
+drm_atomic_helper_suspend/resume() can probably be converted to use
+drm_mode_config_helper_suspend/resume().
+
+Contact: Maintainer of the driver you plan to convert
+
 Core refactorings
 =
 
-- 
2.14.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 1/6] drm/probe-helper: Fix drm_kms_helper_poll_enable() docs

2017-11-06 Thread Noralf Trønnes
Fix docs to reflect code and drm_kms_helper_poll_disable() docs by saying
that calling drm_kms_helper_poll_enable() is fine even if output polling
is not enabled.

Signed-off-by: Noralf Trønnes 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_probe_helper.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index 5840aabbf24e..024a89bf0ba7 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -216,8 +216,7 @@ enum drm_mode_status drm_connector_mode_valid(struct 
drm_connector *connector,
  * suspend/resume.
  *
  * Drivers can call this helper from their device resume implementation. It is
- * an error to call this when the output polling support has not yet been set
- * up.
+ * not an error to call this even when output polling isn't enabled.
  *
  * Note that calls to enable and disable polling must be strictly ordered, 
which
  * is automatically the case when they're only call from suspend/resume
-- 
2.14.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 0/6] drm/: Add simple modeset suspend/resume helpers

2017-11-06 Thread Noralf Trønnes
This patchset adds some simple modeset suspend/resume helpers which
probably most atomic drivers can use.

For those that haven't followed dri-devel closely the past few days,
this patch put in place the fbdev piece necessary to do this:
drm: Add drm_device->fb_helper pointer
https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/drm_fb_
helper.c?id=29ad20b22c8f3ab35e91c2f68b4c7956cee30fd0

I have converted the cma helper drivers as part of my ongoing cma helper
refactoring.

Noralf.

Changes since version 1:
- Improve driver commit message (Liviu)
- fsl-dcu: Fix build error: 'ret' undeclared

Noralf Trønnes (6):
  drm/probe-helper: Fix drm_kms_helper_poll_enable() docs
  drm/modeset-helper: Add simple modeset suspend/resume helpers
  drm/arm/mali: Use drm_mode_config_helper_suspend/resume()
  drm/fsl-dcu: Use drm_mode_config_helper_suspend/resume()
  drm/tinydrm: Use drm_mode_config_helper_suspend/resume()
  drm/docs: Add todo entry for simple modeset suspend/resume

 Documentation/gpu/todo.rst  | 14 --
 drivers/gpu/drm/arm/malidp_drv.c| 24 ++---
 drivers/gpu/drm/arm/malidp_drv.h|  1 -
 drivers/gpu/drm/drm_modeset_helper.c| 76 +
 drivers/gpu/drm/drm_probe_helper.c  |  3 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   | 24 +++--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   |  1 -
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 67 -
 drivers/gpu/drm/tinydrm/mi0283qt.c  |  7 ++-
 include/drm/drm_mode_config.h   |  9 
 include/drm/drm_modeset_helper.h|  3 ++
 include/drm/tinydrm/tinydrm.h   |  4 --
 12 files changed, 112 insertions(+), 121 deletions(-)

-- 
2.14.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 2/6] drm/modeset-helper: Add simple modeset suspend/resume helpers

2017-11-06 Thread Noralf Trønnes
Add drm_mode_config_helper_suspend/resume() which takes care of
atomic modeset suspend/resume for simple use cases.
The suspend state is stored in struct drm_mode_config.

Signed-off-by: Noralf Trønnes 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_modeset_helper.c | 76 
 include/drm/drm_mode_config.h|  9 +
 include/drm/drm_modeset_helper.h |  3 ++
 3 files changed, 88 insertions(+)

diff --git a/drivers/gpu/drm/drm_modeset_helper.c 
b/drivers/gpu/drm/drm_modeset_helper.c
index 9cb1eede0b4d..f1c24ab0ef09 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -20,6 +20,9 @@
  * OF THIS SOFTWARE.
  */
 
+#include 
+#include 
+#include 
 #include 
 #include 
 
@@ -156,3 +159,76 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc 
*crtc,
 NULL);
 }
 EXPORT_SYMBOL(drm_crtc_init);
+
+/**
+ * drm_mode_config_helper_suspend - Modeset suspend helper
+ * @dev: DRM device
+ *
+ * This helper function takes care of suspending the modeset side. It disables
+ * output polling if initialized, suspends fbdev if used and finally calls
+ * drm_atomic_helper_suspend().
+ * If suspending fails, fbdev and polling is re-enabled.
+ *
+ * Returns:
+ * Zero on success, negative error code on error.
+ *
+ * See also:
+ * drm_kms_helper_poll_disable() and drm_fb_helper_set_suspend_unlocked().
+ */
+int drm_mode_config_helper_suspend(struct drm_device *dev)
+{
+   struct drm_atomic_state *state;
+
+   if (!dev)
+   return 0;
+
+   drm_kms_helper_poll_disable(dev);
+   drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 1);
+   state = drm_atomic_helper_suspend(dev);
+   if (IS_ERR(state)) {
+   drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
+   drm_kms_helper_poll_enable(dev);
+   return PTR_ERR(state);
+   }
+
+   dev->mode_config.suspend_state = state;
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_mode_config_helper_suspend);
+
+/**
+ * drm_mode_config_helper_resume - Modeset resume helper
+ * @dev: DRM device
+ *
+ * This helper function takes care of resuming the modeset side. It calls
+ * drm_atomic_helper_resume(), resumes fbdev if used and enables output polling
+ * if initiaized.
+ *
+ * Returns:
+ * Zero on success, negative error code on error.
+ *
+ * See also:
+ * drm_fb_helper_set_suspend_unlocked() and drm_kms_helper_poll_enable().
+ */
+int drm_mode_config_helper_resume(struct drm_device *dev)
+{
+   int ret;
+
+   if (!dev)
+   return 0;
+
+   if (WARN_ON(!dev->mode_config.suspend_state))
+   return -EINVAL;
+
+   ret = drm_atomic_helper_resume(dev, dev->mode_config.suspend_state);
+   if (ret)
+   DRM_ERROR("Failed to resume (%d)\n", ret);
+   dev->mode_config.suspend_state = NULL;
+
+   drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
+   drm_kms_helper_poll_enable(dev);
+
+   return ret;
+}
+EXPORT_SYMBOL(drm_mode_config_helper_resume);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 1b37368416c8..5a872496b409 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -766,6 +766,15 @@ struct drm_mode_config {
/* cursor size */
uint32_t cursor_width, cursor_height;
 
+   /**
+* @suspend_state:
+*
+* Atomic state when suspended.
+* Set by drm_mode_config_helper_suspend() and cleared by
+* drm_mode_config_helper_resume().
+*/
+   struct drm_atomic_state *suspend_state;
+
const struct drm_mode_config_helper_funcs *helper_private;
 };
 
diff --git a/include/drm/drm_modeset_helper.h b/include/drm/drm_modeset_helper.h
index cb0ec92e11e6..efa337f03129 100644
--- a/include/drm/drm_modeset_helper.h
+++ b/include/drm/drm_modeset_helper.h
@@ -34,4 +34,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device *dev,
 int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
  const struct drm_crtc_funcs *funcs);
 
+int drm_mode_config_helper_suspend(struct drm_device *dev);
+int drm_mode_config_helper_resume(struct drm_device *dev);
+
 #endif
-- 
2.14.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 4/6] drm/fsl-dcu: Use drm_mode_config_helper_suspend/resume()

2017-11-06 Thread Noralf Trønnes
Replace driver's code with the generic helpers that do the same thing.

Cc: Stefan Agner 
Cc: Alison Wang 
Signed-off-by: Noralf Trønnes 
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 24 ++--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 -
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c 
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 58e9e0601a61..1a9ee657bbac 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "fsl_dcu_drm_crtc.h"
 #include "fsl_dcu_drm_drv.h"
@@ -188,26 +189,17 @@ static struct drm_driver fsl_dcu_drm_driver = {
 static int fsl_dcu_drm_pm_suspend(struct device *dev)
 {
struct fsl_dcu_drm_device *fsl_dev = dev_get_drvdata(dev);
+   int ret;
 
if (!fsl_dev)
return 0;
 
disable_irq(fsl_dev->irq);
-   drm_kms_helper_poll_disable(fsl_dev->drm);
 
-   console_lock();
-   drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 1);
-   console_unlock();
-
-   fsl_dev->state = drm_atomic_helper_suspend(fsl_dev->drm);
-   if (IS_ERR(fsl_dev->state)) {
-   console_lock();
-   drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 0);
-   console_unlock();
-
-   drm_kms_helper_poll_enable(fsl_dev->drm);
+   ret = drm_mode_config_helper_suspend(fsl_dev->drm);
+   if (ret) {
enable_irq(fsl_dev->irq);
-   return PTR_ERR(fsl_dev->state);
+   return ret;
}
 
clk_disable_unprepare(fsl_dev->pix_clk);
@@ -233,13 +225,9 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
if (fsl_dev->tcon)
fsl_tcon_bypass_enable(fsl_dev->tcon);
fsl_dcu_drm_init_planes(fsl_dev->drm);
-   drm_atomic_helper_resume(fsl_dev->drm, fsl_dev->state);
 
-   console_lock();
-   drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 0);
-   console_unlock();
+   drm_mode_config_helper_resume(fsl_dev->drm);
 
-   drm_kms_helper_poll_enable(fsl_dev->drm);
enable_irq(fsl_dev->irq);
 
return 0;
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h 
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
index da9bfd432ca6..93bfb98012d4 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
@@ -196,7 +196,6 @@ struct fsl_dcu_drm_device {
struct drm_encoder encoder;
struct fsl_dcu_drm_connector connector;
const struct fsl_dcu_soc_data *soc;
-   struct drm_atomic_state *state;
 };
 
 int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev);
-- 
2.14.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 5/6] drm/tinydrm: Use drm_mode_config_helper_suspend/resume()

2017-11-06 Thread Noralf Trønnes
Replace driver's code with the generic helpers that do the same thing.
Remove todo entry.

Signed-off-by: Noralf Trønnes 
---
 Documentation/gpu/todo.rst  |  5 ---
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 67 -
 drivers/gpu/drm/tinydrm/mi0283qt.c  |  7 ++-
 include/drm/tinydrm/tinydrm.h   |  4 --
 4 files changed, 5 insertions(+), 78 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index e9840d693a86..a44f379d2b25 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -404,11 +404,6 @@ those drivers as simple as possible, so lots of room for 
refactoring:
   a drm_device wrong. Doesn't matter, since everyone else gets it wrong
   too :-)
 
-- With the fbdev pointer in dev->mode_config we could also make
-  suspend/resume helpers entirely generic, at least if we add a
-  dev->mode_config.suspend_state. We could even provide a generic pm_ops
-  structure with those.
-
 - also rework the drm_framebuffer_funcs->dirty hook wire-up, see above.
 
 Contact: Noralf Trønnes, Daniel Vetter
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c 
b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index 1a8a57cad431..bd7b82824a34 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -292,71 +292,4 @@ void tinydrm_shutdown(struct tinydrm_device *tdev)
 }
 EXPORT_SYMBOL(tinydrm_shutdown);
 
-/**
- * tinydrm_suspend - Suspend tinydrm
- * @tdev: tinydrm device
- *
- * Used in driver PM operations to suspend tinydrm.
- * Suspends fbdev and DRM.
- * Resume with tinydrm_resume().
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int tinydrm_suspend(struct tinydrm_device *tdev)
-{
-   struct drm_atomic_state *state;
-
-   if (tdev->suspend_state) {
-   DRM_ERROR("Failed to suspend: state already set\n");
-   return -EINVAL;
-   }
-
-   drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 1);
-   state = drm_atomic_helper_suspend(tdev->drm);
-   if (IS_ERR(state)) {
-   drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 0);
-   return PTR_ERR(state);
-   }
-
-   tdev->suspend_state = state;
-
-   return 0;
-}
-EXPORT_SYMBOL(tinydrm_suspend);
-
-/**
- * tinydrm_resume - Resume tinydrm
- * @tdev: tinydrm device
- *
- * Used in driver PM operations to resume tinydrm.
- * Suspend with tinydrm_suspend().
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int tinydrm_resume(struct tinydrm_device *tdev)
-{
-   struct drm_atomic_state *state = tdev->suspend_state;
-   int ret;
-
-   if (!state) {
-   DRM_ERROR("Failed to resume: state is not set\n");
-   return -EINVAL;
-   }
-
-   tdev->suspend_state = NULL;
-
-   ret = drm_atomic_helper_resume(tdev->drm, state);
-   if (ret) {
-   DRM_ERROR("Error resuming state: %d\n", ret);
-   return ret;
-   }
-
-   drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 0);
-
-   return 0;
-}
-EXPORT_SYMBOL(tinydrm_resume);
-
 MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c 
b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 6a83b3093254..70ae4f76f455 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -9,6 +9,7 @@
  * (at your option) any later version.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -231,7 +232,7 @@ static int __maybe_unused mi0283qt_pm_suspend(struct device 
*dev)
struct mipi_dbi *mipi = dev_get_drvdata(dev);
int ret;
 
-   ret = tinydrm_suspend(&mipi->tinydrm);
+   ret = drm_mode_config_helper_suspend(mipi->tinydrm.drm);
if (ret)
return ret;
 
@@ -249,7 +250,9 @@ static int __maybe_unused mi0283qt_pm_resume(struct device 
*dev)
if (ret)
return ret;
 
-   return tinydrm_resume(&mipi->tinydrm);
+   drm_mode_config_helper_resume(mipi->tinydrm.drm);
+
+   return 0;
 }
 
 static const struct dev_pm_ops mi0283qt_pm_ops = {
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 4774fe3d4273..fb0d86600ea3 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -20,7 +20,6 @@
  * @pipe: Display pipe structure
  * @dirty_lock: Serializes framebuffer flushing
  * @fbdev_cma: CMA fbdev structure
- * @suspend_state: Atomic state when suspended
  * @fb_funcs: Framebuffer functions used when creating framebuffers
  */
 struct tinydrm_device {
@@ -28,7 +27,6 @@ struct tinydrm_device {
struct drm_simple_display_pipe pipe;
struct mutex dirty_lock;
struct drm_fbdev_cma *fbdev_cma;
-   struct drm_atomic_state *suspend_state;
const struct drm_framebuffer_funcs *fb_funcs;
 };
 
@@ -92,8 +90,6 @@ int devm_tinydrm_init(struct device *parent, struct 
tinydrm_device *tdev,
   

[Bug 99843] Geometry Shader - Incorrect Output

2017-11-06 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=99843

--- Comment #15 from d...@jerber.co.uk ---
I was reading about the R600 driver and came across a TODO page that suggested
testing using piglit.

I downloaded and compiled this. I ran the "shader.py" test and it gave the
following results:

skip: 26496  Pass: 6567  Warn: 2  Fail:496  Crash: 5

I'm not sure if piglit is a good measure on this but the number of fails seems
very high.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 103544] Graphical glitches r600 in game this war of mine linux native

2017-11-06 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=103544

--- Comment #11 from Roland Scheidegger  ---
(In reply to Ilia Mirkin from comment #10)
> Patch is available at https://patchwork.freedesktop.org/patch/186599/
> (although I'll have to rework the description)

Doesn't help, everything looks the same.
FWIW I (accidentally...) also tested without float rt support (got really
confused first with the results...), and there's still corruption when the game
uses ordinary (gl_rgba) rt. I'm not sure the rendering is otherwise identical,
but at least SOME corruption disappears (in particular, the black boxes),
whereas other corruption remains exactly the same (some vertical stripes
sometimes, and the fires still are black in the interior).

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 103544] Graphical glitches r600 in game this war of mine linux native

2017-11-06 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=103544

--- Comment #12 from Roland Scheidegger  ---
Here's a apitrace for this (1GB, of course the corruption is only seen towards
the end...), should be available a week (?):
https://we.tl/EanuxRG7Yf

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/amd/display: small cleanup in destruct()

2017-11-06 Thread Alex Deucher
On Mon, Nov 6, 2017 at 3:44 AM, Christian König
 wrote:
> Am 06.11.2017 um 08:07 schrieb Dan Carpenter:
>>
>> Static analysis tools get annoyed that we don't indent this if
>> statement.  Actually, the if statement isn't required because kfree()
>> can handle NULL pointers just fine.  The DCE110STRENC_FROM_STRENC()
>> macro is a wrapper around container_of() but it's basically a no-op or a
>> cast.  Anyway, it's not really appropriate here so it should be removed
>> as well.
>>
>> Signed-off-by: Dan Carpenter 
>
>
> Acked-by: Christian König 

Applied.  thanks!

Alex


>
>> ---
>> v2: in v1 I just added a tab
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> index d911590d08bc..4c4bd72d4e40 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> @@ -725,10 +725,8 @@ static void destruct(struct dcn10_resource_pool
>> *pool)
>> }
>> }
>>   - for (i = 0; i < pool->base.stream_enc_count; i++) {
>> -   if (pool->base.stream_enc[i] != NULL)
>> -   kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
>> -   }
>> +   for (i = 0; i < pool->base.stream_enc_count; i++)
>> +   kfree(pool->base.stream_enc[i]);
>> for (i = 0; i < pool->base.audio_count; i++) {
>> if (pool->base.audios[i])
>
>
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/amd/display: checking for NULL instead of IS_ERR()

2017-11-06 Thread Alex Deucher
On Mon, Nov 6, 2017 at 6:51 AM, Christian König
 wrote:
> Am 06.11.2017 um 12:43 schrieb Dan Carpenter:
>>
>> backlight_device_register() never returns NULL, it returns error
>> pointers on error so the check here is wrong.
>>
>> Signed-off-by: Dan Carpenter 
>
>
> Acked-by: Christian König 

Applied.  thanks!

Alex

>
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 33a15a1d818c..75f9383f5b9b 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -1296,7 +1296,7 @@ amdgpu_dm_register_backlight_device(struct
>> amdgpu_display_manager *dm)
>> &amdgpu_dm_backlight_ops,
>> &props);
>>   - if (NULL == dm->backlight_dev)
>> +   if (IS_ERR(dm->backlight_dev))
>> DRM_ERROR("DM: Backlight registration failed!\n");
>> else
>> DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>> bl_name);
>
>
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


  1   2   >