[PATCH 1/2] drm/dp: Add defines for latency in sink

[vathsala nagaraju]

Add defines for dpcd register 2009 (synchronization latency
in sink).

Cc: Rodrigo Vivi 
CC: Puthikorn Voravootivat 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: Vathsala Nagaraju 
---
 include/drm/drm_dp_helper.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 11c39f1..846004e6 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -735,6 +735,9 @@
 # define DP_PSR_SINK_INTERNAL_ERROR 7
 # define DP_PSR_SINK_STATE_MASK 0x07
 
+#define DP_SINK_SYNCHRONIZATION_LATENCY0x2009
+# define DP_MAX_RESYNC_FRAME_CNT_MASK  0xf
+
 #define DP_RECEIVER_ALPM_STATUS0x200b  /* eDP 1.4 */
 # define DP_ALPM_LOCK_TIMEOUT_ERROR(1 << 0)
 
-- 
1.9.1

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

[PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[vathsala nagaraju]

Set frames before SU entry value for max resync frame count of
dpcd register 2009, bit field 0:3.

v2 :
 - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
 - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
 - add check ==1 for dpcd_read call (ville)

v3 : (Rodrigo)
 - move macro EDP_PSR2_FRAME_BEFORE_SU after EDP_PSR2_FRAME_BEFORE_SU
 - replace with &=

Cc: Rodrigo Vivi 
CC: Puthikorn Voravootivat 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: Vathsala Nagaraju 
---
 drivers/gpu/drm/i915/i915_reg.h  |  2 +-
 drivers/gpu/drm/i915/intel_psr.c | 12 ++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 82f36dd..b880c84 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4047,7 +4047,7 @@ enum {
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK(0xf<<4)
 #define   EDP_PSR2_IDLE_MASK   0xf
-#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
+#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)
 
 #define EDP_PSR2_STATUS_CTL_MMIO(0x6f940)
 #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0a17d1f..adf7abc 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 */
uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
uint32_t val;
+   uint8_t sink_latency;
 
val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
@@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 * good enough. */
val |= EDP_PSR2_ENABLE |
-   EDP_SU_TRACK_ENABLE |
-   EDP_FRAMES_BEFORE_SU_ENTRY;
+   EDP_SU_TRACK_ENABLE;
+
+   if (drm_dp_dpcd_readb(_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
+   _latency) == 1) {
+   sink_latency &= DP_MAX_RESYNC_FRAME_CNT_MASK;
+   } else {
+   sink_latency = 0;
+   }
+   val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
 
if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
val |= EDP_PSR2_TP2_TIME_2500;
-- 
1.9.1

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

Re: [PATCH 1/9] drm/ttm: remove unsued options from ttm_mem_global_alloc_page

[Felix Kuehling]

Nice patch series. I wasn't expecting that so quickly.

Patches 4-7 change the ttm_page_alloc allocator, which isn't used by
amdgpu (as far as I can tell). Did you test those changes with a
different driver? I didn't review them in as much detail as the rest.
Someone with a stake in them should probably do a proper review. They
are Acked-by: Felix Kuehling 

The rest of the series is Reviewed-by: Felix Kuehling


As a possible follow up, I think  ttm_vm_bo.c would need more work to do
proper huge-page mappings for CPU access.

Regards,
  Felix


On 2017-09-22 08:13 AM, Christian König wrote:
> From: Christian König 
>
> Nobody is actually using that, remove it.
>
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/ttm/ttm_memory.c | 6 ++
>  drivers/gpu/drm/ttm/ttm_page_alloc.c | 3 +--
>  drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 3 +--
>  include/drm/ttm/ttm_memory.h | 3 +--
>  4 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c 
> b/drivers/gpu/drm/ttm/ttm_memory.c
> index 29855be..4f9978c 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -546,8 +546,7 @@ int ttm_mem_global_alloc(struct ttm_mem_global *glob, 
> uint64_t memory,
>  EXPORT_SYMBOL(ttm_mem_global_alloc);
>  
>  int ttm_mem_global_alloc_page(struct ttm_mem_global *glob,
> -   struct page *page,
> -   bool no_wait, bool interruptible)
> +   struct page *page)
>  {
>  
>   struct ttm_mem_zone *zone = NULL;
> @@ -564,8 +563,7 @@ int ttm_mem_global_alloc_page(struct ttm_mem_global *glob,
>   if (glob->zone_dma32 && page_to_pfn(page) > 0x0010UL)
>   zone = glob->zone_kernel;
>  #endif
> - return ttm_mem_global_alloc_zone(glob, zone, PAGE_SIZE, no_wait,
> -  interruptible);
> + return ttm_mem_global_alloc_zone(glob, zone, PAGE_SIZE, false, false);
>  }
>  
>  void ttm_mem_global_free_page(struct ttm_mem_global *glob, struct page *page)
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
> b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> index 052e1f1..74f465e 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> @@ -882,8 +882,7 @@ int ttm_pool_populate(struct ttm_tt *ttm)
>   return -ENOMEM;
>   }
>  
> - ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i],
> - false, false);
> + ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i]);
>   if (unlikely(ret != 0)) {
>   ttm_pool_unpopulate(ttm);
>   return -ENOMEM;
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
> b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> index 06bc14b..b8905bd 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> @@ -902,8 +902,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct 
> device *dev)
>   return -ENOMEM;
>   }
>  
> - ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i],
> - false, false);
> + ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i]);
>   if (unlikely(ret != 0)) {
>   ttm_dma_unpopulate(ttm_dma, dev);
>   return -ENOMEM;
> diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h
> index c452089..8184ff1 100644
> --- a/include/drm/ttm/ttm_memory.h
> +++ b/include/drm/ttm/ttm_memory.h
> @@ -150,8 +150,7 @@ extern int ttm_mem_global_alloc(struct ttm_mem_global 
> *glob, uint64_t memory,
>  extern void ttm_mem_global_free(struct ttm_mem_global *glob,
>   uint64_t amount);
>  extern int ttm_mem_global_alloc_page(struct ttm_mem_global *glob,
> -  struct page *page,
> -  bool no_wait, bool interruptible);
> +  struct page *page);
>  extern void ttm_mem_global_free_page(struct ttm_mem_global *glob,
>struct page *page);
>  extern size_t ttm_round_pot(size_t size);

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

Re: [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[Rodrigo Vivi]

On Fri, Sep 22, 2017 at 03:58:36PM +, vathsala nagaraju wrote:
> Set frames before SU entry value for max resync frame count of
> dpcd register 2009, bit field 0:3.
> 
> v2 :
>  - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
>  - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
>  - add check ==1 for dpcd_read call (ville)
> 
> Cc: Rodrigo Vivi 
> CC: Puthikorn Voravootivat 
> Signed-off-by: Vathsala Nagaraju 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  2 +-
>  drivers/gpu/drm/i915/intel_psr.c | 12 ++--
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 82f36dd..89c5249 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -170,6 +170,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   (mask) << 16 | (value); })
>  #define _MASKED_BIT_ENABLE(a)({ typeof(a) _a = (a); 
> _MASKED_FIELD(_a, _a); })
>  #define _MASKED_BIT_DISABLE(a)   (_MASKED_FIELD((a), 0))
> +#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)

not here

>  
>  /* Engine ID */
>  
> @@ -4047,7 +4048,6 @@ enum {
>  #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
>  #define   EDP_PSR2_FRAME_BEFORE_SU_MASK  (0xf<<4)
>  #define   EDP_PSR2_IDLE_MASK 0xf
> -#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)

move here

>  
>  #define EDP_PSR2_STATUS_CTL_MMIO(0x6f940)
>  #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 0a17d1f..e505fa6 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>*/
>   uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>   uint32_t val;
> + uint8_t sink_latency;
>  
>   val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>  
> @@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>* mesh at all with our frontbuffer tracking. And the hw alone isn't
>* good enough. */
>   val |= EDP_PSR2_ENABLE |
> - EDP_SU_TRACK_ENABLE |
> - EDP_FRAMES_BEFORE_SU_ENTRY;
> + EDP_SU_TRACK_ENABLE;
> +
> + if (drm_dp_dpcd_readb(_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
> + _latency) == 1) {
> + sink_latency = sink_latency & DP_MAX_RESYNC_FRAME_CNT_MASK;

sink_latency &= DP_MAX_RESYNC_FRAME_CNT_MASK;

with those changes

Reviewed-by: Rodrigo Vivi 


> + } else {
> + sink_latency = 0;
> + }
> + val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
>  
>   if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>   val |= EDP_PSR2_TP2_TIME_2500;
> -- 
> 1.9.1
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 99191] Weird blocky green stuff rendered in High Fidelity

[bugzilla-daemon]

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

Christoph Haag  changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |FIXED

--- Comment #6 from Christoph Haag  ---
It doesn't happen anymore on HiFi's RELEASE-7150 tag and radeonsi git, so
either of them probably fixed it in the meantime.

I'm very happy to close this again.

-- 
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 1/7] drm/rockchip/dsi: correct Feedback divider setting

[Matthias Kaehlcke]

Hi Nickey,

El Mon, Sep 18, 2017 at 05:05:33PM +0800 Nickey Yang ha dit:

> This patch correct Feedback divider setting:
> 1、Set Feedback divider [8:5] when HIGH_PROGRAM_EN
> 2、Due to the use of a "by 2 pre-scaler," the range of the
> feedback multiplication Feedback divider is limited to even
> division numbers, and Feedback divider must be greater than
> 12, less than 1000.
> 3、Make the previously configured Feedback divider(LSB)
> factors effective
> 
> Signed-off-by: Nickey Yang 
> ---
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 83 
> ++
>  1 file changed, 54 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 9a20b9d..52698b7 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -228,7 +228,7 @@
>  #define LOW_PROGRAM_EN   0
>  #define HIGH_PROGRAM_EN  BIT(7)
>  #define LOOP_DIV_LOW_SEL(val)(((val) - 1) & 0x1f)
> -#define LOOP_DIV_HIGH_SEL(val)   val) - 1) >> 5) & 0x1f)
> +#define LOOP_DIV_HIGH_SEL(val)   val) - 1) >> 5) & 0xf)
>  #define PLL_LOOP_DIV_EN  BIT(5)
>  #define PLL_INPUT_DIV_EN BIT(4)
>  
> @@ -461,6 +461,7 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>   dw_mipi_dsi_phy_write(dsi, 0x17, INPUT_DIVIDER(dsi->input_div));
>   dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_LOW_SEL(dsi->feedback_div) |
>LOW_PROGRAM_EN);
> + dw_mipi_dsi_phy_write(dsi, 0x19, PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN);
>   dw_mipi_dsi_phy_write(dsi, 0x18, LOOP_DIV_HIGH_SEL(dsi->feedback_div) |
>HIGH_PROGRAM_EN);
>   dw_mipi_dsi_phy_write(dsi, 0x19, PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN);
> @@ -521,11 +522,16 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>  static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi *dsi,
>   struct drm_display_mode *mode)
>  {
> - unsigned int i, pre;
> - unsigned long mpclk, pllref, tmp;
> - unsigned int m = 1, n = 1, target_mbps = 1000;
> + unsigned long mpclk, tmp;
> + unsigned int target_mbps = 1000;
>   unsigned int max_mbps = dptdin_map[ARRAY_SIZE(dptdin_map) - 1].max_mbps;
>   int bpp;
> + unsigned long best_freq = 0;
> + unsigned long fvco_min, fvco_max, fin ,fout;

nit: should be 'fin, fout'.

> + unsigned int min_prediv, max_prediv;
> + unsigned int _prediv, uninitialized_var(best_prediv);
> + unsigned long _fbdiv, uninitialized_var(best_fbdiv);
> + unsigned long min_delta = UINT_MAX;
>  
>   bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>   if (bpp < 0) {
> @@ -544,34 +550,53 @@ static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi 
> *dsi,
>   dev_err(dsi->dev, "DPHY clock frequency is out of 
> range\n");
>   }
>  
> - pllref = DIV_ROUND_UP(clk_get_rate(dsi->pllref_clk), USEC_PER_SEC);
> - tmp = pllref;
> -
> - /*
> -  * The limits on the PLL divisor are:
> -  *
> -  *  5MHz <= (pllref / n) <= 40MHz
> -  *
> -  * we walk over these values in descreasing order so that if we hit
> -  * an exact match for target_mbps it is more likely that "m" will be
> -  * even.
> -  *
> -  * TODO: ensure that "m" is even after this loop.
> -  */
> - for (i = pllref / 5; i > (pllref / 40); i--) {
> - pre = pllref / i;
> - if ((tmp > (target_mbps % pre)) && (target_mbps / pre < 512)) {
> - tmp = target_mbps % pre;
> - n = i;
> - m = target_mbps / pre;
> + fin = clk_get_rate(dsi->pllref_clk);
> + fout = target_mbps * USEC_PER_SEC;
> +
> + /* constraint: 5Mhz < Fref / N < 40MHz */

According to the previous comment above it should be '<=' instead of
'<'.

> + min_prediv = DIV_ROUND_UP(fin, 40 * USEC_PER_SEC);
> + max_prediv = fin / (5 * USEC_PER_SEC);
> +
> + /* constraint: 80MHz < Fvco < 1500Mhz */

Same here.

> + fvco_min = 80 * USEC_PER_SEC;
> + fvco_max = 1500 * USEC_PER_SEC;
> +
> + for (_prediv = min_prediv; _prediv <= max_prediv; _prediv++) {
> + u32 delta;
> + /* Fvco = Fref * M / N */
> + tmp = fout * _prediv;
> + do_div(tmp, fin);
> + _fbdiv = tmp;
> + /*
> +  * Due to the use of a "by 2 pre-scaler," the range of the
> +  * feedback multiplication value M is limited to even division
> +  * numbers, and m must be greater than 12, less than 1000.
> +  */

I didn't encounter the second part of the constraint in my version of
the databook, judging from the code below it seems the comment should
be "12 <= m <= 1000".

> + if (_fbdiv < 12 || _fbdiv > 1000)
> + continue;
> +

[PATCH hwc] drm_hwcomposer: move library to /vendor

[Rob Herring]

As part of Treble project in Android O, all the device specific files have
to be located in a separate vendor partition. This is done by setting
LOCAL_PROPRIETARY_MODULE (the name is misleading). This change will not
break existing platforms without a vendor partition as it will just move
files to /system/vendor.

Signed-off-by: Rob Herring 
---
 Android.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Android.mk b/Android.mk
index 2c4f7e3ee930..aa95b44c6075 100644
--- a/Android.mk
+++ b/Android.mk
@@ -94,6 +94,8 @@ LOCAL_MODULE_TAGS := optional
 LOCAL_MODULE_RELATIVE_PATH := hw
 LOCAL_MODULE_CLASS := SHARED_LIBRARIES
 LOCAL_MODULE_SUFFIX := $(TARGET_SHLIB_SUFFIX)
+LOCAL_VENDOR_MODULE := true
+
 include $(BUILD_SHARED_LIBRARY)
 
 include $(call all-makefiles-under,$(LOCAL_PATH))
-- 
2.11.0

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

[bug report] drm/pl111: Replace custom connector with panel bridge

[Dan Carpenter]

Hello Linus Walleij,

This is a semi-automatic email about new static checker warnings.

The patch 001485d5255c: "drm/pl111: Replace custom connector with
panel bridge" from Sep 8, 2017, leads to the following Smatch
complaint:

drivers/gpu/drm/pl111/pl111_drv.c:130 pl111_modeset_init()
error: we previously assumed 'panel' could be null (see line 103)

drivers/gpu/drm/pl111/pl111_drv.c
   102  return ret;
   103  if (panel) {
^
Patch adds new check

   104  bridge = drm_panel_bridge_add(panel,
   105
DRM_MODE_CONNECTOR_Unknown);
   106  if (IS_ERR(bridge)) {
   107  ret = PTR_ERR(bridge);
   108  goto out_config;
   109  }
   110  /*
   111   * TODO: when we are using a different bridge than a 
panel
   112   * (such as a dumb VGA connector) we need to devise a 
different
   113   * method to get the connector out of the bridge.
   114   */
   115  }
   116  
   117  ret = pl111_display_init(dev);
   118  if (ret != 0) {
   119  dev_err(dev->dev, "Failed to init display\n");
   120  goto out_bridge;
   121  }
   122  
   123  ret = drm_simple_display_pipe_attach_bridge(>pipe,
   124  bridge);
   125  if (ret)
   126  return ret;
   127  
   128  priv->bridge = bridge;
   129  priv->panel = panel;
   130  priv->connector = panel->connector;
  
but it also adds a new dereference.

   131  
   132  ret = drm_vblank_init(dev, 1);

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

Re: [PATCH hwc v1] drm_hwcomposer: Add CONTRIBUTING file

[Robert Foss]

Pushed!

On Fri, 2017-09-22 at 09:23 -0700, Sean Paul wrote:
> On Fri, Sep 22, 2017 at 02:37:18AM +0200, Robert Foss wrote:
> > Some basic guidelines for contributions could come in handy.
> > 
> > These are copied from IGT and modified to be suitable.
> > 
> > Signed-off-by: Robert Foss 
> > ---
> >  CONTRIBUTING | 31 +++
> >  1 file changed, 31 insertions(+)
> >  create mode 100644 CONTRIBUTING
> > 
> > diff --git a/CONTRIBUTING b/CONTRIBUTING
> > new file mode 100644
> > index 000..f1b4775
> > --- /dev/null
> > +++ b/CONTRIBUTING
> > @@ -0,0 +1,31 @@
> > +Patches to drm_hwcomposer are very much welcome, we really want
> > this to be the
> > +universal HW composer implementation for Android and similar
> > platforms
> > +So please bring on porting patches, bugfixes, improvements for
> > documentation
> > +and new features.
> > +
> > +A short list of contribution guidelines:
> > +
> > +- Please submit patches formatted with git send-email/git format-
> > patch or
> > +  equivalent to
> > +
> > +dri-devel 
> > +
> > +  Please use --subject-prefix="PATCH hwc" so that drm_hwcomposer
> > patches are easily
> > +  identified in the massive amount mails on dri-devel. To ensure
> > this is always
> > +  done, run:
> > +
> > +git config format.subjectprefix "PATCH hwc"
> > +
> > +- drm_hwcomposer is Apache 2.0 Licensed and we require
> > contributions to follow the
> > +  developer's certificate of origin: http://developercertificate.o
> > rg/
> > +
> > +- When submitting new code please follow the naming conventions
> > documented
> > +  in the generated documentation. Also please make full use of all
> > the helpers and
> > +  convenience macros provided by drm_hwcomposer. The below command
> > can help you
> > +  with formatting of your patches:
> > +  git diff | clang-format-diff-3.5 -p 1 -style=file
> > +
> > +- Harware specific changes should get tested on relevant platforms
> 
> *Hardware
> 
> With that,
> 
> Reviewed-by: Sean Paul 
> 
> > +  before committing.
> > +
> > +Happy hacking!
> > -- 
> > 2.11.0
> > 
> 
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3] drm/gem-fb-helper: Improve documentation

[Eric Anholt]

Noralf Trønnes  writes:

> Make the docs read a little better.

This all looks nice to me.

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: [PATCH][drm-next] drm/i915/gvt: ensure -ve return value is handled correctly

[Wang, Zhi A]

Thanks for the reply. Learned a lot. :)

GEM_BUG_ON is new to me since it wasn't there at the beginning of GVT-g 
upstream. It showed up later. So I left a lot of WARN_ON in the code and some 
of them should be GEM_BUG_ON now.

Now I can figure out those differences. We can discuss with our QA to see if 
they would like to enable I915_GEM_DEBUG and then we can move to GEM_BUG_ON 
also, or maybe we can have a dedicated GVT_BUG_ON. :) Thank you so much. Have a 
great weekend.

Thanks,
Zhi.

-Original Message-
From: Joonas Lahtinen [mailto:joonas.lahti...@linux.intel.com] 
Sent: Friday, September 22, 2017 2:11 PM
To: Wang, Zhi A ; Zhenyu Wang ; 
Joe Perches 
Cc: Gao, Fred ; David Airlie ; 
intel-...@lists.freedesktop.org; kernel-janit...@vger.kernel.org; 
linux-ker...@vger.kernel.org; Jani Nikula ; 
dri-devel@lists.freedesktop.org; Vivi, Rodrigo ; Colin 
King ; intel-gvt-...@lists.freedesktop.org
Subject: Re: [PATCH][drm-next] drm/i915/gvt: ensure -ve return value is handled 
correctly

On Thu, 2017-09-21 at 16:17 +, Wang, Zhi A wrote:
> Hi Joonas:
> 
> Thanks for the introduction. I have been thinking about the 
> possibility of introducing GEM_BUG_ON into GVT-g recently and 
> investigating on it. I'm just a bit confused about the usage between 
> GEM_BUG_ON and WARN_ON.

GEM_BUG_ON is basically there to catch things that we do not expect ever to 
happen within the driver. So we often list the function preconditions as 
GEM_BUG_ON. It's there for the same reason as the lockdep_assert_held and 
KASAN. It's sometimes heavy checks that we really want to run when functionally 
validating kernel.

GEM_BUG_ON became to existence because adding checks for obvious conditions at 
the critical command submission path GEM is not sustainable for performance in 
production.

The expectation is that each GEM_BUG_ON has a testcase in I-G-T that has the 
potential to hit it if driver was modified not to respect those preconditions. 
So once our testest passes, we can disable the GEM_BUG_ONs and be confident of 
the internal driver quality and get the release performance.

WARN_ON is mostly used for the cases when the hardware is behaving differently 
than we expect. We can't remove them as we don't have all the hardware in the 
world to test, but we try to exercise them too through I-G-Ts. The test will 
often be the subtest that was written to reproduce the problem with our 
expectations of hardware in case of hangs and other bugs. After we've corrected 
the driver behaviour, or got a hardware W/A assigned, we keep the test and add 
a WARN_ON to make sure there will be no regression back to the same situation.

This is at least what should happen, given time constraints, there may be 
variations.

User behaving unexpectedly should never result in WARN_ON (or even worse, 
BUG_ON), should always just be debug messages displayed (not to trigger the CI) 
and errors propagated back to user:

https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-uapi.html#recommended
-ioctl-return-values

Bare BUG_ON should only be used when there's the danger of corrupting system 
memory or filesystems, so from graphics driver, that's not very often. 
Controlled propagation of errors and maybe WARN_ON is always preferred if 
possible.


> GEM_BUG_ON is only enabled when kernel debug is enabled, which mostly 
> is disabled in a production kernel. In the case of i915, I'm sure it 
> will be enabled in CI test so that it can catch broken code path.
> Looking into GVT-g, the similar scenario is we enable it in QA test.
> 
> Let's say GEM_BUG_ON can do its work very well in QA test but QA test 
> is not fully covered all the condition, then something might be still 
> broken when it comes to the production kernel for user and GEM_BUG_ON 
> will be disabled and will not catch that, I guess.
> 
> That's my confusion which scratched my mind during the investigation:
> If GEM_BUG_ON is not always working, then it looks WARN_ON should 
> always be used Expected to learn more about the story behind. :)

So if the saying is some object is "never going to be bigger than 2G", there 
should be either:

1. GEM_BUG_ON like assertion for it and a test that tries to hit it, by trying 
to allocate a huge object for example, and should get rejection as -EINVAL

2. Test to see if the object is bigger, and propagate back the error if it is. 
Either resulting in user reported error if the origin of the object is outside 
of kernel <-> hardware. Or a WARN_ON if it's strange hardware or kernel driver 
behavior.

You should choose depending on how often your function gets called, and how 
critical the execution time is.

Hopefully this clarified things.

Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

Re: [PATCH 01/10] drm/panel: Keep track of enabled/prepared

[Eric Anholt]

Andrzej Hajda  writes:

> On 21.09.2017 19:06, Sean Paul wrote:
>> This patch adds state tracking to the drm_panel functions which keep
>> track of enabled and prepared. If the calls are unbalanced, a WARNING is
>> issued.
>>
>> The motivation for this change is that a number of panel drivers
>> (including panel-simple) all do this to protect their regulator
>> refcounts. The atomic framework ensures the calls are balanced, and
>> there  aren't any panel drivers being used by legacy drivers. As such,
>> these checks are useless, but let's add a WARNING just in case something
>> crazy happens (like a legacy driver using a panel).
>>
>> Less code == better.
>>
>> Signed-off-by: Sean Paul 
>
> I wonder if the tracking is needed at all, panels power states are
> usually the same as states of encoders they are connected to.
> But it is just remark to consider, no strong opposition :)
>
>> ---
>>  drivers/gpu/drm/drm_panel.c |  2 ++
>>  include/drm/drm_panel.h | 38 ++
>>  2 files changed, 36 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
>> index 308d442a531b..9515219d3d2c 100644
>> --- a/drivers/gpu/drm/drm_panel.c
>> +++ b/drivers/gpu/drm/drm_panel.c
>> @@ -48,6 +48,8 @@ static LIST_HEAD(panel_list);
>>  void drm_panel_init(struct drm_panel *panel)
>>  {
>>  INIT_LIST_HEAD(>list);
>> +panel->enabled = false;
>> +panel->prepared = false;
>>  }
>>  EXPORT_SYMBOL(drm_panel_init);
>>  
>> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
>> index 14ac240a1f64..b9a86a4cf29c 100644
>> --- a/include/drm/drm_panel.h
>> +++ b/include/drm/drm_panel.h
>> @@ -24,6 +24,7 @@
>>  #ifndef __DRM_PANEL_H__
>>  #define __DRM_PANEL_H__
>>  
>> +#include 
>>  #include 
>>  #include 
>>  
>> @@ -84,6 +85,8 @@ struct drm_panel_funcs {
>>   * @dev: parent device of the panel
>>   * @funcs: operations that can be performed on the panel
>>   * @list: panel entry in registry
>> + * @enabled: keeps track of the panel enabled status
>> + * @prepared: keeps track of the panel prepared status
>>   */
>>  struct drm_panel {
>>  struct drm_device *drm;
>> @@ -93,6 +96,9 @@ struct drm_panel {
>>  const struct drm_panel_funcs *funcs;
>>  
>>  struct list_head list;
>> +
>> +bool enabled;
>> +bool prepared;
>>  };
>>  
>>  /**
>> @@ -104,12 +110,18 @@ struct drm_panel {
>>   * is usually no longer possible to communicate with the panel until another
>>   * call to drm_panel_prepare().
>>   *
>> + * Atomic framework should ensure that prepare/unprepare are properly 
>> balanced.
>> + * If this is not the case, a WARNING will be issued.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  static inline int drm_panel_unprepare(struct drm_panel *panel)
>>  {
>> -if (panel && panel->funcs && panel->funcs->unprepare)
>> +if (panel && panel->funcs && panel->funcs->unprepare) {
>> +WARN_ON(!panel->prepared);
>
> WARN_ON(!panel->prepared || panel->enabled);
>
> Similar double checks should be used in other places.

I like this idea!  I feel like I've seen enough drivers that didn't
balance prepare/unprepare and enable/disable because for their one panel
they supported it didn't matter.


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

Re: [PATCH] qxl: fix framebuffer unpinning

[Gabriel Krisman Bertazi]

Gerd Hoffmann  writes:

>> I can simply disable X, lightdm or whatever is using it and then
>> rmmod.
>
> Works for me only with CONFIG_FBCON=n, otherwise the fbcon framebuffer
> keeps qxl busy.  So is most cases (typical distro kernel build with
> FBCON=y) you simply can't rmmod qxl.
>
> I agree that "rmmod qxl" should be fixed, but IMHO this is independent
> from this bugfix, and I'd like to get it out of the door ASAP.  Can I
> get your reviewed-by?

Yes, getting this out of the door asap makes total sense to me.  Please
add my reviewed-by:

Reviewed-by: Gabriel Krisman Bertazi 

Thanks,

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

Re: [hwc PATCH v1] drm_hwcomposer: reorder source layers according to zorder

[Sean Paul]

On Thu, Sep 21, 2017 at 05:02:21PM -0700, Zach Reizner wrote:
> On Thu, Sep 21, 2017 at 4:53 PM, Adrian Salido  wrote:
> > Precomp layers may be added to the back at different points which may
> > cause elements to be unsorted. Make sure that these are sorted after
> > provisioning planes to ensure right composition based on zorder.
> >
> > Signed-off-by: Adrian Salido 
> Reviewed-by: Zach Reizner 

Applied.

Sean

> > ---
> >  drmdisplaycomposition.cpp | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drmdisplaycomposition.cpp b/drmdisplaycomposition.cpp
> > index 293160bfd012..0f8084b39706 100644
> > --- a/drmdisplaycomposition.cpp
> > +++ b/drmdisplaycomposition.cpp
> > @@ -379,6 +379,9 @@ int DrmDisplayComposition::Plan(SquashState *squash,
> >  if (!i.plane())
> >continue;
> >
> > +// make sure that source layers are ordered based on zorder
> > +std::sort(i.source_layers().begin(), i.source_layers().end());
> > +
> >  std::vector *container;
> >  if (i.plane()->type() == DRM_PLANE_TYPE_PRIMARY)
> >container = primary_planes;
> > --
> > 2.14.1.821.g8fa685d3b7-goog
> >
> > ___
> > 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

-- 
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: [PATCH hwc v1] drm_hwcomposer: Add CONTRIBUTING file

[Sean Paul]

On Fri, Sep 22, 2017 at 02:37:18AM +0200, Robert Foss wrote:
> Some basic guidelines for contributions could come in handy.
> 
> These are copied from IGT and modified to be suitable.
> 
> Signed-off-by: Robert Foss 
> ---
>  CONTRIBUTING | 31 +++
>  1 file changed, 31 insertions(+)
>  create mode 100644 CONTRIBUTING
> 
> diff --git a/CONTRIBUTING b/CONTRIBUTING
> new file mode 100644
> index 000..f1b4775
> --- /dev/null
> +++ b/CONTRIBUTING
> @@ -0,0 +1,31 @@
> +Patches to drm_hwcomposer are very much welcome, we really want this to be 
> the
> +universal HW composer implementation for Android and similar platforms
> +So please bring on porting patches, bugfixes, improvements for documentation
> +and new features.
> +
> +A short list of contribution guidelines:
> +
> +- Please submit patches formatted with git send-email/git format-patch or
> +  equivalent to
> +
> +dri-devel 
> +
> +  Please use --subject-prefix="PATCH hwc" so that drm_hwcomposer patches are 
> easily
> +  identified in the massive amount mails on dri-devel. To ensure this is 
> always
> +  done, run:
> +
> +git config format.subjectprefix "PATCH hwc"
> +
> +- drm_hwcomposer is Apache 2.0 Licensed and we require contributions to 
> follow the
> +  developer's certificate of origin: http://developercertificate.org/
> +
> +- When submitting new code please follow the naming conventions documented
> +  in the generated documentation. Also please make full use of all the 
> helpers and
> +  convenience macros provided by drm_hwcomposer. The below command can help 
> you
> +  with formatting of your patches:
> +  git diff | clang-format-diff-3.5 -p 1 -style=file
> +
> +- Harware specific changes should get tested on relevant platforms

*Hardware

With that,

Reviewed-by: Sean Paul 

> +  before committing.
> +
> +Happy hacking!
> -- 
> 2.11.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: [PATCH hwc v1] drm_hwcomposer: Add CONTRIBUTING file

[Zach Reizner]

On Thu, Sep 21, 2017 at 5:37 PM, Robert Foss  wrote:
> Some basic guidelines for contributions could come in handy.
>
> These are copied from IGT and modified to be suitable.
>
> Signed-off-by: Robert Foss 
Reviewed-by: Zach Reizner 
> ---
>  CONTRIBUTING | 31 +++
>  1 file changed, 31 insertions(+)
>  create mode 100644 CONTRIBUTING
>
> diff --git a/CONTRIBUTING b/CONTRIBUTING
> new file mode 100644
> index 000..f1b4775
> --- /dev/null
> +++ b/CONTRIBUTING
> @@ -0,0 +1,31 @@
> +Patches to drm_hwcomposer are very much welcome, we really want this to be 
> the
> +universal HW composer implementation for Android and similar platforms
> +So please bring on porting patches, bugfixes, improvements for documentation
> +and new features.
> +
> +A short list of contribution guidelines:
> +
> +- Please submit patches formatted with git send-email/git format-patch or
> +  equivalent to
> +
> +dri-devel 
> +
> +  Please use --subject-prefix="PATCH hwc" so that drm_hwcomposer patches are 
> easily
> +  identified in the massive amount mails on dri-devel. To ensure this is 
> always
> +  done, run:
> +
> +git config format.subjectprefix "PATCH hwc"
> +
> +- drm_hwcomposer is Apache 2.0 Licensed and we require contributions to 
> follow the
> +  developer's certificate of origin: http://developercertificate.org/
> +
> +- When submitting new code please follow the naming conventions documented
> +  in the generated documentation. Also please make full use of all the 
> helpers and
> +  convenience macros provided by drm_hwcomposer. The below command can help 
> you
> +  with formatting of your patches:
> +  git diff | clang-format-diff-3.5 -p 1 -style=file
> +
> +- Harware specific changes should get tested on relevant platforms
> +  before committing.
> +
> +Happy hacking!
> --
> 2.11.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH][drm-next] drm/tve200: make two functions static

[Colin King]

From: Colin Ian King 

The functions tve200_display_disable and tve200_display_funcs are
local to the source and do not need to be in global scope, so make
them static.

Cleans up sparse warnings:
symbol 'tve200_display_disable' was not declared. Should it be static?
symbol 'tve200_display_funcs' was not declared. Should it be static?

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/tve200/tve200_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tve200/tve200_display.c 
b/drivers/gpu/drm/tve200/tve200_display.c
index 18457de47bbc..904e38b93530 100644
--- a/drivers/gpu/drm/tve200/tve200_display.c
+++ b/drivers/gpu/drm/tve200/tve200_display.c
@@ -221,7 +221,7 @@ static void tve200_display_enable(struct 
drm_simple_display_pipe *pipe,
drm_crtc_vblank_on(crtc);
 }
 
-void tve200_display_disable(struct drm_simple_display_pipe *pipe)
+static void tve200_display_disable(struct drm_simple_display_pipe *pipe)
 {
struct drm_crtc *crtc = >crtc;
struct drm_device *drm = crtc->dev;
@@ -293,7 +293,7 @@ static int tve200_display_prepare_fb(struct 
drm_simple_display_pipe *pipe,
return drm_fb_cma_prepare_fb(>plane, plane_state);
 }
 
-const struct drm_simple_display_pipe_funcs tve200_display_funcs = {
+static const struct drm_simple_display_pipe_funcs tve200_display_funcs = {
.check = tve200_display_check,
.enable = tve200_display_enable,
.disable = tve200_display_disable,
-- 
2.14.1

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

[PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[vathsala nagaraju]

Set frames before SU entry value for max resync frame count of
dpcd register 2009, bit field 0:3.

v2 :
 - add macro  EDP_PSR2_FRAME_BEFORE_SU (Rodrigo)
 - remove EDP_FRAMES_BEFORE_SU_ENTRY (Rodrigo)
 - add check ==1 for dpcd_read call (ville)

Cc: Rodrigo Vivi 
CC: Puthikorn Voravootivat 
Signed-off-by: Vathsala Nagaraju 
---
 drivers/gpu/drm/i915/i915_reg.h  |  2 +-
 drivers/gpu/drm/i915/intel_psr.c | 12 ++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 82f36dd..89c5249 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -170,6 +170,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
(mask) << 16 | (value); })
 #define _MASKED_BIT_ENABLE(a)  ({ typeof(a) _a = (a); _MASKED_FIELD(_a, _a); })
 #define _MASKED_BIT_DISABLE(a) (_MASKED_FIELD((a), 0))
+#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT)
 
 /* Engine ID */
 
@@ -4047,7 +4048,6 @@ enum {
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK(0xf<<4)
 #define   EDP_PSR2_IDLE_MASK   0xf
-#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
 
 #define EDP_PSR2_STATUS_CTL_MMIO(0x6f940)
 #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0a17d1f..e505fa6 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 */
uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
uint32_t val;
+   uint8_t sink_latency;
 
val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
@@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 * good enough. */
val |= EDP_PSR2_ENABLE |
-   EDP_SU_TRACK_ENABLE |
-   EDP_FRAMES_BEFORE_SU_ENTRY;
+   EDP_SU_TRACK_ENABLE;
+
+   if (drm_dp_dpcd_readb(_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
+   _latency) == 1) {
+   sink_latency = sink_latency & DP_MAX_RESYNC_FRAME_CNT_MASK;
+   } else {
+   sink_latency = 0;
+   }
+   val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
 
if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
val |= EDP_PSR2_TP2_TIME_2500;
-- 
1.9.1

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

[PATCH v3] drm/gem-fb-helper: Improve documentation

[Noralf Trønnes]

Make the docs read a little better.

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

Changes since version 2:
- Capitalized start of sentences/descriptions
- Made sure all functions have Returns:
- Added some info about how things tie together

 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 82 ++--
 1 file changed, 52 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c 
b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index fc7e995..aa8cb9b 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -27,19 +27,24 @@
  * DOC: overview
  *
  * This library provides helpers for drivers that don't subclass
- * _framebuffer and and use _gem_object for their backing storage.
+ * _framebuffer and use _gem_object for their backing storage.
  *
  * Drivers without additional needs to validate framebuffers can simply use
- * drm_gem_fb_create() and everything is wired up automatically. But all
- * parts can be used individually.
+ * drm_gem_fb_create() and everything is wired up automatically. Other drivers
+ * can use all parts independently.
  */
 
 /**
- * drm_gem_fb_get_obj() - Get GEM object for framebuffer
- * @fb: The framebuffer
- * @plane: Which plane
+ * drm_gem_fb_get_obj() - Get GEM object backing the framebuffer
+ * @fb: Framebuffer
+ * @plane: Plane index
  *
- * Returns the GEM object for given framebuffer.
+ * No additional reference is taken beyond the one that the _frambuffer
+ * already holds.
+ *
+ * Returns:
+ * Pointer to _gem_object for the given framebuffer and plane index or NULL
+ * if it does not exist.
  */
 struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
  unsigned int plane)
@@ -82,7 +87,7 @@ drm_gem_fb_alloc(struct drm_device *dev,
 
 /**
  * drm_gem_fb_destroy - Free GEM backed framebuffer
- * @fb: DRM framebuffer
+ * @fb: Framebuffer
  *
  * Frees a GEM backed framebuffer with its backing buffer(s) and the structure
  * itself. Drivers can use this as their _framebuffer_funcs->destroy
@@ -102,12 +107,13 @@ EXPORT_SYMBOL(drm_gem_fb_destroy);
 
 /**
  * drm_gem_fb_create_handle - Create handle for GEM backed framebuffer
- * @fb: DRM framebuffer
- * @file: drm file
- * @handle: handle created
+ * @fb: Framebuffer
+ * @file: DRM file to register the handle for
+ * @handle: Pointer to return the created handle
  *
+ * This function creates a handle for the GEM object backing the framebuffer.
  * Drivers can use this as their _framebuffer_funcs->create_handle
- * callback.
+ * callback. The GETFB IOCTL calls into this callback.
  *
  * Returns:
  * 0 on success or a negative error code on failure.
@@ -120,18 +126,21 @@ int drm_gem_fb_create_handle(struct drm_framebuffer *fb, 
struct drm_file *file,
 EXPORT_SYMBOL(drm_gem_fb_create_handle);
 
 /**
- * drm_gem_fb_create_with_funcs() - helper function for the
+ * drm_gem_fb_create_with_funcs() - Helper function for the
  *  _mode_config_funcs.fb_create
  *  callback
  * @dev: DRM device
- * @file: drm file for the ioctl call
- * @mode_cmd: metadata from the userspace fb creation request
+ * @file: DRM file that holds the GEM handle(s) backing the framebuffer
+ * @mode_cmd: Metadata from the userspace framebuffer creation request
  * @funcs: vtable to be used for the new framebuffer object
  *
  * This can be used to set _framebuffer_funcs for drivers that need the
  * _framebuffer_funcs.dirty callback. Use drm_gem_fb_create() if you don't
  * need to change _framebuffer_funcs.
  * The function does buffer size validation.
+ *
+ * Returns:
+ * Pointer to a _framebuffer on success or an error pointer on failure.
  */
 struct drm_framebuffer *
 drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
@@ -192,15 +201,26 @@ static const struct drm_framebuffer_funcs 
drm_gem_fb_funcs = {
 };
 
 /**
- * drm_gem_fb_create() - _mode_config_funcs.fb_create callback function
+ * drm_gem_fb_create() - Helper function for the
+ *   _mode_config_funcs.fb_create callback
  * @dev: DRM device
- * @file: drm file for the ioctl call
- * @mode_cmd: metadata from the userspace fb creation request
+ * @file: DRM file that holds the GEM handle(s) backing the framebuffer
+ * @mode_cmd: Metadata from the userspace framebuffer creation request
+ *
+ * This function creates a new framebuffer object described by
+ * _mode_fb_cmd2. This description includes handles for the buffer(s)
+ * backing the framebuffer.
  *
  * If your hardware has special alignment or pitch requirements these should be
  * checked before calling this function. The function does buffer size
  * validation. Use drm_gem_fb_create_with_funcs() if you need to set
  * _framebuffer_funcs.dirty.
+ *
+ * Drivers can use 

Re: drm_hwcomposer moving to fd.o

[Robert Foss]

Hey Chih-Wei,

On Fri, 2017-09-22 at 10:40 +0800, Chih-Wei Huang wrote:
> Great news!
> Thanks a lot to make it happen.
> 
> I hope I am wrong.
> From my understanding most x86 GPUs still
> cannot work with drm_hwcomposer since
> they lack of atomic mode-setting API support
> required by drm_hwcomposer.
> Hope this could be addressed soon since
> drm_hwcomposer becomes a standard project of fd.o.

I haven't personally run drm_hwc ontop of any x86 hardware, but there
are people who do it.
The android-ia project has supported using drm_hwcomposer and an
alternative hwcomposer, so it would think there should be no issues.

> 
> How about the gralloc?
> There are also several implementations
> scattered somewhere.
> I think it's also important to standardize it to fd.o
> and make it work with fd.o's drm_hwcomposer.

Gralloc is a different story, and the right person to ask about it is
not me, but maybe Rob Herring.


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

Re: drm_hwcomposer moving to fd.o

[Robert Foss]

Hey Jani,

On Fri, 2017-09-22 at 14:21 +0300, Jani Nikula wrote:
> On Fri, 22 Sep 2017, Robert Foss  wrote:
> > After talking to Liviu Dudau at LPC and Sean Paul & Kaveh Nasri at
> > XDC
> > it seems that we all could benefit from community maintainership of
> > drm_hwcomposer.
> 
> Sounds good!
> 
> > Regarding contributions, they are accepted on the dri-devel list,
> > using
> > the patch prefix "hwc".
> 
> dri-devel averages around 2400-2500 messages per month AFAICT. If hwc
> community development picks up any kind of momentum, please consider
> starting a new list. Or would a joint userspace list with e.g. libdrm
> be
> beneficial? Daniel Stone will help you here.

A separate ML if there ends up being a significant volume of traffic
sounds like a good idea.

I guess we're following the IGT model here, and will move if/when
sustained development is occurring.

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

[PATCH v2] drm/tinydrm: Move backlight helpers to a separate file

[Meghana Madhyastha]

Move backlight helpers from tinydrm-helpers.c to
tinydrm-backlight.c. This is because it is organizationally
simpler to understand and advantageous to group functions
performing a similar function to a separate file as opposed to
having one helper file with heteregenous helper functions.

Signed-off-by: Meghana Madhyastha 
---
Changes in v2:
 -Improved commit message by explaining why the changes were made.

 drivers/gpu/drm/tinydrm/core/Makefile|   2 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c | 103 +++
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c   |  94 -
 drivers/gpu/drm/tinydrm/mi0283qt.c   |   1 +
 drivers/gpu/drm/tinydrm/mipi-dbi.c   |   1 +
 include/drm/tinydrm/tinydrm-backlight.h  |  18 
 6 files changed, 124 insertions(+), 95 deletions(-)
 create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c
 create mode 100644 include/drm/tinydrm/tinydrm-backlight.h

diff --git a/drivers/gpu/drm/tinydrm/core/Makefile 
b/drivers/gpu/drm/tinydrm/core/Makefile
index fb221e6..389ca7a 100644
--- a/drivers/gpu/drm/tinydrm/core/Makefile
+++ b/drivers/gpu/drm/tinydrm/core/Makefile
@@ -1,3 +1,3 @@
-tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-helpers.o
+tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-backlight.o 
tinydrm-helpers.o
 
 obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c 
b/drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c
new file mode 100644
index 000..dc6f17d
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c
@@ -0,0 +1,103 @@
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+/**
+ * tinydrm_of_find_backlight - Find backlight device in device-tree
+ * @dev: Device
+ *
+ * This function looks for a DT node pointed to by a property named 'backlight'
+ * and uses of_find_backlight_by_node() to get the backlight device.
+ * Additionally if the brightness property is zero, it is set to
+ * max_brightness.
+ *
+ * Returns:
+ * NULL if there's no backlight property.
+ * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
+ * is found.
+ * If the backlight device is found, a pointer to the structure is returned.
+ */
+
+struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
+{
+   struct backlight_device *backlight;
+   struct device_node *np;
+
+   np = of_parse_phandle(dev->of_node, "backlight", 0);
+   if (!np)
+   return NULL;
+
+   backlight = of_find_backlight_by_node(np);
+   of_node_put(np);
+
+   if (!backlight)
+   return ERR_PTR(-EPROBE_DEFER);
+
+   if (!backlight->props.brightness) {
+   backlight->props.brightness = backlight->props.max_brightness;
+   DRM_DEBUG_KMS("Backlight brightness set to %d\n",
+ backlight->props.brightness);
+   }
+
+   return backlight;
+}
+EXPORT_SYMBOL(tinydrm_of_find_backlight);
+
+/**
+ * tinydrm_enable_backlight - Enable backlight helper
+ * @backlight: Backlight device
+ *
+ * Returns:
+ * Zero on success, negative error code on failure.
+ */
+int tinydrm_enable_backlight(struct backlight_device *backlight)
+{
+   unsigned int old_state;
+   int ret;
+
+   if (!backlight)
+   return 0;
+
+   old_state = backlight->props.state;
+   backlight->props.state &= ~BL_CORE_FBBLANK;
+   DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
+ backlight->props.state);
+
+   ret = backlight_update_status(backlight);
+   if (ret)
+   DRM_ERROR("Failed to enable backlight %d\n", ret);
+
+   return ret;
+}
+EXPORT_SYMBOL(tinydrm_enable_backlight);
+
+/**
+ * tinydrm_disable_backlight - Disable backlight helper
+ * @backlight: Backlight device
+ *
+ * Returns:
+ * Zero on success, negative error code on failure.
+ */
+int tinydrm_disable_backlight(struct backlight_device *backlight)
+{
+   unsigned int old_state;
+   int ret;
+
+   if (!backlight)
+   return 0;
+
+   old_state = backlight->props.state;
+   backlight->props.state |= BL_CORE_FBBLANK;
+   DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
+ backlight->props.state);
+   ret = backlight_update_status(backlight);
+   if (ret)
+   DRM_ERROR("Failed to disable backlight %d\n", ret);
+
+   return ret;
+}
+EXPORT_SYMBOL(tinydrm_disable_backlight);
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c 
b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index bd6cce0..ee8ad8c 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -236,100 +236,6 @@ void tinydrm_xrgb_to_gray8(u8 *dst, void *vaddr, 
struct drm_framebuffer *fb,
 }
 EXPORT_SYMBOL(tinydrm_xrgb_to_gray8);
 
-/**
- * 

[PATCH] drm/amdgpu: Replace ref/unref with get/put

[Meghana Madhyastha]

Replace functions with reference/unreference suffix with
get/put suffix. Get/put is shorter and more consistent with
the kernel function suffixes.

Signed-off-by: Meghana Madhyastha 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c|  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c   | 12 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c|  6 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  6 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 26 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h|  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c  |  6 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c   | 10 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c   | 10 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 12 ++--
 17 files changed, 63 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 5432af3..4d04194 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -223,7 +223,7 @@ int alloc_gtt_mem(struct kgd_dev *kgd, size_t size,
 allocate_mem_pin_bo_failed:
amdgpu_bo_unreserve((*mem)->bo);
 allocate_mem_reserve_bo_failed:
-   amdgpu_bo_unref(&(*mem)->bo);
+   amdgpu_bo_put(&(*mem)->bo);
 
return r;
 }
@@ -238,7 +238,7 @@ void free_gtt_mem(struct kgd_dev *kgd, void *mem_obj)
amdgpu_bo_kunmap(mem->bo);
amdgpu_bo_unpin(mem->bo);
amdgpu_bo_unreserve(mem->bo);
-   amdgpu_bo_unref(&(mem->bo));
+   amdgpu_bo_put(&(mem->bo));
kfree(mem);
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
index 63ec1e1..a218d340 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -128,7 +128,7 @@ static void amdgpu_benchmark_move(struct amdgpu_device 
*adev, unsigned size,
amdgpu_bo_unpin(sobj);
amdgpu_bo_unreserve(sobj);
}
-   amdgpu_bo_unref();
+   amdgpu_bo_put();
}
if (dobj) {
r = amdgpu_bo_reserve(dobj, true);
@@ -136,7 +136,7 @@ static void amdgpu_benchmark_move(struct amdgpu_device 
*adev, unsigned size,
amdgpu_bo_unpin(dobj);
amdgpu_bo_unreserve(dobj);
}
-   amdgpu_bo_unref();
+   amdgpu_bo_put();
}
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 59089e0..6a6c8c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -48,7 +48,7 @@ static void amdgpu_bo_list_release_rcu(struct kref *ref)
   refcount);
 
for (i = 0; i < list->num_entries; ++i)
-   amdgpu_bo_unref(>array[i].robj);
+   amdgpu_bo_put(>array[i].robj);
 
mutex_destroy(>lock);
kvfree(list->array);
@@ -135,13 +135,13 @@ static int amdgpu_bo_list_set(struct amdgpu_device *adev,
goto error_free;
}
 
-   bo = amdgpu_bo_ref(gem_to_amdgpu_bo(gobj));
+   bo = amdgpu_bo_get(gem_to_amdgpu_bo(gobj));
drm_gem_object_put_unlocked(gobj);
 
usermm = amdgpu_ttm_tt_get_usermm(bo->tbo.ttm);
if (usermm) {
if (usermm != current->mm) {
-   amdgpu_bo_unref();
+   amdgpu_bo_put();
r = -EPERM;
goto error_free;
}
@@ -168,7 +168,7 @@ static int amdgpu_bo_list_set(struct amdgpu_device *adev,
}
 
for (i = 0; i < list->num_entries; ++i)
-   amdgpu_bo_unref(>array[i].robj);
+   amdgpu_bo_put(>array[i].robj);
 
kvfree(list->array);
 
@@ -184,7 +184,7 @@ static int amdgpu_bo_list_set(struct amdgpu_device *adev,
 
 error_free:
while (i--)
-   amdgpu_bo_unref([i].robj);
+   amdgpu_bo_put([i].robj);
kvfree(array);
return r;
 }
@@ -254,7 +254,7 @@ void amdgpu_bo_list_free(struct amdgpu_bo_list *list)
unsigned i;
 
for (i = 0; i < list->num_entries; ++i)
-   amdgpu_bo_unref(>array[i].robj);
+   amdgpu_bo_put(>array[i].robj);
 

Re: [PATCH v2 1/6] gpu: host1x: Enable Tegra186 syncpoint protection

[Mikko Perttunen]

On 09/05/2017 04:33 PM, Dmitry Osipenko wrote:

On 05.09.2017 11:10, Mikko Perttunen wrote:
... >> diff --git a/drivers/gpu/host1x/hw/channel_hw.c 

b/drivers/gpu/host1x/hw/channel_hw.c

index 8447a56c41ca..0161da331702 100644
--- a/drivers/gpu/host1x/hw/channel_hw.c
+++ b/drivers/gpu/host1x/hw/channel_hw.c
@@ -147,6 +147,9 @@ static int channel_submit(struct host1x_job *job)
  
  	syncval = host1x_syncpt_incr_max(sp, user_syncpt_incrs);
  
+	/* assign syncpoint to channel */

+   host1x_hw_syncpt_assign_channel(host, sp, ch);


This function could be renamed to host1x_hw_assign_syncpt_to_channel() and then
comment to it won't be needed.


Maybe host1x_hw_syncpt_assign_to_channel? I'd like to keep the current 
noun_verb format. Though IMHO even the current name is pretty 
descriptive in itself.




It is not very nice that channel would be re-assigned on each submit. Maybe that
assignment should be done by host1x_syncpt_request() ?


host1x_syncpt_request doesn't know about the channel so we'd have to 
thread this information there and through each client driver in 
drm/tegra/, so I decided not to do this at this time. I'm planning a new 
channel allocation model which will change that side of the code anyway, 
so I'd like to revisit this at that point. For our current channel 
model, the current implementation doesn't have any functional downsides 
anyway.





+
job->syncpt_end = syncval;
  
  	/* add a setclass for modules that require it */

diff --git a/drivers/gpu/host1x/hw/syncpt_hw.c 
b/drivers/gpu/host1x/hw/syncpt_hw.c
index 7b0270d60742..dc7a44614fef 100644
--- a/drivers/gpu/host1x/hw/syncpt_hw.c
+++ b/drivers/gpu/host1x/hw/syncpt_hw.c
@@ -106,6 +106,50 @@ static int syncpt_patch_wait(struct host1x_syncpt *sp, 
void *patch_addr)
return 0;
  }
  
+/**

+ * syncpt_assign_channel() - Assign syncpoint to channel
+ * @sp: syncpoint
+ * @ch: channel
+ *
+ * On chips with the syncpoint protection feature (Tegra186+), assign @sp to
+ * @ch, preventing other channels from incrementing the syncpoints. If @ch is
+ * NULL, unassigns the syncpoint.
+ *
+ * On older chips, do nothing.
+ */
+static void syncpt_assign_channel(struct host1x_syncpt *sp,
+ struct host1x_channel *ch)
+{
+#if HOST1X_HW >= 6
+   struct host1x *host = sp->host;
+
+   if (!host->hv_regs)
+   return;
+
+   host1x_sync_writel(host,
+  HOST1X_SYNC_SYNCPT_CH_APP_CH(ch ? ch->id : 0xff),
+  HOST1X_SYNC_SYNCPT_CH_APP(sp->id));
+#endif
+}
+
+/**
+ * syncpt_enable_protection() - Enable syncpoint protection
+ * @host: host1x instance
+ *
+ * On chips with the syncpoint protection feature (Tegra186+), enable this
+ * feature. On older chips, do nothing.
+ */
+static void syncpt_enable_protection(struct host1x *host)
+{
+#if HOST1X_HW >= 6
+   if (!host->hv_regs)
+   return;
+
+   host1x_hypervisor_writel(host, HOST1X_HV_SYNCPT_PROT_EN_CH_EN,
+HOST1X_HV_SYNCPT_PROT_EN);
+#endif
+}
+
  static const struct host1x_syncpt_ops host1x_syncpt_ops = {
.restore = syncpt_restore,
.restore_wait_base = syncpt_restore_wait_base,
@@ -113,4 +157,6 @@ static const struct host1x_syncpt_ops host1x_syncpt_ops = {
.load = syncpt_load,
.cpu_incr = syncpt_cpu_incr,
.patch_wait = syncpt_patch_wait,
+   .assign_channel = syncpt_assign_channel,
+   .enable_protection = syncpt_enable_protection,
  };
diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c
index 048ac9e344ce..4c7a4c8b2ad2 100644
--- a/drivers/gpu/host1x/syncpt.c
+++ b/drivers/gpu/host1x/syncpt.c
@@ -398,6 +398,13 @@ int host1x_syncpt_init(struct host1x *host)
for (i = 0; i < host->info->nb_pts; i++) {
syncpt[i].id = i;
syncpt[i].host = host;
+
+   /*
+* Unassign syncpt from channels for purposes of Tegra186
+* syncpoint protection. This prevents any channel from
+* accessing it until it is reassigned.
+*/
+   host1x_hw_syncpt_assign_channel(host, [i], NULL);
}
  
  	for (i = 0; i < host->info->nb_bases; i++)

@@ -408,6 +415,7 @@ int host1x_syncpt_init(struct host1x *host)
host->bases = bases;
  
  	host1x_syncpt_restore(host);

+   host1x_hw_syncpt_enable_protection(host);
  
  	/* Allocate sync point to use for clearing waits for expired fences */

host->nop_sp = host1x_syncpt_alloc(host, NULL, 0);





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

[Bug 99553] Tracker bug for runnning OpenCL applications on Clover

[bugzilla-daemon]

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

--- Comment #17 from darkbasic  ---
Hi, any news on the OpenCL image support? I really miss darktable support.

-- 
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

[PATCH 9/9] drm/amdgpu: add VM support for huge pages

[Christian König]

From: Christian König 

Convert GTT mappings into linear ones for huge page handling.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 24 +---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3c86381..16454bd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1732,6 +1732,7 @@ static int amdgpu_vm_bo_split_mapping(struct 
amdgpu_device *adev,
}
 
do {
+   dma_addr_t *dma_addr = NULL;
uint64_t max_entries;
uint64_t addr, last;
 
@@ -1745,15 +1746,32 @@ static int amdgpu_vm_bo_split_mapping(struct 
amdgpu_device *adev,
}
 
if (pages_addr) {
+   uint64_t count;
+
max_entries = min(max_entries, 16ull * 1024ull);
-   addr = 0;
+   for (count = 1; count < max_entries; ++count) {
+   uint64_t idx = pfn + count;
+
+   if (pages_addr[idx] !=
+   (pages_addr[idx - 1] + PAGE_SIZE))
+   break;
+   }
+
+   if (count < 64) {
+   addr = pfn << PAGE_SHIFT;
+   dma_addr = pages_addr;
+   } else {
+   addr = pages_addr[pfn];
+   max_entries = count;
+   }
+
} else if (flags & AMDGPU_PTE_VALID) {
addr += adev->vm_manager.vram_base_offset;
+   addr += pfn << PAGE_SHIFT;
}
-   addr += pfn << PAGE_SHIFT;
 
last = min((uint64_t)mapping->last, start + max_entries - 1);
-   r = amdgpu_vm_bo_update_mapping(adev, exclusive, pages_addr, vm,
+   r = amdgpu_vm_bo_update_mapping(adev, exclusive, dma_addr, vm,
start, last, flags, addr,
fence);
if (r)
-- 
2.7.4

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

[PATCH 4/9] drm/ttm: allocate/free multiple pages in a single call

[Christian König]

From: Christian König 

Totally surprisingly this is more efficient than doing it page by page.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 29 ++---
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index e11fd76..482dd9a 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -873,15 +873,14 @@ int ttm_pool_populate(struct ttm_tt *ttm)
if (ttm->state != tt_unpopulated)
return 0;
 
-   for (i = 0; i < ttm->num_pages; ++i) {
-   ret = ttm_get_pages(>pages[i], 1,
-   ttm->page_flags,
-   ttm->caching_state);
-   if (ret != 0) {
-   ttm_pool_unpopulate(ttm);
-   return -ENOMEM;
-   }
+   ret = ttm_get_pages(ttm->pages, ttm->num_pages, ttm->page_flags,
+   ttm->caching_state);
+   if (unlikely(ret != 0)) {
+   ttm_pool_unpopulate(ttm);
+   return ret;
+   }
 
+   for (i = 0; i < ttm->num_pages; ++i) {
ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i],
PAGE_SIZE);
if (unlikely(ret != 0)) {
@@ -908,14 +907,14 @@ void ttm_pool_unpopulate(struct ttm_tt *ttm)
unsigned i;
 
for (i = 0; i < ttm->num_pages; ++i) {
-   if (ttm->pages[i]) {
-   ttm_mem_global_free_page(ttm->glob->mem_glob,
-ttm->pages[i], PAGE_SIZE);
-   ttm_put_pages(>pages[i], 1,
- ttm->page_flags,
- ttm->caching_state);
-   }
+   if (!ttm->pages[i])
+   continue;
+
+   ttm_mem_global_free_page(ttm->glob->mem_glob, ttm->pages[i],
+PAGE_SIZE);
}
+   ttm_put_pages(ttm->pages, ttm->num_pages, ttm->page_flags,
+ ttm->caching_state);
ttm->state = tt_unpopulated;
 }
 EXPORT_SYMBOL(ttm_pool_unpopulate);
-- 
2.7.4

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

[PATCH 8/9] drm/amdgpu: minor coding style fix

[Christian König]

From: Christian König 

Fix two minor 80 char issues.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8fcc743..3c86381 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2541,7 +2541,8 @@ static uint32_t amdgpu_vm_get_block_size(uint64_t vm_size)
  * @adev: amdgpu_device pointer
  * @fragment_size_default: the default fragment size if it's set auto
  */
-void amdgpu_vm_set_fragment_size(struct amdgpu_device *adev, uint32_t 
fragment_size_default)
+void amdgpu_vm_set_fragment_size(struct amdgpu_device *adev,
+uint32_t fragment_size_default)
 {
if (amdgpu_vm_fragment_size == -1)
adev->vm_manager.fragment_size = fragment_size_default;
@@ -2555,7 +2556,8 @@ void amdgpu_vm_set_fragment_size(struct amdgpu_device 
*adev, uint32_t fragment_s
  * @adev: amdgpu_device pointer
  * @vm_size: the default vm size if it's set auto
  */
-void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint64_t vm_size, 
uint32_t fragment_size_default)
+void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint64_t vm_size,
+  uint32_t fragment_size_default)
 {
/* adjust vm size firstly */
if (amdgpu_vm_size == -1)
-- 
2.7.4

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

[PATCH 7/9] drm/ttm: move more logic into ttm_page_pool_get_pages

[Christian König]

From: Christian König 

Make it easier to add huge page pool.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 98 +++-
 1 file changed, 52 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 3dd2a8a..7044ffa 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -627,19 +627,20 @@ static void ttm_page_pool_fill_locked(struct 
ttm_page_pool *pool,
 }
 
 /**
- * Cut 'count' number of pages from the pool and put them on the return list.
+ * Allocate pages from the pool and put them on the return list.
  *
- * @return count of pages still required to fulfill the request.
+ * @return zero for success or negative error code.
  */
-static unsigned ttm_page_pool_get_pages(struct ttm_page_pool *pool,
-   struct list_head *pages,
-   int ttm_flags,
-   enum ttm_caching_state cstate,
-   unsigned count)
+static int ttm_page_pool_get_pages(struct ttm_page_pool *pool,
+  struct list_head *pages,
+  int ttm_flags,
+  enum ttm_caching_state cstate,
+  unsigned count)
 {
unsigned long irq_flags;
struct list_head *p;
unsigned i;
+   int r = 0;
 
spin_lock_irqsave(>lock, irq_flags);
ttm_page_pool_fill_locked(pool, ttm_flags, cstate, count, _flags);
@@ -672,7 +673,35 @@ static unsigned ttm_page_pool_get_pages(struct 
ttm_page_pool *pool,
count = 0;
 out:
spin_unlock_irqrestore(>lock, irq_flags);
-   return count;
+
+   /* clear the pages coming from the pool if requested */
+   if (ttm_flags & TTM_PAGE_FLAG_ZERO_ALLOC) {
+   struct page *page;
+
+   list_for_each_entry(page, pages, lru) {
+   if (PageHighMem(page))
+   clear_highpage(page);
+   else
+   clear_page(page_address(page));
+   }
+   }
+
+   /* If pool didn't have enough pages allocate new one. */
+   if (count) {
+   gfp_t gfp_flags = pool->gfp_flags;
+
+   /* set zero flag for page allocation if required */
+   if (ttm_flags & TTM_PAGE_FLAG_ZERO_ALLOC)
+   gfp_flags |= __GFP_ZERO;
+
+   /* ttm_alloc_new_pages doesn't reference pool so we can run
+* multiple requests in parallel.
+**/
+   r = ttm_alloc_new_pages(pages, gfp_flags, ttm_flags, cstate,
+   count);
+   }
+
+   return r;
 }
 
 /* Put all pages in pages list to correct pool to wait for reuse */
@@ -742,18 +771,18 @@ static int ttm_get_pages(struct page **pages, unsigned 
npages, int flags,
struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
struct list_head plist;
struct page *p = NULL;
-   gfp_t gfp_flags = GFP_USER;
unsigned count;
int r;
 
-   /* set zero flag for page allocation if required */
-   if (flags & TTM_PAGE_FLAG_ZERO_ALLOC)
-   gfp_flags |= __GFP_ZERO;
-
/* No pool for cached pages */
if (pool == NULL) {
+   gfp_t gfp_flags = GFP_USER;
unsigned i, j;
 
+   /* set zero flag for page allocation if required */
+   if (flags & TTM_PAGE_FLAG_ZERO_ALLOC)
+   gfp_flags |= __GFP_ZERO;
+
if (flags & TTM_PAGE_FLAG_DMA32)
gfp_flags |= GFP_DMA32;
else
@@ -791,44 +820,21 @@ static int ttm_get_pages(struct page **pages, unsigned 
npages, int flags,
return 0;
}
 
-   /* combine zero flag to pool flags */
-   gfp_flags |= pool->gfp_flags;
-
/* First we take pages from the pool */
INIT_LIST_HEAD();
-   npages = ttm_page_pool_get_pages(pool, , flags, cstate, npages);
+   r = ttm_page_pool_get_pages(pool, , flags, cstate, npages);
+
count = 0;
-   list_for_each_entry(p, , lru) {
+   list_for_each_entry(p, , lru)
pages[count++] = p;
-   }
-
-   /* clear the pages coming from the pool if requested */
-   if (flags & TTM_PAGE_FLAG_ZERO_ALLOC) {
-   list_for_each_entry(p, , lru) {
-   if (PageHighMem(p))
-   clear_highpage(p);
-   else
-   clear_page(page_address(p));
-   }
-   }
 
-   /* If pool didn't have enough pages allocate new one. */
-   if (npages > 0) {
-   /* ttm_alloc_new_pages doesn't 

[PATCH 5/9] drm/ttm: DMA map/unmap compound pages as a whole

[Christian König]

From: Christian König 

Instead of mapping them bit by bit map/unmap the whole
compound pages as in one call.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 31 +++
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 482dd9a..3309926 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -922,16 +922,18 @@ EXPORT_SYMBOL(ttm_pool_unpopulate);
 #if defined(CONFIG_SWIOTLB) || defined(CONFIG_INTEL_IOMMU)
 int ttm_populate_and_map_pages(struct device *dev, struct ttm_dma_tt *tt)
 {
-   unsigned i;
+   unsigned i, j;
int r;
 
r = ttm_pool_populate(>ttm);
if (r)
return r;
 
-   for (i = 0; i < tt->ttm.num_pages; i++) {
+   for (i = 0; i < tt->ttm.num_pages; ++i) {
+   size_t num_pages = 1 << compound_order(tt->ttm.pages[i]);
+
tt->dma_address[i] = dma_map_page(dev, tt->ttm.pages[i],
- 0, PAGE_SIZE,
+ 0, num_pages * PAGE_SIZE,
  DMA_BIDIRECTIONAL);
if (dma_mapping_error(dev, tt->dma_address[i])) {
while (i--) {
@@ -942,6 +944,11 @@ int ttm_populate_and_map_pages(struct device *dev, struct 
ttm_dma_tt *tt)
ttm_pool_unpopulate(>ttm);
return -EFAULT;
}
+
+   for (j = 1; j < num_pages; ++j) {
+   tt->dma_address[i + 1] = tt->dma_address[i] + PAGE_SIZE;
+   ++i;
+   }
}
return 0;
 }
@@ -950,12 +957,20 @@ EXPORT_SYMBOL(ttm_populate_and_map_pages);
 void ttm_unmap_and_unpopulate_pages(struct device *dev, struct ttm_dma_tt *tt)
 {
unsigned i;
-   
-   for (i = 0; i < tt->ttm.num_pages; i++) {
-   if (tt->dma_address[i]) {
-   dma_unmap_page(dev, tt->dma_address[i],
-  PAGE_SIZE, DMA_BIDIRECTIONAL);
+
+   for (i = 0; i < tt->ttm.num_pages;) {
+   size_t num_pages;
+
+   if (!tt->dma_address[i] || !tt->ttm.pages[i]) {
+   ++i;
+   continue;
}
+
+   num_pages = 1 << compound_order(tt->ttm.pages[i]);
+   dma_unmap_page(dev, tt->dma_address[i], num_pages * PAGE_SIZE,
+  DMA_BIDIRECTIONAL);
+
+   i += num_pages;
}
ttm_pool_unpopulate(>ttm);
 }
-- 
2.7.4

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

[PATCH 2/9] drm/ttm: add support for different pool sizes

[Christian König]

From: Christian König 

Correctly handle different page sizes in the memory accounting.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_memory.c | 10 +-
 drivers/gpu/drm/ttm/ttm_page_alloc.c |  5 +++--
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c |  7 ---
 include/drm/ttm/ttm_memory.h |  4 ++--
 4 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 4f9978c..e963749 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -546,7 +546,7 @@ int ttm_mem_global_alloc(struct ttm_mem_global *glob, 
uint64_t memory,
 EXPORT_SYMBOL(ttm_mem_global_alloc);
 
 int ttm_mem_global_alloc_page(struct ttm_mem_global *glob,
- struct page *page)
+ struct page *page, uint64_t size)
 {
 
struct ttm_mem_zone *zone = NULL;
@@ -563,10 +563,11 @@ int ttm_mem_global_alloc_page(struct ttm_mem_global *glob,
if (glob->zone_dma32 && page_to_pfn(page) > 0x0010UL)
zone = glob->zone_kernel;
 #endif
-   return ttm_mem_global_alloc_zone(glob, zone, PAGE_SIZE, false, false);
+   return ttm_mem_global_alloc_zone(glob, zone, size, false, false);
 }
 
-void ttm_mem_global_free_page(struct ttm_mem_global *glob, struct page *page)
+void ttm_mem_global_free_page(struct ttm_mem_global *glob, struct page *page,
+ uint64_t size)
 {
struct ttm_mem_zone *zone = NULL;
 
@@ -577,10 +578,9 @@ void ttm_mem_global_free_page(struct ttm_mem_global *glob, 
struct page *page)
if (glob->zone_dma32 && page_to_pfn(page) > 0x0010UL)
zone = glob->zone_kernel;
 #endif
-   ttm_mem_global_free_zone(glob, zone, PAGE_SIZE);
+   ttm_mem_global_free_zone(glob, zone, size);
 }
 
-
 size_t ttm_round_pot(size_t size)
 {
if ((size & (size - 1)) == 0)
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 74f465e..e11fd76 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -882,7 +882,8 @@ int ttm_pool_populate(struct ttm_tt *ttm)
return -ENOMEM;
}
 
-   ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i]);
+   ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i],
+   PAGE_SIZE);
if (unlikely(ret != 0)) {
ttm_pool_unpopulate(ttm);
return -ENOMEM;
@@ -909,7 +910,7 @@ void ttm_pool_unpopulate(struct ttm_tt *ttm)
for (i = 0; i < ttm->num_pages; ++i) {
if (ttm->pages[i]) {
ttm_mem_global_free_page(ttm->glob->mem_glob,
-ttm->pages[i]);
+ttm->pages[i], PAGE_SIZE);
ttm_put_pages(>pages[i], 1,
  ttm->page_flags,
  ttm->caching_state);
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index b8905bd..5362657 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -902,7 +902,8 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct 
device *dev)
return -ENOMEM;
}
 
-   ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i]);
+   ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i],
+   pool->size);
if (unlikely(ret != 0)) {
ttm_dma_unpopulate(ttm_dma, dev);
return -ENOMEM;
@@ -967,13 +968,13 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, 
struct device *dev)
if (is_cached) {
list_for_each_entry_safe(d_page, next, _dma->pages_list, 
page_list) {
ttm_mem_global_free_page(ttm->glob->mem_glob,
-d_page->p);
+d_page->p, pool->size);
ttm_dma_page_put(pool, d_page);
}
} else {
for (i = 0; i < count; i++) {
ttm_mem_global_free_page(ttm->glob->mem_glob,
-ttm->pages[i]);
+ttm->pages[i], pool->size);
}
}
 
diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h
index 8184ff1..2c1e359 100644
--- a/include/drm/ttm/ttm_memory.h
+++ b/include/drm/ttm/ttm_memory.h
@@ -150,9 +150,9 @@ extern int ttm_mem_global_alloc(struct ttm_mem_global 
*glob, uint64_t memory,
 extern void 

[PATCH 6/9] drm/ttm: add transparent huge page support for cached allocations

[Christian König]

From: Christian König 

Try to allocate huge pages when it makes sense.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 49 ++--
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 3309926..3dd2a8a 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -685,12 +685,24 @@ static void ttm_put_pages(struct page **pages, unsigned 
npages, int flags,
 
if (pool == NULL) {
/* No pool for this memory type so free the pages */
-   for (i = 0; i < npages; i++) {
-   if (pages[i]) {
-   if (page_count(pages[i]) != 1)
-   pr_err("Erroneous page count. Leaking 
pages.\n");
-   __free_page(pages[i]);
-   pages[i] = NULL;
+   i = 0;
+   while (i < npages) {
+   unsigned order;
+
+   if (!pages[i]) {
+   ++i;
+   continue;
+   }
+
+   if (page_count(pages[i]) != 1)
+   pr_err("Erroneous page count. Leaking 
pages.\n");
+   order = compound_order(pages[i]);
+   __free_pages(pages[i], order);
+
+   order = 1 << order;
+   while (order) {
+   pages[i++] = NULL;
+   --order;
}
}
return;
@@ -740,12 +752,32 @@ static int ttm_get_pages(struct page **pages, unsigned 
npages, int flags,
 
/* No pool for cached pages */
if (pool == NULL) {
+   unsigned i, j;
+
if (flags & TTM_PAGE_FLAG_DMA32)
gfp_flags |= GFP_DMA32;
else
gfp_flags |= GFP_HIGHUSER;
 
-   for (r = 0; r < npages; ++r) {
+   i = 0;
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+   while (npages >= HPAGE_PMD_NR) {
+   gfp_t huge_flags = gfp_flags;
+
+   huge_flags |= GFP_TRANSHUGE;
+   huge_flags &= ~__GFP_MOVABLE;
+   p = alloc_pages(huge_flags, HPAGE_PMD_ORDER);
+   if (!p)
+   break;
+
+   for (j = 0; j < HPAGE_PMD_NR; ++j)
+   pages[i++] = p++;
+
+   npages -= HPAGE_PMD_NR;
+   }
+#endif
+
+   while (npages) {
p = alloc_page(gfp_flags);
if (!p) {
 
@@ -753,7 +785,8 @@ static int ttm_get_pages(struct page **pages, unsigned 
npages, int flags,
return -ENOMEM;
}
 
-   pages[r] = p;
+   pages[i++] = p;
+   --npages;
}
return 0;
}
-- 
2.7.4

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

[PATCH 1/9] drm/ttm: remove unsued options from ttm_mem_global_alloc_page

[Christian König]

From: Christian König 

Nobody is actually using that, remove it.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_memory.c | 6 ++
 drivers/gpu/drm/ttm/ttm_page_alloc.c | 3 +--
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 3 +--
 include/drm/ttm/ttm_memory.h | 3 +--
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 29855be..4f9978c 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -546,8 +546,7 @@ int ttm_mem_global_alloc(struct ttm_mem_global *glob, 
uint64_t memory,
 EXPORT_SYMBOL(ttm_mem_global_alloc);
 
 int ttm_mem_global_alloc_page(struct ttm_mem_global *glob,
- struct page *page,
- bool no_wait, bool interruptible)
+ struct page *page)
 {
 
struct ttm_mem_zone *zone = NULL;
@@ -564,8 +563,7 @@ int ttm_mem_global_alloc_page(struct ttm_mem_global *glob,
if (glob->zone_dma32 && page_to_pfn(page) > 0x0010UL)
zone = glob->zone_kernel;
 #endif
-   return ttm_mem_global_alloc_zone(glob, zone, PAGE_SIZE, no_wait,
-interruptible);
+   return ttm_mem_global_alloc_zone(glob, zone, PAGE_SIZE, false, false);
 }
 
 void ttm_mem_global_free_page(struct ttm_mem_global *glob, struct page *page)
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 052e1f1..74f465e 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -882,8 +882,7 @@ int ttm_pool_populate(struct ttm_tt *ttm)
return -ENOMEM;
}
 
-   ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i],
-   false, false);
+   ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i]);
if (unlikely(ret != 0)) {
ttm_pool_unpopulate(ttm);
return -ENOMEM;
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 06bc14b..b8905bd 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -902,8 +902,7 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct 
device *dev)
return -ENOMEM;
}
 
-   ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i],
-   false, false);
+   ret = ttm_mem_global_alloc_page(mem_glob, ttm->pages[i]);
if (unlikely(ret != 0)) {
ttm_dma_unpopulate(ttm_dma, dev);
return -ENOMEM;
diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h
index c452089..8184ff1 100644
--- a/include/drm/ttm/ttm_memory.h
+++ b/include/drm/ttm/ttm_memory.h
@@ -150,8 +150,7 @@ extern int ttm_mem_global_alloc(struct ttm_mem_global 
*glob, uint64_t memory,
 extern void ttm_mem_global_free(struct ttm_mem_global *glob,
uint64_t amount);
 extern int ttm_mem_global_alloc_page(struct ttm_mem_global *glob,
-struct page *page,
-bool no_wait, bool interruptible);
+struct page *page);
 extern void ttm_mem_global_free_page(struct ttm_mem_global *glob,
 struct page *page);
 extern size_t ttm_round_pot(size_t size);
-- 
2.7.4

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

[PATCH 3/9] drm/ttm: add transparent huge page support for DMA allocations v2

[Christian König]

From: Christian König 

Try to allocate huge pages when it makes sense.

v2: fix comment and use ifdef

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 217 ---
 1 file changed, 169 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c 
b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 5362657..e5ef10d 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -60,21 +60,25 @@
 #define NUM_PAGES_TO_ALLOC (PAGE_SIZE/sizeof(struct page *))
 #define SMALL_ALLOCATION   4
 #define FREE_ALL_PAGES (~0U)
+#define VADDR_FLAG_HUGE_POOL   1UL
 
 enum pool_type {
IS_UNDEFINED= 0,
IS_WC   = 1 << 1,
IS_UC   = 1 << 2,
IS_CACHED   = 1 << 3,
-   IS_DMA32= 1 << 4
+   IS_DMA32= 1 << 4,
+   IS_HUGE = 1 << 5
 };
 
 /*
- * The pool structure. There are usually six pools:
+ * The pool structure. There are up to nine pools:
  *  - generic (not restricted to DMA32):
  *  - write combined, uncached, cached.
  *  - dma32 (up to 2^32 - so up 4GB):
  *  - write combined, uncached, cached.
+ *  - huge (not restricted to DMA32):
+ *  - write combined, uncached, cached.
  * for each 'struct device'. The 'cached' is for pages that are actively used.
  * The other ones can be shrunk by the shrinker API if neccessary.
  * @pools: The 'struct device->dma_pools' link.
@@ -114,13 +118,14 @@ struct dma_pool {
  * The accounting page keeping track of the allocated page along with
  * the DMA address.
  * @page_list: The link to the 'page_list' in 'struct dma_pool'.
- * @vaddr: The virtual address of the page
+ * @vaddr: The virtual address of the page and a flag if the page belongs to a
+ * huge pool
  * @dma: The bus address of the page. If the page is not allocated
  *   via the DMA API, it will be -1.
  */
 struct dma_page {
struct list_head page_list;
-   void *vaddr;
+   unsigned long vaddr;
struct page *p;
dma_addr_t dma;
 };
@@ -319,7 +324,8 @@ static int ttm_set_pages_caching(struct dma_pool *pool,
 static void __ttm_dma_free_page(struct dma_pool *pool, struct dma_page *d_page)
 {
dma_addr_t dma = d_page->dma;
-   dma_free_coherent(pool->dev, pool->size, d_page->vaddr, dma);
+   d_page->vaddr &= ~VADDR_FLAG_HUGE_POOL;
+   dma_free_coherent(pool->dev, pool->size, (void *)d_page->vaddr, dma);
 
kfree(d_page);
d_page = NULL;
@@ -327,19 +333,22 @@ static void __ttm_dma_free_page(struct dma_pool *pool, 
struct dma_page *d_page)
 static struct dma_page *__ttm_dma_alloc_page(struct dma_pool *pool)
 {
struct dma_page *d_page;
+   void *vaddr;
 
d_page = kmalloc(sizeof(struct dma_page), GFP_KERNEL);
if (!d_page)
return NULL;
 
-   d_page->vaddr = dma_alloc_coherent(pool->dev, pool->size,
-  _page->dma,
-  pool->gfp_flags);
-   if (d_page->vaddr) {
-   if (is_vmalloc_addr(d_page->vaddr))
-   d_page->p = vmalloc_to_page(d_page->vaddr);
+   vaddr = dma_alloc_coherent(pool->dev, pool->size, _page->dma,
+  pool->gfp_flags);
+   if (vaddr) {
+   if (is_vmalloc_addr(vaddr))
+   d_page->p = vmalloc_to_page(vaddr);
else
-   d_page->p = virt_to_page(d_page->vaddr);
+   d_page->p = virt_to_page(vaddr);
+   d_page->vaddr = (unsigned long)vaddr;
+   if (pool->type & IS_HUGE)
+   d_page->vaddr |= VADDR_FLAG_HUGE_POOL;
} else {
kfree(d_page);
d_page = NULL;
@@ -371,11 +380,40 @@ static void ttm_pool_update_free_locked(struct dma_pool 
*pool,
 }
 
 /* set memory back to wb and free the pages. */
+static void ttm_dma_page_put(struct dma_pool *pool, struct dma_page *d_page)
+{
+   struct page *page = d_page->p;
+   unsigned i, num_pages;
+   int ret;
+
+   /* Don't set WB on WB page pool. */
+   if (!(pool->type & IS_CACHED)) {
+   num_pages = pool->size / PAGE_SIZE;
+   for (i = 0; i < num_pages; ++i, ++page) {
+   ret = set_pages_array_wb(, 1);
+   if (ret) {
+   pr_err("%s: Failed to set %d pages to wb!\n",
+  pool->dev_name, 1);
+   }
+   }
+   }
+
+   list_del(_page->page_list);
+   __ttm_dma_free_page(pool, d_page);
+}
+
 static void ttm_dma_pages_put(struct dma_pool *pool, struct list_head *d_pages,
  struct page *pages[], unsigned npages)
 {
struct dma_page 

Re: drm_hwcomposer moving to fd.o

[Jani Nikula]

On Fri, 22 Sep 2017, Robert Foss  wrote:
> After talking to Liviu Dudau at LPC and Sean Paul & Kaveh Nasri at XDC
> it seems that we all could benefit from community maintainership of
> drm_hwcomposer.

Sounds good!

> Regarding contributions, they are accepted on the dri-devel list, using
> the patch prefix "hwc".

dri-devel averages around 2400-2500 messages per month AFAICT. If hwc
community development picks up any kind of momentum, please consider
starting a new list. Or would a joint userspace list with e.g. libdrm be
beneficial? Daniel Stone will help you here.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH][drm-next] drm/i915/gvt: ensure -ve return value is handled correctly

[Joonas Lahtinen]

On Thu, 2017-09-21 at 16:17 +, Wang, Zhi A wrote:
> Hi Joonas:
> 
> Thanks for the introduction. I have been thinking about the
> possibility of introducing GEM_BUG_ON into GVT-g recently and
> investigating on it. I'm just a bit confused about the usage between
> GEM_BUG_ON and WARN_ON.

GEM_BUG_ON is basically there to catch things that we do not expect
ever to happen within the driver. So we often list the function
preconditions as GEM_BUG_ON. It's there for the same reason as the
lockdep_assert_held and KASAN. It's sometimes heavy checks that we
really want to run when functionally validating kernel.

GEM_BUG_ON became to existence because adding checks for obvious
conditions at the critical command submission path GEM is not
sustainable for performance in production.

The expectation is that each GEM_BUG_ON has a testcase in I-G-T that
has the potential to hit it if driver was modified not to respect those
preconditions. So once our testest passes, we can disable the
GEM_BUG_ONs and be confident of the internal driver quality and get the
release performance.

WARN_ON is mostly used for the cases when the hardware is behaving
differently than we expect. We can't remove them as we don't have all
the hardware in the world to test, but we try to exercise them too
through I-G-Ts. The test will often be the subtest that was written to
reproduce the problem with our expectations of hardware in case of
hangs and other bugs. After we've corrected the driver behaviour, or
got a hardware W/A assigned, we keep the test and add a WARN_ON to make
sure there will be no regression back to the same situation.

This is at least what should happen, given time constraints, there may
be variations.

User behaving unexpectedly should never result in WARN_ON (or even
worse, BUG_ON), should always just be debug messages displayed (not to
trigger the CI) and errors propagated back to user:

https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-uapi.html#recommended
-ioctl-return-values

Bare BUG_ON should only be used when there's the danger of corrupting
system memory or filesystems, so from graphics driver, that's not very
often. Controlled propagation of errors and maybe WARN_ON is always
preferred if possible.


> GEM_BUG_ON is only enabled when kernel debug is enabled, which mostly
> is disabled in a production kernel. In the case of i915, I'm sure it
> will be enabled in CI test so that it can catch broken code path.
> Looking into GVT-g, the similar scenario is we enable it in QA test.
> 
> Let's say GEM_BUG_ON can do its work very well in QA test but QA test
> is not fully covered all the condition, then something might be still
> broken when it comes to the production kernel for user and GEM_BUG_ON
> will be disabled and will not catch that, I guess.
> 
> That's my confusion which scratched my mind during the investigation:
> If GEM_BUG_ON is not always working, then it looks WARN_ON should
> always be used Expected to learn more about the story behind. :)

So if the saying is some object is "never going to be bigger than 2G",
there should be either:

1. GEM_BUG_ON like assertion for it and a test that tries to hit it, by
trying to allocate a huge object for example, and should get rejection
as -EINVAL

2. Test to see if the object is bigger, and propagate back the error if
it is. Either resulting in user reported error if the origin of the
object is outside of kernel <-> hardware. Or a WARN_ON if it's strange
hardware or kernel driver behavior.

You should choose depending on how often your function gets called, and
how critical the execution time is.

Hopefully this clarified things.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Outreachy kernel] [PATCH] drm/tinydrm: Move backlight helpers to a separate file

[Greg KH]

On Fri, Sep 22, 2017 at 04:27:28PM +0530, Meghana Madhyastha wrote:
> Move backlight helpers from tinydrm-helpers.c to
> tinydrm-backlight.c

That says _what_ you did, but not _why_ you are doing it, which really
is the more important part.

thanks,

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

[PATCH] drm/tinydrm: Move backlight helpers to a separate file

[Meghana Madhyastha]

Move backlight helpers from tinydrm-helpers.c to
tinydrm-backlight.c

Signed-off-by: Meghana Madhyastha 
---
 drivers/gpu/drm/tinydrm/core/Makefile|   2 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c | 103 +++
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c   |  94 -
 drivers/gpu/drm/tinydrm/mi0283qt.c   |   1 +
 drivers/gpu/drm/tinydrm/mipi-dbi.c   |   1 +
 include/drm/tinydrm/tinydrm-backlight.h  |  18 
 6 files changed, 124 insertions(+), 95 deletions(-)
 create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c
 create mode 100644 include/drm/tinydrm/tinydrm-backlight.h

diff --git a/drivers/gpu/drm/tinydrm/core/Makefile 
b/drivers/gpu/drm/tinydrm/core/Makefile
index fb221e6..389ca7a 100644
--- a/drivers/gpu/drm/tinydrm/core/Makefile
+++ b/drivers/gpu/drm/tinydrm/core/Makefile
@@ -1,3 +1,3 @@
-tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-helpers.o
+tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-backlight.o 
tinydrm-helpers.o
 
 obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c 
b/drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c
new file mode 100644
index 000..dc6f17d
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-backlight.c
@@ -0,0 +1,103 @@
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+/**
+ * tinydrm_of_find_backlight - Find backlight device in device-tree
+ * @dev: Device
+ *
+ * This function looks for a DT node pointed to by a property named 'backlight'
+ * and uses of_find_backlight_by_node() to get the backlight device.
+ * Additionally if the brightness property is zero, it is set to
+ * max_brightness.
+ *
+ * Returns:
+ * NULL if there's no backlight property.
+ * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
+ * is found.
+ * If the backlight device is found, a pointer to the structure is returned.
+ */
+
+struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
+{
+   struct backlight_device *backlight;
+   struct device_node *np;
+
+   np = of_parse_phandle(dev->of_node, "backlight", 0);
+   if (!np)
+   return NULL;
+
+   backlight = of_find_backlight_by_node(np);
+   of_node_put(np);
+
+   if (!backlight)
+   return ERR_PTR(-EPROBE_DEFER);
+
+   if (!backlight->props.brightness) {
+   backlight->props.brightness = backlight->props.max_brightness;
+   DRM_DEBUG_KMS("Backlight brightness set to %d\n",
+ backlight->props.brightness);
+   }
+
+   return backlight;
+}
+EXPORT_SYMBOL(tinydrm_of_find_backlight);
+
+/**
+ * tinydrm_enable_backlight - Enable backlight helper
+ * @backlight: Backlight device
+ *
+ * Returns:
+ * Zero on success, negative error code on failure.
+ */
+int tinydrm_enable_backlight(struct backlight_device *backlight)
+{
+   unsigned int old_state;
+   int ret;
+
+   if (!backlight)
+   return 0;
+
+   old_state = backlight->props.state;
+   backlight->props.state &= ~BL_CORE_FBBLANK;
+   DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
+ backlight->props.state);
+
+   ret = backlight_update_status(backlight);
+   if (ret)
+   DRM_ERROR("Failed to enable backlight %d\n", ret);
+
+   return ret;
+}
+EXPORT_SYMBOL(tinydrm_enable_backlight);
+
+/**
+ * tinydrm_disable_backlight - Disable backlight helper
+ * @backlight: Backlight device
+ *
+ * Returns:
+ * Zero on success, negative error code on failure.
+ */
+int tinydrm_disable_backlight(struct backlight_device *backlight)
+{
+   unsigned int old_state;
+   int ret;
+
+   if (!backlight)
+   return 0;
+
+   old_state = backlight->props.state;
+   backlight->props.state |= BL_CORE_FBBLANK;
+   DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
+ backlight->props.state);
+   ret = backlight_update_status(backlight);
+   if (ret)
+   DRM_ERROR("Failed to disable backlight %d\n", ret);
+
+   return ret;
+}
+EXPORT_SYMBOL(tinydrm_disable_backlight);
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c 
b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index bd6cce0..ee8ad8c 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -236,100 +236,6 @@ void tinydrm_xrgb_to_gray8(u8 *dst, void *vaddr, 
struct drm_framebuffer *fb,
 }
 EXPORT_SYMBOL(tinydrm_xrgb_to_gray8);
 
-/**
- * tinydrm_of_find_backlight - Find backlight device in device-tree
- * @dev: Device
- *
- * This function looks for a DT node pointed to by a property named 'backlight'
- * and uses of_find_backlight_by_node() to get the backlight device.
- * Additionally if the brightness property is zero, it is set to
- * 

[GIT PULL] etnaviv-fixes for 4.14

[Lucas Stach]

Hi Dave,

just two small etnaviv fixes, one fixing a list corruption, the other
fixing a NULL ptr deref in an error path.

Regards,
Lucas

The following changes since commit 569dbb88e80deb68974ef6fdd6a13edb9d686261:

  Linux 4.13 (2017-09-03 13:56:17 -0700)

are available in the git repository at:

  https://git.pengutronix.de/git/lst/linux etnaviv/fixes

for you to fetch changes up to 518417525f3652c12fb5fad6da4ade66c0072fa3:

  etnaviv: fix gem object list corruption (2017-09-13 15:06:56 +0200)


Lucas Stach (2):
  etnaviv: fix submit error path
  etnaviv: fix gem object list corruption

 drivers/gpu/drm/etnaviv/etnaviv_gem.c| 3 +++
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 6 --
 2 files changed, 7 insertions(+), 2 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 44995] Missing support for MGA G200eW WPCM450 [102b:0532]

[bugzilla-daemon]

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

--- Comment #7 from weizoujian  ---
This card is "video memory" = 0，framebuffer can't used! Why!

-- 
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: [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling

[Jonathan Cameron]

On Thu, 21 Sep 2017 16:15:28 +0200
Wolfram Sang  wrote:

> > > > +/**
> > > > + * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync 
> > > > with i2c_msg
> > > > + * @msg: the message to be synced with
> > > > + * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be 
> > > > NULL.
> > > > + */
> > > > +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
> > > > +{
> > > > +   if (!buf || buf == msg->buf)
> > > > +   return;
> > > > +
> > > > +   if (msg->flags & I2C_M_RD)
> > > > +   memcpy(msg->buf, buf, msg->len);
> > > > +
> > > > +   kfree(buf);  
> > 
> > Only free when you actually allocated it.  Seems to me like you need
> > to check if (!(msg->flags & I2C_M_DMA_SAFE)) before kfree.
> > 
> > Otherwise the logic to do this will be needed in every driver
> > which will get irritating fast.  
> 
> Well, I return early if (buf == msg->buf) which is only true for
> I2C_M_DMA_SAFE. If not, I allocated the buffer. Am I missing something?
> It would be very strange to call this function if the caller allocated
> the buffer manually.
> 
> Thanks for the review!

Doh missed that check and my comment was bonkers even if it hadn't been there.
I come back to the claim of insufficient caffeine.

You are quite correct.  Please ignore previous comment - the code is
fine as is. 

Jonathan
> 
> 

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

[PATCH 1/2] drm: introduce drm_dev_{get/put} functions

[Aishwarya Pant]

Reference counting functions in the kernel typically use get/put suffixes. For
maintaining coding style consistency, introduce drm_dev_{get/put} functions
and let the old APIs (with ref/unref suffixes) remain for compatibility.

Signed-off-by: Aishwarya Pant 
---
 drivers/gpu/drm/drm_drv.c | 64 +--
 include/drm/drm_drv.h |  4 ++-
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index be38ac7..4592314 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -286,13 +286,13 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
spin_lock_irqsave(_minor_lock, flags);
minor = idr_find(_minors_idr, minor_id);
if (minor)
-   drm_dev_ref(minor->dev);
+   drm_dev_get(minor->dev);
spin_unlock_irqrestore(_minor_lock, flags);
 
if (!minor) {
return ERR_PTR(-ENODEV);
} else if (drm_dev_is_unplugged(minor->dev)) {
-   drm_dev_unref(minor->dev);
+   drm_dev_put(minor->dev);
return ERR_PTR(-ENODEV);
}
 
@@ -301,7 +301,7 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)
 
 void drm_minor_release(struct drm_minor *minor)
 {
-   drm_dev_unref(minor->dev);
+   drm_dev_put(minor->dev);
 }
 
 /**
@@ -326,11 +326,11 @@ void drm_minor_release(struct drm_minor *minor)
  * When cleaning up a device instance everything needs to be done in reverse:
  * First unpublish the device instance with drm_dev_unregister(). Then clean up
  * any other resources allocated at device initialization and drop the driver's
- * reference to _device using drm_dev_unref().
+ * reference to _device using drm_dev_put().
  *
  * Note that the lifetime rules for _device instance has still a lot of
  * historical baggage. Hence use the reference counting provided by
- * drm_dev_ref() and drm_dev_unref() only carefully.
+ * drm_dev_get() and drm_dev_put() only carefully.
  *
  * It is recommended that drivers embed  drm_device into their own 
device
  * structure, which is supported through drm_dev_init().
@@ -345,7 +345,7 @@ void drm_minor_release(struct drm_minor *minor)
  * Cleans up all DRM device, calling drm_lastclose().
  *
  * Note: Use of this function is deprecated. It will eventually go away
- * completely.  Please use drm_dev_unregister() and drm_dev_unref() explicitly
+ * completely.  Please use drm_dev_unregister() and drm_dev_put() explicitly
  * instead to make sure that the device isn't userspace accessible any more
  * while teardown is in progress, ensuring that userspace can't access an
  * inconsistent state.
@@ -360,7 +360,7 @@ void drm_put_dev(struct drm_device *dev)
}
 
drm_dev_unregister(dev);
-   drm_dev_unref(dev);
+   drm_dev_put(dev);
 }
 EXPORT_SYMBOL(drm_put_dev);
 
@@ -386,7 +386,7 @@ void drm_dev_unplug(struct drm_device *dev)
mutex_lock(_global_mutex);
drm_device_set_unplugged(dev);
if (dev->open_count == 0)
-   drm_dev_unref(dev);
+   drm_dev_put(dev);
mutex_unlock(_global_mutex);
 }
 EXPORT_SYMBOL(drm_dev_unplug);
@@ -475,8 +475,8 @@ static void drm_fs_inode_free(struct inode *inode)
  * initialization sequence to make sure userspace can't access an inconsistent
  * state.
  *
- * The initial ref-count of the object is 1. Use drm_dev_ref() and
- * drm_dev_unref() to take and drop further ref-counts.
+ * The initial ref-count of the object is 1. Use drm_dev_get() and
+ * drm_dev_put() to take and drop further ref-counts.
  *
  * Note that for purely virtual devices @parent can be NULL.
  *
@@ -626,8 +626,8 @@ EXPORT_SYMBOL(drm_dev_fini);
  * initialization sequence to make sure userspace can't access an inconsistent
  * state.
  *
- * The initial ref-count of the object is 1. Use drm_dev_ref() and
- * drm_dev_unref() to take and drop further ref-counts.
+ * The initial ref-count of the object is 1. Use drm_dev_get() and
+ * drm_dev_put() to take and drop further ref-counts.
  *
  * Note that for purely virtual devices @parent can be NULL.
  *
@@ -670,36 +670,62 @@ static void drm_dev_release(struct kref *ref)
 }
 
 /**
- * drm_dev_ref - Take reference of a DRM device
+ * drm_dev_get - Take reference of a DRM device
  * @dev: device to take reference of or NULL
  *
  * This increases the ref-count of @dev by one. You *must* already own a
- * reference when calling this. Use drm_dev_unref() to drop this reference
+ * reference when calling this. Use drm_dev_put() to drop this reference
  * again.
  *
  * This function never fails. However, this function does not provide *any*
  * guarantee whether the device is alive or running. It only provides a
  * reference to the object and the memory associated with it.
  */
-void drm_dev_ref(struct drm_device *dev)
+void drm_dev_get(struct drm_device *dev)
 {
if (dev)

[PATCH 2/2] drm/tilcdc: replace reference/unreference() with get/put

[Aishwarya Pant]

For maintaining consistency with kernel coding style replace
reference/unreference in ref counting functions with get/put.

The following cocci script was used to generate the tilcdc patch:

@@
expression ex;
@@

(
-drm_framebuffer_unreference(ex);
+drm_framebuffer_put(ex);
|
-drm_dev_unref(ex);
+drm_dev_put(ex);
|
-drm_framebuffer_reference(ex);
+drm_framebuffer_get(ex);
)

Signed-off-by: Aishwarya Pant 
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 6 +++---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 406fe45..d2589f310 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -75,7 +75,7 @@ static void unref_worker(struct drm_flip_work *work, void 
*val)
struct drm_device *dev = tilcdc_crtc->base.dev;
 
mutex_lock(>mode_config.mutex);
-   drm_framebuffer_unreference(val);
+   drm_framebuffer_put(val);
mutex_unlock(>mode_config.mutex);
 }
 
@@ -456,7 +456,7 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
 
set_scanout(crtc, fb);
 
-   drm_framebuffer_reference(fb);
+   drm_framebuffer_get(fb);
 
crtc->hwmode = crtc->state->adjusted_mode;
 }
@@ -633,7 +633,7 @@ int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
return -EBUSY;
}
 
-   drm_framebuffer_reference(fb);
+   drm_framebuffer_get(fb);
 
crtc->primary->fb = fb;
tilcdc_crtc->event = event;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index b0d70f9..74276ef 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -225,7 +225,7 @@ static void tilcdc_fini(struct drm_device *dev)
 
pm_runtime_disable(dev->dev);
 
-   drm_dev_unref(dev);
+   drm_dev_put(dev);
 }
 
 static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
-- 
2.7.4

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

Re: [Outreachy kernel] [RESEND PATCH] drm: Remove obsolete "This is gross" comment

[Haneen Mohammed]

On Thu, Sep 21, 2017 at 11:16:44PM +0200, Julia Lawall wrote:
> 
> 
> On Thu, 21 Sep 2017, Haneen Mohammed wrote:
> 
> > Remove obsolete comment which was initially added in 2008 to annotate
> > that idr_find() was used before idr_remove() since idr_remove() didn't
> > use to return feedback. The comment now is irrelevant with
> > commit f6cd7daecff5 ("drm: Release driver references to handle before
> > making it available again").
> 
> Why did you send it again?
> 
> julia
> 

I forgot to include the dri-devel mailing list for the first patch.

Haneen

> >
> > Signed-off-by: Haneen Mohammed 
> > ---
> >  drivers/gpu/drm/drm_gem.c | 9 -
> >  1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index c55f338..b9bddaa 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -282,15 +282,6 @@ drm_gem_handle_delete(struct drm_file *filp, u32 
> > handle)
> >  {
> > struct drm_gem_object *obj;
> >
> > -   /* This is gross. The idr system doesn't let us try a delete and
> > -* return an error code.  It just spews if you fail at deleting.
> > -* So, we have to grab a lock around finding the object and then
> > -* doing the delete on it and dropping the refcount, or the user
> > -* could race us to double-decrement the refcount and cause a
> > -* use-after-free later.  Given the frequency of our handle lookups,
> > -* we may want to use ida for number allocation and a hash table
> > -* for the pointers, anyway.
> > -*/
> > spin_lock(>table_lock);
> >
> > /* Check if we currently have a reference on the object */
> > --
> > 2.7.4
> >
> > --
> > You received this message because you are subscribed to the Google Groups 
> > "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an 
> > email to outreachy-kernel+unsubscr...@googlegroups.com.
> > To post to this group, send email to outreachy-ker...@googlegroups.com.
> > To view this discussion on the web visit 
> > https://groups.google.com/d/msgid/outreachy-kernel/20170921210424.GA21951%40Haneen.
> > For more options, visit https://groups.google.com/d/optout.
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH v5 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE

[Jonathan Cameron]

On Wed, 20 Sep 2017 20:59:56 +0200
Wolfram Sang  wrote:

> Signed-off-by: Wolfram Sang 

Makes sense as do the other drivers.

Feel free to add

Reviewed-by: Jonathan Cameron 

to all of them (though they hardly took a lot of reviewing given how simple
the patches were :)

> ---
>  drivers/i2c/i2c-dev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
> index 6f638bbc922db4..bbc7aadb4c899d 100644
> --- a/drivers/i2c/i2c-dev.c
> +++ b/drivers/i2c/i2c-dev.c
> @@ -280,6 +280,8 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client 
> *client,
>   res = PTR_ERR(rdwr_pa[i].buf);
>   break;
>   }
> + /* memdup_user allocates with GFP_KERNEL, so DMA is ok */
> + rdwr_pa[i].flags |= I2C_M_DMA_SAFE;
>  
>   /*
>* If the message length is received from the slave (similar

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

Re: [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling

[Jonathan Cameron]

On Wed, 20 Sep 2017 20:59:52 +0200
Wolfram Sang  wrote:

> One helper checks if DMA is suitable and optionally creates a bounce
> buffer, if not. The other function returns the bounce buffer and makes
> sure the data is properly copied back to the message.
> 
> Signed-off-by: Wolfram Sang 

One minor suggestion for wording. Otherwise looks good to me.

Reviewed-by: Jonathan Cameron 

> ---
>  drivers/i2c/i2c-core-base.c | 45 
> +
>  include/linux/i2c.h |  3 +++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 56e46581b84bdb..925a22794d3ced 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2241,6 +2241,51 @@ void i2c_put_adapter(struct i2c_adapter *adap)
>  }
>  EXPORT_SYMBOL(i2c_put_adapter);
>  
> +/**
> + * i2c_get_dma_safe_msg_buf() - get a DMA safe buffer for the given i2c_msg
> + * @msg: the message to be checked
> + * @threshold: the amount of byte from which using DMA makes sense

the minimum number of bytes for which using DMA makes sense.

> + *
> + * Return: NULL if a DMA safe buffer was not obtained. Use msg->buf with PIO.
> + *
> + *  Or a valid pointer to be used with DMA. Note that it can either be
> + *  msg->buf or a bounce buffer. After use, release it by calling
> + *  i2c_release_dma_safe_msg_buf().
> + *
> + * This function must only be called from process context!
> + */
> +u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold)
> +{
> + if (msg->len < threshold)
> + return NULL;
> +
> + if (msg->flags & I2C_M_DMA_SAFE)
> + return msg->buf;
> +
> + if (msg->flags & I2C_M_RD)
> + return kzalloc(msg->len, GFP_KERNEL);
> + else
> + return kmemdup(msg->buf, msg->len, GFP_KERNEL);
> +}
> +EXPORT_SYMBOL_GPL(i2c_get_dma_safe_msg_buf);
> +
> +/**
> + * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with 
> i2c_msg
> + * @msg: the message to be synced with
> + * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
> + */
> +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
> +{
> + if (!buf || buf == msg->buf)
> + return;
> +
> + if (msg->flags & I2C_M_RD)
> + memcpy(msg->buf, buf, msg->len);
> +
> + kfree(buf);
> +}
> +EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_buf);
> +
>  MODULE_AUTHOR("Simon G. Vogl ");
>  MODULE_DESCRIPTION("I2C-Bus main module");
>  MODULE_LICENSE("GPL");
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index d501d3956f13f0..1e99342f180f45 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -767,6 +767,9 @@ static inline u8 i2c_8bit_addr_from_msg(const struct 
> i2c_msg *msg)
>   return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
>  }
>  
> +u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
> +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf);
> +
>  int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short 
> addr);
>  /**
>   * module_i2c_driver() - Helper macro for registering a modular I2C driver

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

[PATCH v2] drm: Remove obsolete "This is gross" comment

[Haneen Mohammed]

Remove obsolete comment which was initially added in 2008 to annotate
that idr_find() was used before idr_remove() since idr_remove() didn't
use to return feedback. The comment now is irrelevant with
commit f6cd7daecff5 ("drm: Release driver references to handle before
making it available again").
In addition, remove the todo item about this issue.

Signed-off-by: Haneen Mohammed 
---
Changes in v2:
- remove todo item regarding this comment

 Documentation/gpu/todo.rst | 6 --
 drivers/gpu/drm/drm_gem.c  | 9 -
 2 files changed, 15 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 22af55d..5e96dc7 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -180,12 +180,6 @@ Contact: Daniel Vetter, respective driver maintainers
 Core refactorings
 =
 
-Use new IDR deletion interface to clean up drm_gem_handle_delete()
---
-
-See the "This is gross" comment -- apparently the IDR system now can return an
-error code instead of oopsing.
-
 Clean up the DRM header mess
 
 
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index c55f338..b9bddaa 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -282,15 +282,6 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
 {
struct drm_gem_object *obj;
 
-   /* This is gross. The idr system doesn't let us try a delete and
-* return an error code.  It just spews if you fail at deleting.
-* So, we have to grab a lock around finding the object and then
-* doing the delete on it and dropping the refcount, or the user
-* could race us to double-decrement the refcount and cause a
-* use-after-free later.  Given the frequency of our handle lookups,
-* we may want to use ida for number allocation and a hash table
-* for the pointers, anyway.
-*/
spin_lock(>table_lock);
 
/* Check if we currently have a reference on the object */
-- 
2.7.4

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

Re: [RFC PATCH v5 3/6] i2c: add docs to clarify DMA handling

[Jonathan Cameron]

On Wed, 20 Sep 2017 16:56:48 -0300
Mauro Carvalho Chehab  wrote:

> Em Wed, 20 Sep 2017 20:59:53 +0200
> Wolfram Sang  escreveu:
> 
> > Signed-off-by: Wolfram Sang   
> 
> Documentation looks OK on my eyes. So:
> 
> Reviewed-by: Mauro Carvalho Chehab 

Really minor suggestion inline. I don't really care either way as
what you had is perfectly comprehensible. 

Reviewed-by: Jonathan Cameron 

> 
> > ---
> >  Documentation/i2c/DMA-considerations | 58 
> > 
> >  1 file changed, 58 insertions(+)
> >  create mode 100644 Documentation/i2c/DMA-considerations
> > 
> > diff --git a/Documentation/i2c/DMA-considerations 
> > b/Documentation/i2c/DMA-considerations
> > new file mode 100644
> > index 00..5a63355c6a9b6f
> > --- /dev/null
> > +++ b/Documentation/i2c/DMA-considerations
> > @@ -0,0 +1,58 @@
> > +=
> > +Linux I2C and DMA
> > +=
> > +
> > +Given that I2C is a low-speed bus where largely small messages are 
> > transferred,

Slightly nicer as:

Given that i2c is a low-speed bus, over which the majority of messages 
transferred are small,

> > +it is not considered a prime user of DMA access. At this time of writing, 
> > only
> > +10% of I2C bus master drivers have DMA support implemented. And the vast
> > +majority of transactions are so small that setting up DMA for it will 
> > likely
> > +add more overhead than a plain PIO transfer.
> > +
> > +Therefore, it is *not* mandatory that the buffer of an I2C message is DMA 
> > safe.
> > +It does not seem reasonable to apply additional burdens when the feature 
> > is so
> > +rarely used. However, it is recommended to use a DMA-safe buffer if your
> > +message size is likely applicable for DMA. Most drivers have this threshold
> > +around 8 bytes (as of today, this is mostly an educated guess, however). 
> > For
> > +any message of 16 byte or larger, it is probably a really good idea. Please
> > +note that other subsystems you use might add requirements. E.g., if your
> > +I2C bus master driver is using USB as a bridge, then you need to have DMA
> > +safe buffers always, because USB requires it.
> > +
> > +For clients, if you use a DMA safe buffer in i2c_msg, set the 
> > I2C_M_DMA_SAFE
> > +flag with it. Then, the I2C core and drivers know they can safely operate 
> > DMA
> > +on it. Note that using this flag is optional. I2C host drivers which are 
> > not
> > +updated to use this flag will work like before. And like before, they risk
> > +using an unsafe DMA buffer. To improve this situation, using 
> > I2C_M_DMA_SAFE in
> > +more and more clients and host drivers is the planned way forward. Note 
> > also
> > +that setting this flag makes only sense in kernel space. User space data is
> > +copied into kernel space anyhow. The I2C core makes sure the destination
> > +buffers in kernel space are always DMA capable.
> > +
> > +FIXME: Need to implement i2c_master_{send|receive}_dma and proper buffers 
> > for i2c_smbus_xfer_emulated.
> > +
> > +Drivers wishing to implement safe DMA can use helper functions from the I2C
> > +core. One gives you a DMA-safe buffer for a given i2c_msg as long as a 
> > certain
> > +threshold is met::
> > +
> > +   dma_buf = i2c_get_dma_safe_msg_buf(msg, threshold_in_byte);
> > +
> > +If a buffer is returned, it is either msg->buf for the I2C_M_DMA_SAFE case 
> > or a
> > +bounce buffer. But you don't need to care about that detail, just use the
> > +returned buffer. If NULL is returned, the threshold was not met or a bounce
> > +buffer could not be allocated. Fall back to PIO in that case.
> > +
> > +In any case, a buffer obtained from above needs to be released. It ensures 
> > data
> > +is copied back to the message and a potentially used bounce buffer is 
> > freed::
> > +
> > +   i2c_release_dma_safe_msg_buf(msg, dma_buf);
> > +
> > +The bounce buffer handling from the core is generic and simple. It will 
> > always
> > +allocate a new bounce buffer. If you want a more sophisticated handling 
> > (e.g.
> > +reusing pre-allocated buffers), you are free to implement your own.
> > +
> > +Please also check the in-kernel documentation for details. The 
> > i2c-sh_mobile
> > +driver can be used as a reference example how to use the above helpers.
> > +
> > +Final note: If you plan to use DMA with I2C (or with anything else, 
> > actually)
> > +make sure you have CONFIG_DMA_API_DEBUG enabled during development. It can 
> > help
> > +you find various issues which can be complex to debug otherwise.  
> 
> 
> 
> Thanks,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

___
dri-devel mailing list

[PATCH 7/7] drm/rockchip: Cocci spatch "vma_pages"

[Thomas Meyer]

Use vma_pages function on vma object instead of explicit computation.
Found by coccinelle spatch "api/vma_pages.cocci"

Signed-off-by: Thomas Meyer 
---

diff -u -p a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -220,7 +220,7 @@ static int rockchip_drm_gem_object_mmap_
 {
struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
unsigned int i, count = obj->size >> PAGE_SHIFT;
-   unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+   unsigned long user_count = vma_pages(vma);
unsigned long uaddr = vma->vm_start;
unsigned long offset = vma->vm_pgoff;
unsigned long end = user_count + offset;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling

[Jonathan Cameron]

On Thu, 21 Sep 2017 14:59:22 +0100
Jonathan Cameron  wrote:

> On Wed, 20 Sep 2017 20:59:52 +0200
> Wolfram Sang  wrote:
> 
> > One helper checks if DMA is suitable and optionally creates a bounce
> > buffer, if not. The other function returns the bounce buffer and makes
> > sure the data is properly copied back to the message.
> > 
> > Signed-off-by: Wolfram Sang   
> 
> One minor suggestion for wording. Otherwise looks good to me.
> 
> Reviewed-by: Jonathan Cameron 
>

Need more coffee. See below.
 
> > ---
> >  drivers/i2c/i2c-core-base.c | 45 
> > +
> >  include/linux/i2c.h |  3 +++
> >  2 files changed, 48 insertions(+)
> > 
> > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> > index 56e46581b84bdb..925a22794d3ced 100644
> > --- a/drivers/i2c/i2c-core-base.c
> > +++ b/drivers/i2c/i2c-core-base.c
> > @@ -2241,6 +2241,51 @@ void i2c_put_adapter(struct i2c_adapter *adap)
> >  }
> >  EXPORT_SYMBOL(i2c_put_adapter);
> >  
> > +/**
> > + * i2c_get_dma_safe_msg_buf() - get a DMA safe buffer for the given i2c_msg
> > + * @msg: the message to be checked
> > + * @threshold: the amount of byte from which using DMA makes sense  
> 
> the minimum number of bytes for which using DMA makes sense.
> 
> > + *
> > + * Return: NULL if a DMA safe buffer was not obtained. Use msg->buf with 
> > PIO.
> > + *
> > + *Or a valid pointer to be used with DMA. Note that it can either be
> > + *msg->buf or a bounce buffer. After use, release it by calling
> > + *i2c_release_dma_safe_msg_buf().
> > + *
> > + * This function must only be called from process context!
> > + */
> > +u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold)
> > +{
> > +   if (msg->len < threshold)
> > +   return NULL;
> > +
> > +   if (msg->flags & I2C_M_DMA_SAFE)
> > +   return msg->buf;
> > +
> > +   if (msg->flags & I2C_M_RD)
> > +   return kzalloc(msg->len, GFP_KERNEL);
> > +   else
> > +   return kmemdup(msg->buf, msg->len, GFP_KERNEL);
> > +}
> > +EXPORT_SYMBOL_GPL(i2c_get_dma_safe_msg_buf);
> > +
> > +/**
> > + * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with 
> > i2c_msg
> > + * @msg: the message to be synced with
> > + * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
> > + */
> > +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
> > +{
> > +   if (!buf || buf == msg->buf)
> > +   return;
> > +
> > +   if (msg->flags & I2C_M_RD)
> > +   memcpy(msg->buf, buf, msg->len);
> > +
> > +   kfree(buf);

Only free when you actually allocated it.  Seems to me like you need
to check if (!(msg->flags & I2C_M_DMA_SAFE)) before kfree.

Otherwise the logic to do this will be needed in every driver
which will get irritating fast.


> > +}
> > +EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_buf);
> > +
> >  MODULE_AUTHOR("Simon G. Vogl ");
> >  MODULE_DESCRIPTION("I2C-Bus main module");
> >  MODULE_LICENSE("GPL");
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index d501d3956f13f0..1e99342f180f45 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -767,6 +767,9 @@ static inline u8 i2c_8bit_addr_from_msg(const struct 
> > i2c_msg *msg)
> > return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
> >  }
> >  
> > +u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
> > +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf);
> > +
> >  int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short 
> > addr);
> >  /**
> >   * module_i2c_driver() - Helper macro for registering a modular I2C driver 
> >  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH v4] drm/bridge/sii8620: add remote control support

[Sean Young]

On Mon, Sep 18, 2017 at 04:37:52PM +0200, Hans Verkuil wrote:
> On 09/18/2017 04:15 PM, Maciej Purski wrote:
> > Hi Hans,
> > some time ago in reply to your email I described what messages does
> > the MHL driver receive and at what time intervals.
> > Regarding that information, do you think that a similar solution as
> > in [1] is required? Would it be OK, if I just set REP_DELAY and REP_PERIOD
> > to values, which I presented in my last email?
> 
> Based on the timings you measured I would say that there is a 99% chance that 
> MHL
> uses exactly the same "Press and Hold" mechanism as CEC. Since you already 
> specify
> RC_PROTO_BIT_CEC in the driver, it will set REP_DELAY and REP_PERIOD to the 
> right
> values automatically.
> 
> You will have to implement the same code as in cec-adap.c for the key press 
> handling,
> though. It's a bit tricky to get it right.
> 
> Since you will have to do the same thing as I do in CEC, I wonder if it would 
> make
> more sense to move this code to the RC core. Basically it ensures that repeat 
> mode
> doesn't turn on until you get two identical key presses 550ms or less apart. 
> This
> is independent of REP_DELAY which can be changed by userspace.
> 
> Sean, what do you think?

Yes, this makes sense. In fact, since IR protocols have different repeat
times, I would like to make this protocol dependant anyway.

To be honest I find REP_DELAY of 500ms too long and REP_PERIOD of 125ms
too short; IOW it takes too long for a key to start repeating, and once
it starts repeating it is very hard to control. If I try to increase the
volume with my remote it first hardly changes at all and then after 500ms
the volume shoots up far too quickly, same thing when navigating menus.

CEC dictates a delay period of 550ms, which is not great for the user IMO.

Anyway I'll have a look at this over the weekend.


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

[PATCH 1/4] drm/amd/powerplay: Cocci spatch "alloc_cast"

[Thomas Meyer]

Remove casting the values returned by memory allocation functions like
kmalloc, kzalloc, kmem_cache_alloc, kmem_cache_zalloc etc."
Found by coccinelle spatch "api/alloc/alloc_cast.cocci"

Signed-off-by: Thomas Meyer 
---

diff -u -p a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c
@@ -291,7 +291,7 @@ static int get_mm_clock_voltage_table(
table_size = sizeof(uint32_t) +
sizeof(phm_ppt_v1_mm_clock_voltage_dependency_record) *
mm_dependency_table->ucNumEntries;
-   mm_table = (phm_ppt_v1_mm_clock_voltage_dependency_table *)
+   mm_table =
kzalloc(table_size, GFP_KERNEL);
 
if (!mm_table)
@@ -519,7 +519,7 @@ static int get_socclk_voltage_dependency
sizeof(phm_ppt_v1_clock_voltage_dependency_record) *
clk_dep_table->ucNumEntries;
 
-   clk_table = (phm_ppt_v1_clock_voltage_dependency_table *)
+   clk_table =
kzalloc(table_size, GFP_KERNEL);
 
if (!clk_table)
@@ -554,7 +554,7 @@ static int get_mclk_voltage_dependency_t
sizeof(phm_ppt_v1_clock_voltage_dependency_record) *
mclk_dep_table->ucNumEntries;
 
-   mclk_table = (phm_ppt_v1_clock_voltage_dependency_table *)
+   mclk_table =
kzalloc(table_size, GFP_KERNEL);
 
if (!mclk_table)
@@ -596,7 +596,7 @@ static int get_gfxclk_voltage_dependency
sizeof(phm_ppt_v1_clock_voltage_dependency_record) *
clk_dep_table->ucNumEntries;
 
-   clk_table = (struct phm_ppt_v1_clock_voltage_dependency_table *)
+   clk_table =
kzalloc(table_size, GFP_KERNEL);
 
if (!clk_table)
@@ -663,7 +663,7 @@ static int get_pix_clk_voltage_dependenc
sizeof(phm_ppt_v1_clock_voltage_dependency_record) *
clk_dep_table->ucNumEntries;
 
-   clk_table = (struct phm_ppt_v1_clock_voltage_dependency_table *)
+   clk_table =
kzalloc(table_size, GFP_KERNEL);
 
if (!clk_table)
@@ -728,7 +728,7 @@ static int get_dcefclk_voltage_dependenc
sizeof(phm_ppt_v1_clock_voltage_dependency_record) *
num_entries;
 
-   clk_table = (struct phm_ppt_v1_clock_voltage_dependency_table *)
+   clk_table =
kzalloc(table_size, GFP_KERNEL);
 
if (!clk_table)
@@ -772,7 +772,7 @@ static int get_pcie_table(struct pp_hwmg
sizeof(struct phm_ppt_v1_pcie_record) *
atom_pcie_table->ucNumEntries;
 
-   pcie_table = (struct phm_ppt_v1_pcie_table *)
+   pcie_table =
kzalloc(table_size, GFP_KERNEL);
 
if (!pcie_table)
@@ -1026,7 +1026,7 @@ static int get_vddc_lookup_table(
table_size = sizeof(uint32_t) +
sizeof(phm_ppt_v1_voltage_lookup_record) * max_levels;
 
-   table = (phm_ppt_v1_voltage_lookup_table *)
+   table =
kzalloc(table_size, GFP_KERNEL);
 
if (NULL == table)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RESEND PATCH] drm: Remove obsolete "This is gross" comment

[Haneen Mohammed]

Remove obsolete comment which was initially added in 2008 to annotate
that idr_find() was used before idr_remove() since idr_remove() didn't
use to return feedback. The comment now is irrelevant with
commit f6cd7daecff5 ("drm: Release driver references to handle before
making it available again").

Signed-off-by: Haneen Mohammed 
---
 drivers/gpu/drm/drm_gem.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index c55f338..b9bddaa 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -282,15 +282,6 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
 {
struct drm_gem_object *obj;
 
-   /* This is gross. The idr system doesn't let us try a delete and
-* return an error code.  It just spews if you fail at deleting.
-* So, we have to grab a lock around finding the object and then
-* doing the delete on it and dropping the refcount, or the user
-* could race us to double-decrement the refcount and cause a
-* use-after-free later.  Given the frequency of our handle lookups,
-* we may want to use ida for number allocation and a hash table
-* for the pointers, anyway.
-*/
spin_lock(>table_lock);
 
/* Check if we currently have a reference on the object */
-- 
2.7.4

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

[PATCH 0/2] drm/tilcdc: replace reference/unreference with get/put

[Aishwarya Pant]

This patchset introduces drm_dev_get() and drm_dev_put() functions that are
intented to be replacements for drm_dev_{ref/unref}.

Then all usages of ref/reference and unref/unreference suffixes are replaced by
get/put reference count functions in tilcdc. The following cocci script was used
to make the tilcdc patch:

@@
expression ex;
@@

(
-drm_framebuffer_unreference(ex);
+drm_framebuffer_put(ex);
|
-drm_dev_unref(ex);
+drm_dev_put(ex);
|
-drm_framebuffer_reference(ex);
+drm_framebuffer_get(ex);
)

Aishwarya Pant (2):
  drm: introduce drm_dev_{get/put} functions
  drm/tilcdc: replace reference/unreference() with get/put

 drivers/gpu/drm/drm_drv.c| 64 +---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |  6 ++--
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  2 +-
 include/drm/drm_drv.h|  4 ++-
 4 files changed, 52 insertions(+), 24 deletions(-)

-- 
2.7.4

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

Re: [PATCH 01/10] drm/panel: Keep track of enabled/prepared

[Andrzej Hajda]

On 22.09.2017 09:13, Andrzej Hajda wrote:
> On 21.09.2017 19:06, Sean Paul wrote:
>> This patch adds state tracking to the drm_panel functions which keep
>> track of enabled and prepared. If the calls are unbalanced, a WARNING is
>> issued.
>>
>> The motivation for this change is that a number of panel drivers
>> (including panel-simple) all do this to protect their regulator
>> refcounts. The atomic framework ensures the calls are balanced, and
>> there  aren't any panel drivers being used by legacy drivers. As such,
>> these checks are useless, but let's add a WARNING just in case something
>> crazy happens (like a legacy driver using a panel).
>>
>> Less code == better.
>>
>> Signed-off-by: Sean Paul 
> I wonder if the tracking is needed at all, panels power states are
> usually the same as states of encoders they are connected to.
> But it is just remark to consider, no strong opposition :)
>
>> ---
>>  drivers/gpu/drm/drm_panel.c |  2 ++
>>  include/drm/drm_panel.h | 38 ++
>>  2 files changed, 36 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
>> index 308d442a531b..9515219d3d2c 100644
>> --- a/drivers/gpu/drm/drm_panel.c
>> +++ b/drivers/gpu/drm/drm_panel.c
>> @@ -48,6 +48,8 @@ static LIST_HEAD(panel_list);
>>  void drm_panel_init(struct drm_panel *panel)
>>  {
>>  INIT_LIST_HEAD(>list);
>> +panel->enabled = false;
>> +panel->prepared = false;
>>  }
>>  EXPORT_SYMBOL(drm_panel_init);
>>  
>> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
>> index 14ac240a1f64..b9a86a4cf29c 100644
>> --- a/include/drm/drm_panel.h
>> +++ b/include/drm/drm_panel.h
>> @@ -24,6 +24,7 @@
>>  #ifndef __DRM_PANEL_H__
>>  #define __DRM_PANEL_H__
>>  
>> +#include 
>>  #include 
>>  #include 
>>  
>> @@ -84,6 +85,8 @@ struct drm_panel_funcs {
>>   * @dev: parent device of the panel
>>   * @funcs: operations that can be performed on the panel
>>   * @list: panel entry in registry
>> + * @enabled: keeps track of the panel enabled status
>> + * @prepared: keeps track of the panel prepared status
>>   */
>>  struct drm_panel {
>>  struct drm_device *drm;
>> @@ -93,6 +96,9 @@ struct drm_panel {
>>  const struct drm_panel_funcs *funcs;
>>  
>>  struct list_head list;
>> +
>> +bool enabled;
>> +bool prepared;

I think better would be to use enum {disabled, prepared, enabled} here,
the checks will be more readable, and will fit better to panel state
machine :), just to consider.

Regards
Andrzej

>>  };
>>  
>>  /**
>> @@ -104,12 +110,18 @@ struct drm_panel {
>>   * is usually no longer possible to communicate with the panel until another
>>   * call to drm_panel_prepare().
>>   *
>> + * Atomic framework should ensure that prepare/unprepare are properly 
>> balanced.
>> + * If this is not the case, a WARNING will be issued.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  static inline int drm_panel_unprepare(struct drm_panel *panel)
>>  {
>> -if (panel && panel->funcs && panel->funcs->unprepare)
>> +if (panel && panel->funcs && panel->funcs->unprepare) {
>> +WARN_ON(!panel->prepared);
> WARN_ON(!panel->prepared || panel->enabled);
>
> Similar double checks should be used in other places.
>
>> +panel->prepared = false;
>>  return panel->funcs->unprepare(panel);
> ret = panel->funcs->unprepare(panel);
> if (!ret)
> panel->prepared = false;
> return ret;
>
> Again this pattern should be repeated in other places.
>
> Different thing is meaning of unsuccessful unprepare/disable? But this
> is other issue.
>
>> +}
>>  
>>  return panel ? -ENOSYS : -EINVAL;
> I think tracking should be performed also for no-op case.
>
> Regards
> Andrzej
>
>>  }
>> @@ -122,12 +134,18 @@ static inline int drm_panel_unprepare(struct drm_panel 
>> *panel)
>>   * drivers. For smart panels it should still be possible to communicate with
>>   * the integrated circuitry via any command bus after this call.
>>   *
>> + * Atomic framework should ensure that enable/disable are properly balanced.
>> + * If this is not the case, a WARNING will be issued.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  static inline int drm_panel_disable(struct drm_panel *panel)
>>  {
>> -if (panel && panel->funcs && panel->funcs->disable)
>> +if (panel && panel->funcs && panel->funcs->disable) {
>> +WARN_ON(!panel->enabled);
>> +panel->enabled = false;
>>  return panel->funcs->disable(panel);
>> +}
>>  
>>  return panel ? -ENOSYS : -EINVAL;
>>  }
>> @@ -140,12 +158,18 @@ static inline int drm_panel_disable(struct drm_panel 
>> *panel)
>>   * the panel. After this has completed it is possible to communicate with 
>> any
>>   * integrated circuitry via a command bus.
>>   *
>> + * Atomic framework should ensure that prepare/unprepare are 

Re: [PATCH 01/10] drm/panel: Keep track of enabled/prepared

[Andrzej Hajda]

On 21.09.2017 19:06, Sean Paul wrote:
> This patch adds state tracking to the drm_panel functions which keep
> track of enabled and prepared. If the calls are unbalanced, a WARNING is
> issued.
>
> The motivation for this change is that a number of panel drivers
> (including panel-simple) all do this to protect their regulator
> refcounts. The atomic framework ensures the calls are balanced, and
> there  aren't any panel drivers being used by legacy drivers. As such,
> these checks are useless, but let's add a WARNING just in case something
> crazy happens (like a legacy driver using a panel).
>
> Less code == better.
>
> Signed-off-by: Sean Paul 

I wonder if the tracking is needed at all, panels power states are
usually the same as states of encoders they are connected to.
But it is just remark to consider, no strong opposition :)

> ---
>  drivers/gpu/drm/drm_panel.c |  2 ++
>  include/drm/drm_panel.h | 38 ++
>  2 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 308d442a531b..9515219d3d2c 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -48,6 +48,8 @@ static LIST_HEAD(panel_list);
>  void drm_panel_init(struct drm_panel *panel)
>  {
>   INIT_LIST_HEAD(>list);
> + panel->enabled = false;
> + panel->prepared = false;
>  }
>  EXPORT_SYMBOL(drm_panel_init);
>  
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 14ac240a1f64..b9a86a4cf29c 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -24,6 +24,7 @@
>  #ifndef __DRM_PANEL_H__
>  #define __DRM_PANEL_H__
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -84,6 +85,8 @@ struct drm_panel_funcs {
>   * @dev: parent device of the panel
>   * @funcs: operations that can be performed on the panel
>   * @list: panel entry in registry
> + * @enabled: keeps track of the panel enabled status
> + * @prepared: keeps track of the panel prepared status
>   */
>  struct drm_panel {
>   struct drm_device *drm;
> @@ -93,6 +96,9 @@ struct drm_panel {
>   const struct drm_panel_funcs *funcs;
>  
>   struct list_head list;
> +
> + bool enabled;
> + bool prepared;
>  };
>  
>  /**
> @@ -104,12 +110,18 @@ struct drm_panel {
>   * is usually no longer possible to communicate with the panel until another
>   * call to drm_panel_prepare().
>   *
> + * Atomic framework should ensure that prepare/unprepare are properly 
> balanced.
> + * If this is not the case, a WARNING will be issued.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  static inline int drm_panel_unprepare(struct drm_panel *panel)
>  {
> - if (panel && panel->funcs && panel->funcs->unprepare)
> + if (panel && panel->funcs && panel->funcs->unprepare) {
> + WARN_ON(!panel->prepared);

WARN_ON(!panel->prepared || panel->enabled);

Similar double checks should be used in other places.

> + panel->prepared = false;
>   return panel->funcs->unprepare(panel);

ret = panel->funcs->unprepare(panel);
if (!ret)
panel->prepared = false;
return ret;

Again this pattern should be repeated in other places.

Different thing is meaning of unsuccessful unprepare/disable? But this
is other issue.

> + }
>  
>   return panel ? -ENOSYS : -EINVAL;

I think tracking should be performed also for no-op case.

Regards
Andrzej

>  }
> @@ -122,12 +134,18 @@ static inline int drm_panel_unprepare(struct drm_panel 
> *panel)
>   * drivers. For smart panels it should still be possible to communicate with
>   * the integrated circuitry via any command bus after this call.
>   *
> + * Atomic framework should ensure that enable/disable are properly balanced.
> + * If this is not the case, a WARNING will be issued.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  static inline int drm_panel_disable(struct drm_panel *panel)
>  {
> - if (panel && panel->funcs && panel->funcs->disable)
> + if (panel && panel->funcs && panel->funcs->disable) {
> + WARN_ON(!panel->enabled);
> + panel->enabled = false;
>   return panel->funcs->disable(panel);
> + }
>  
>   return panel ? -ENOSYS : -EINVAL;
>  }
> @@ -140,12 +158,18 @@ static inline int drm_panel_disable(struct drm_panel 
> *panel)
>   * the panel. After this has completed it is possible to communicate with any
>   * integrated circuitry via a command bus.
>   *
> + * Atomic framework should ensure that prepare/unprepare are properly 
> balanced.
> + * If this is not the case, a WARNING will be issued.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  static inline int drm_panel_prepare(struct drm_panel *panel)
>  {
> - if (panel && panel->funcs && panel->funcs->prepare)
> + if (panel && panel->funcs && panel->funcs->prepare) {
> + 

Re: [PATCH 00/11] drm/sun4i: add CEC support

[Maxime Ripard]

On Thu, Sep 21, 2017 at 09:19:43PM +, Hans Verkuil wrote:
> On 21/09/17 22:37, Maxime Ripard wrote:
> > Hi Hans,
> > 
> > On Fri, Sep 08, 2017 at 10:59:44AM +, Hans Verkuil wrote:
> >> Hi Maxime,
> >>
> >> On 07/18/17 18:29, Maxime Ripard wrote:
> >>> Hi,
> >>>
> >>> On Tue, Jul 11, 2017 at 11:06:52PM +0200, Hans Verkuil wrote:
>  On 11/07/17 22:39, Maxime Ripard wrote:
> > On Tue, Jul 11, 2017 at 08:30:33AM +0200, Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >>
> >> This patch series adds CEC support for the sun4i HDMI controller.
> >>
> >> The CEC hardware support for the A10 is very low-level as it just
> >> controls the CEC pin. Since I also wanted to support GPIO-based CEC
> >> hardware most of this patch series is in the CEC framework to
> >> add a generic low-level CEC pin framework. It is only the final patch
> >> that adds the sun4i support.
> >>
> >> This patch series first makes some small changes in the CEC framework
> >> (patches 1-4) to prepare for this CEC pin support.
> >>
> >> Patch 5-7 adds the new API elements and documents it. Patch 6 reworks
> >> the CEC core event handling.
> >>
> >> Patch 8 adds pin monitoring support (allows userspace to see all
> >> CEC pin transitions as they happen).
> >>
> >> Patch 9 adds the core cec-pin implementation that translates low-level
> >> pin transitions into valid CEC messages. Basically this does what any
> >> SoC with a proper CEC hardware implementation does.
> >>
> >> Patch 10 documents the cec-pin kAPI (and also the cec-notifier kAPI
> >> which was missing).
> >>
> >> Finally patch 11 adds the actual sun4i_hdmi CEC implementation.
> >>
> >> I tested this on my cubieboard. There were no errors at all
> >> after 126264 calls of 'cec-ctl --give-device-vendor-id' while at the
> >> same time running a 'make -j4' of the v4l-utils git repository and
> >> doing a continuous scp to create network traffic.
> >>
> >> This patch series is based on top of the mainline kernel as of
> >> yesterday (so with all the sun4i and cec patches for 4.13 merged).
> >
> > For the whole serie:
> > Reviewed-by: Maxime Ripard 
> >
> >> Maxime, patches 1-10 will go through the media subsystem. How do you
> >> want to handle the final patch? It can either go through the media
> >> subsystem as well, or you can sit on it and handle this yourself during
> >> the 4.14 merge window. Another option is to separate the Kconfig change
> >> into its own patch. That way you can merge the code changes and only
> >> have to handle the Kconfig patch as a final change during the merge
> >> window.
> >
> > We'll probably have a number of reworks for 4.14, so it would be
> > better if I merged it.
> >
> > However, I guess if we just switch to a depends on CEC_PIN instead of
> > a select, everything would just work even if we merge your patches in
> > a separate tree, right?
> 
>  This small patch will do it:
> 
>  diff --git a/drivers/gpu/drm/sun4i/Kconfig 
>  b/drivers/gpu/drm/sun4i/Kconfig
>  index e884d265c0b3..ebad80aefc87 100644
>  --- a/drivers/gpu/drm/sun4i/Kconfig
>  +++ b/drivers/gpu/drm/sun4i/Kconfig
>  @@ -25,7 +25,7 @@ config DRM_SUN4I_HDMI_CEC
>  bool "Allwinner A10 HDMI CEC Support"
>  depends on DRM_SUN4I_HDMI
>  select CEC_CORE
>  -   select CEC_PIN
>  +   depends on CEC_PIN
>  help
> Choose this option if you have an Allwinner SoC with an HDMI
> controller and want to use CEC.
> >>
> >> Just a reminder: now that both this driver and the CEC_PIN code has been
> >> merged in 4.14, this 'depends on' can become a 'select' again.
> > 
> > Thanks for the reminder.
> > 
> > Would that commit work for you:
> > http://code.bulix.org/19o9y6-201254
> 
> Acked-by: Hans Verkuil 
> 
> This obviously should be merged for 4.14.

Yep. I just did, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


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

Re: [PATCH] qxl: fix framebuffer unpinning

[Gerd Hoffmann]

On Fri, 2017-09-22 at 02:03 -0300, Gabriel Krisman Bertazi wrote:
> Gerd Hoffmann  writes:
> 
> >   Hi,
> > 
> > > > "removing the device"?  qxl can't be hotplugged ...
> > > > Or do you mean "rmmod qxl"?
> > > 
> > > rmmod qxl
> > 
> > rmmod: ERROR: Module qxl is in use.
> > 
> > How do you do that?  CONFIG_FBCON=n?
> 
> I can simply disable X, lightdm or whatever is using it and then
> rmmod.

Works for me only with CONFIG_FBCON=n, otherwise the fbcon framebuffer
keeps qxl busy.  So is most cases (typical distro kernel build with
FBCON=y) you simply can't rmmod qxl.

I agree that "rmmod qxl" should be fixed, but IMHO this is independent
from this bugfix, and I'd like to get it out of the door ASAP.  Can I
get your reviewed-by?

cheers,
  Gerd

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

Re: [Outreachy kernel] [RESEND PATCH] drm: Remove obsolete "This is gross" comment

[Julia Lawall]

On Thu, 21 Sep 2017, Haneen Mohammed wrote:

> On Thu, Sep 21, 2017 at 11:16:44PM +0200, Julia Lawall wrote:
> >
> >
> > On Thu, 21 Sep 2017, Haneen Mohammed wrote:
> >
> > > Remove obsolete comment which was initially added in 2008 to annotate
> > > that idr_find() was used before idr_remove() since idr_remove() didn't
> > > use to return feedback. The comment now is irrelevant with
> > > commit f6cd7daecff5 ("drm: Release driver references to handle before
> > > making it available again").
> >
> > Why did you send it again?
> >
> > julia
> >
>
> I forgot to include the dri-devel mailing list for the first patch.

OK, it should have been a v2 with a comment about what changed.  Even if
it is not a technical issue, it saves people wondering about what
happened.

julia

>
> Haneen
>
> > >
> > > Signed-off-by: Haneen Mohammed 
> > > ---
> > >  drivers/gpu/drm/drm_gem.c | 9 -
> > >  1 file changed, 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > > index c55f338..b9bddaa 100644
> > > --- a/drivers/gpu/drm/drm_gem.c
> > > +++ b/drivers/gpu/drm/drm_gem.c
> > > @@ -282,15 +282,6 @@ drm_gem_handle_delete(struct drm_file *filp, u32 
> > > handle)
> > >  {
> > >   struct drm_gem_object *obj;
> > >
> > > - /* This is gross. The idr system doesn't let us try a delete and
> > > -  * return an error code.  It just spews if you fail at deleting.
> > > -  * So, we have to grab a lock around finding the object and then
> > > -  * doing the delete on it and dropping the refcount, or the user
> > > -  * could race us to double-decrement the refcount and cause a
> > > -  * use-after-free later.  Given the frequency of our handle lookups,
> > > -  * we may want to use ida for number allocation and a hash table
> > > -  * for the pointers, anyway.
> > > -  */
> > >   spin_lock(>table_lock);
> > >
> > >   /* Check if we currently have a reference on the object */
> > > --
> > > 2.7.4
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups 
> > > "outreachy-kernel" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an 
> > > email to outreachy-kernel+unsubscr...@googlegroups.com.
> > > To post to this group, send email to outreachy-ker...@googlegroups.com.
> > > To view this discussion on the web visit 
> > > https://groups.google.com/d/msgid/outreachy-kernel/20170921210424.GA21951%40Haneen.
> > > For more options, visit https://groups.google.com/d/optout.
> > >
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170921214707.GA22325%40Haneen.
> For more options, visit https://groups.google.com/d/optout.
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel