Re: [RFC,3/3] drm/komeda: Allow non-component drm_bridge only endpoints

[james qian wang (Arm Technology China)]

On Fri, Oct 04, 2019 at 02:34:42PM +, Mihail Atanassov wrote:
> To support transmitters other than the tda998x, we need to allow
> non-component framework bridges to be attached to a dummy drm_encoder in
> our driver.
> 
> For the existing supported encoder (tda998x), keep the behaviour as-is,
> since there's no way to unbind if a drm_bridge module goes away under
> our feet.
> 
> Signed-off-by: Mihail Atanassov 
> ---
>  .../gpu/drm/arm/display/komeda/komeda_dev.h   |   5 +
>  .../gpu/drm/arm/display/komeda/komeda_drv.c   |  58 ++--
>  .../gpu/drm/arm/display/komeda/komeda_kms.c   | 133 +-
>  .../gpu/drm/arm/display/komeda/komeda_kms.h   |   5 +
>  4 files changed, 187 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h 
> b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> index e392b8ffc372..64d2902e2e0c 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
> @@ -176,6 +176,11 @@ struct komeda_dev {
>  
>   /** @irq: irq number */
>   int irq;
> + /** @has_components:
> +  *
> +  * if true, use the component framework to bind/unbind driver
> +  */
> + bool has_components;
>  
>   /** @lock: used to protect dpmode */
>   struct mutex lock;
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c 
> b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> index 9ed25ffe0e22..34cfc6a4c3e4 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> @@ -10,6 +10,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include "komeda_dev.h"
>  #include "komeda_kms.h"
>  
> @@ -69,18 +71,35 @@ static int compare_of(struct device *dev, void *data)
>   return dev->of_node == data;
>  }
>  
> -static void komeda_add_slave(struct device *master,
> -  struct component_match **match,
> -  struct device_node *np,
> -  u32 port, u32 endpoint)
> +static int komeda_add_slave(struct device *master,
> + struct komeda_drv *mdrv,
> + struct component_match **match,
> + struct device_node *np,
> + u32 port, u32 endpoint)
>  {
>   struct device_node *remote;
> + struct drm_bridge *bridge;
> + int ret = 0;
>  
>   remote = of_graph_get_remote_node(np, port, endpoint);
> - if (remote) {
> + if (!remote)
> + return 0;
> +
> + if (komeda_remote_device_is_component(np, port, endpoint)) {
> + mdrv->mdev.has_components = true;
>   drm_of_component_match_add(master, match, compare_of, remote);
> - of_node_put(remote);
> + goto exit;
> + }
> +
> + bridge = of_drm_find_bridge(remote);
> + if (!bridge) {
> + ret = -EPROBE_DEFER;
> + goto exit;
>   }
> +
> +exit:
> + of_node_put(remote);
> + return ret;
>  }
>  
>  static int komeda_platform_probe(struct platform_device *pdev)
> @@ -89,6 +108,7 @@ static int komeda_platform_probe(struct platform_device 
> *pdev)
>   struct component_match *match = NULL;
>   struct device_node *child;
>   struct komeda_drv *mdrv;
> + int ret;
>  
>   if (!dev->of_node)
>   return -ENODEV;
> @@ -101,14 +121,27 @@ static int komeda_platform_probe(struct platform_device 
> *pdev)
>   if (of_node_cmp(child->name, "pipeline") != 0)
>   continue;
>  
> - /* add connector */
> - komeda_add_slave(dev, , child, KOMEDA_OF_PORT_OUTPUT, 0);
> - komeda_add_slave(dev, , child, KOMEDA_OF_PORT_OUTPUT, 1);
> + /* attach any remote devices if present */
> + ret = komeda_add_slave(dev, mdrv, , child,
> +KOMEDA_OF_PORT_OUTPUT, 0);
> + if (ret)
> + goto free_mdrv;
> + ret = komeda_add_slave(dev, mdrv, , child,
> +KOMEDA_OF_PORT_OUTPUT, 1);
> + if (ret)
> + goto free_mdrv;
>   }
>  
>   dev_set_drvdata(dev, mdrv);
>  
> - return component_master_add_with_match(dev, _master_ops, match);
> + return mdrv->mdev.has_components
> + ? component_master_add_with_match(
> + dev, _master_ops, match)
> + : komeda_bind(dev);
> +
> +free_mdrv:
> + devm_kfree(dev, mdrv);
> + return ret;
>  }
>  
>  static int komeda_platform_remove(struct platform_device *pdev)
> @@ -116,7 +149,10 @@ static int komeda_platform_remove(struct platform_device 
> *pdev)
>   struct device *dev = >dev;
>   struct komeda_drv *mdrv = dev_get_drvdata(dev);
>  
> - component_master_del(dev, _master_ops);
> + if (mdrv->mdev.has_components)
> + 

Re: [PATCH] drm/komeda: Adds zorder support

[james qian wang (Arm Technology China)]

On Tue, Oct 08, 2019 at 05:00:46PM +0200, Daniel Vetter wrote:
> On Mon, May 20, 2019 at 5:33 AM Lowry Li (Arm Technology China)
>  wrote:
> >
> > - Creates the zpos property.
> > - Implement komeda_crtc_normalize_zpos to replace
> > drm_atomic_normalize_zpos, reasons as the following:
> >
> > 1. The drm_atomic_normalize_zpos allows to configure same zpos for
> > different planes, but komeda doesn't support such configuration.
> 
> Just stumbled over your custom normalized_zpos calculation, and it
> looks very fishy. You seem to reinvent the normalized zpos
> computation, which has the entire job of resolving duplicated zpos
> values (which can happen with legacy kms). So the above is definitely
> wrong.
> 
> Can you pls do a patch to remove your own code, and replace it with
> helper usage? Or at least explain why exactly you can't use the
> standard normalized zpos stuff and need your own (since it really
> looks like just reinventing the same thing).
> -Daniel
> 
> > 2. For further slave pipline case, Komeda need to calculate the
> > max_slave_zorder, we will merge such calculation into
> > komed_crtc_normalize_zpos to save a separated plane_state loop.

> > 3. For feature none-scaling layer_split, which a plane_state will be
> > assigned to two individual layers(left/right), which requires two
> > normalize_zpos for this plane, plane_st->normalize_zpos will be used
> > by left layer, normalize_zpos + 1 for right_layer.
> >

Yes, the komeda/drm zpos_normalize are simlilar, 

First requirement is here the "layer split" support.

Komeda HW has a input size limitation for YUV format, compare with
RGB, which only has the half capablities, like

- Layer support 4K RGB input size.
- YUV only can support 2K.

To hide this limitation, we introdue Layer split, which splits
one drm_plane data flows to two halves (left/right), and handle them by
two individual HW layers, and of course, two layers need two different
normalize_zpos. but drm_atomic_normalize_zpos() only assign one value
to this plane, but we need two values for layer_split.

Secondary requirement:

we also need the ordered plane list that built by normalize_zpos(),
but current drm version doesn't return it.

For more komeda things:
https://www.kernel.org/doc/html/latest/gpu/komeda-kms.html?highlight=komeda

LAST.
Yes, the komeda and drm version zpos normalize are similar,
we can not use the standard version only because some tiny but
special requirements.
I saw the upstream is working for the zorder refinement, maybe we
can add komeda's requirements into the drm_atomic_normalize_zpos()
and drop komeda version together with this refinement.

Best Regards
James

> > This patch series depends on:
> > - https://patchwork.freedesktop.org/series/58710/
> > - https://patchwork.freedesktop.org/series/59000/
> > - https://patchwork.freedesktop.org/series/59002/
> > - https://patchwork.freedesktop.org/series/59747/
> > - https://patchwork.freedesktop.org/series/59915/
> > - https://patchwork.freedesktop.org/series/60083/
> > - https://patchwork.freedesktop.org/series/60698/
> >
> > Signed-off-by: Lowry Li (Arm Technology China) 
> > ---
> >  drivers/gpu/drm/arm/display/komeda/komeda_kms.c   | 90 
> > ++-
> >  drivers/gpu/drm/arm/display/komeda/komeda_kms.h   |  3 +
> >  drivers/gpu/drm/arm/display/komeda/komeda_plane.c |  6 +-
> >  3 files changed, 97 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c 
> > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > index 306ea06..0ec7665 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> > @@ -100,6 +100,90 @@ static void komeda_kms_commit_tail(struct 
> > drm_atomic_state *old_state)
> > .atomic_commit_tail = komeda_kms_commit_tail,
> >  };
> >
> > +static int komeda_plane_state_list_add(struct drm_plane_state *plane_st,
> > +  struct list_head *zorder_list)
> > +{
> > +   struct komeda_plane_state *new = to_kplane_st(plane_st);
> > +   struct komeda_plane_state *node, *last;
> > +
> > +   last = list_empty(zorder_list) ?
> > +  NULL : list_last_entry(zorder_list, typeof(*last), 
> > zlist_node);
> > +
> > +   /* Considering the list sequence is zpos increasing, so if list is 
> > empty
> > +* or the zpos of new node bigger than the last node in list, no 
> > need
> > +* loop and just insert the new one to the tail of the list.
> > +*/
> > +   if (!last || (new->base.zpos > last->base.zpos)) {
> > +   list_add_tail(>zlist_node, zorder_list);
> > +   return 0;
> > +   }
> > +
> > +   /* Build the list by zpos increasing */
> > +   list_for_each_entry(node, zorder_list, zlist_node) {
> > +   if (new->base.zpos < node->base.zpos) {
> > +   list_add_tail(>zlist_node, >zlist_node);
> > +   

[Bug 111922] amdgpu fails to resume on 5.2 kernel [regression]

[bugzilla-daemon]

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

--- Comment #2 from Pierre Ossman  ---
Not easily unfortunately as I've only been using Fedora kernels, so I don't
have a build environment set up.

-- 
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 RFC v2 5/5] ARM: dts: qcom: msm8974-hammerhead: add support for external display

[Stephen Boyd]

Quoting Brian Masney (2019-10-06 18:45:09)
> diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts 
> b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> index b607c9ff9e12..380a805cd1f0 100644
> --- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> +++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
> @@ -371,6 +401,40 @@
> function = "gpio";
> };
> };
> +
> +   hdmi_pin: hdmi {
> +   cec {
> +   pins = "gpio31";
> +   function = "hdmi_cec";
> +   };
> +
> +   ddc {
> +   pins = "gpio32", "gpio33";
> +   function = "hdmi_ddc";
> +   };
> +
> +   hpd {
> +   pins = "gpio34";
> +   function = "hdmi_hpd";
> +   };
> +   };
> +
> +   anx_msm_pin: anx {
> +   irq {
> +   pins = "gpio28";
> +   function = "gpio";

Is function = "gpio" necessary anymore? I thought we would turn gpios
into gpio function when it's requested as a gpio by some consumer.

> +   drive-strength = <8>;
> +   bias-pull-up;
> +   input-enable;
> +   };
> +
> +   reset {
> +   pins = "gpio68";
> +   function = "gpio";
> +   drive-strength = <8>;
> +   bias-pull-up;
> +   };
> +   };
> };
>  
> vibrator@fd8c3450 {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC v2 4/5] ARM: dts: qcom: msm8974: add HDMI nodes

[Stephen Boyd]

Quoting Brian Masney (2019-10-06 18:45:08)
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi 
> b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 7fc23e422cc5..af02eace14e2 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -1335,6 +1342,77 @@
> clocks = < MDSS_AHB_CLK>;
> clock-names = "iface";
> };
> +
> +   hdmi: hdmi-tx@fd922100 {
> +   status = "disabled";
> +
> +   compatible = "qcom,hdmi-tx-8974";
> +   reg = <0xfd922100 0x35c>,
> + <0xfc4b8000 0x60f0>;
> +   reg-names = "core_physical",
> +   "qfprom_physical";

Is this the qfprom "uncorrected" physical address? If so, why can't this
node use an nvmem to read whatever it needs out of the qfprom?

> +
> +   interrupt-parent = <>;
> +   interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
> +
> +   power-domains = < MDSS_GDSC>;
> +
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/2] drm/komeda: Add line size support

[james qian wang (Arm Technology China)]

On Fri, Sep 27, 2019 at 12:42:13PM +, Liviu Dudau wrote:
> On Fri, Sep 27, 2019 at 02:02:59AM +, james qian wang (Arm Technology 
> China) wrote:
> > On Thu, Sep 26, 2019 at 11:51:21AM +, Liviu Dudau wrote:
> > > On Thu, Sep 26, 2019 at 10:00:22AM +, Lowry Li (Arm Technology China) 
> > > wrote:
> > > > Hi Lowry,
> > > > On Wed, Sep 25, 2019 at 10:24:58AM +, Liviu Dudau wrote:
> > > > > Hi Lowry,
> > > > > 
> > > > > On Tue, Sep 24, 2019 at 08:00:44AM +, Lowry Li (Arm Technology 
> > > > > China) wrote:
> > > > > > From: "Lowry Li (Arm Technology China)" 
> > > > > > 
> > > > > > On D71, we are using the global line size. From D32, every
> > > > > > component have a line size register to indicate the fifo size.
> > > > > > 
> > > > > > So this patch is to set line size support and do the line size
> > > > > > check.
> > > > > > 
> > > > > > Signed-off-by: Lowry Li (Arm Technology China) 
> > > > > > ---
> > > > > >  .../arm/display/komeda/d71/d71_component.c| 57 
> > > > > > ---
> > > > > >  .../gpu/drm/arm/display/komeda/d71/d71_regs.h |  9 +--
> > > > > >  .../drm/arm/display/komeda/komeda_pipeline.h  |  2 +
> > > > > >  .../display/komeda/komeda_pipeline_state.c| 17 ++
> > > > > >  4 files changed, 70 insertions(+), 15 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c 
> > > > > > b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > > index 7b374a3b911e..357837b9d6ed 100644
> > > > > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > > > > > @@ -106,6 +106,23 @@ static void dump_block_header(struct seq_file 
> > > > > > *sf, void __iomem *reg)
> > > > > >i, hdr.output_ids[i]);
> > > > > >  }
> > > > > >  
> > > > > > +/* On D71, we are using the global line size. From D32, every 
> > > > > > component have
> > > > > > + * a line size register to indicate the fifo size.
> > > > > > + */
> > > > > > +static u32 __get_blk_line_size(struct d71_dev *d71, u32 __iomem 
> > > > > > *reg,
> > > > > > +  u32 max_default)
> > > > > > +{
> > > > > > +   if (!d71->periph_addr)
> > > > > > +   max_default = malidp_read32(reg, BLK_MAX_LINE_SIZE);
> > > > > > +
> > > > > > +   return max_default;
> > > > > > +}
> > > > > > +
> > > > > > +static u32 get_blk_line_size(struct d71_dev *d71, u32 __iomem *reg)
> > > > > > +{
> > > > > > +   return __get_blk_line_size(d71, reg, d71->max_line_size);
> > > > > > +}
> > > > > 
> > > > > I know you're trying to save typing the extra parameter, but looking 
> > > > > at the rest of
> > > > > the diff I think it would look better if you get rid of 
> > > > > get_blk_line_size() function
> > > > > and use the name for the function with 3 parameters.
> > > > > 
> > > > > Otherwise, patch looks good to me.
> > > > > 
> > > > > Reviewed-by: Liviu Dudau 

Reviewed-by: James Qian Wang (Arm Technology China) 
> > > > > 
> > > > > Best regards,
> > > > > Liviu
> > > > Thanks for the comments.
> > > > But considering from D32 every component have a line size register
> > > > and no need default value, so we have get_blk_line_size() without
> > > > default argument and also can save some typing and lines. That's
> > > > why we want to keep __get_blk_line_size().
> > > 
> > > I was suggesting to remove get_blk_line_size and only use 
> > > __get_blk_line_size() with
> > > explicit use of d71->max_line_size where it makes sense.
> > >
> > 
> > Hi Liviu:
> 
> Hi James,
> 
> > 
> > Thank you for the suggestion.
> > 
> > Seems lowry doesn't describe it clearly.
> > 
> > the stroy is like this, for current komeda products:
> > 
> > - D71: Doesn't have per component line_size register to indicate the
> >fifo size.
> >And the fifo size is quite customized for every component,
> >Alought for HW it is just a const value. but since it doesn't exposed
> >to SW. So for driver It's quite annoy to judage via lots of 
> > hints. 
> > 
> > - D32 or newer: have line_size register.
> > 
> > So for compatible with these two class products, we defined two functions:
> > 
> > - __get_blk_line_size():
> > 
> >   for d71 specific component like spliter/merger, that's why here we
> >   need input a default line_size, since all d71 specific component
> >   doesn't have its own line_size register or can not be indicated via
> >   the register GCU->line_size.
> >   Need to set it manually according to the specific component and lots
> >   of hints.
> > 
> > - get_blk_line_size(): 
> > 
> >   Two cases:
> >   -- d32 or newer: which have its own fifo line_size register
> >   -- d71: the line_size always same as the GCU->line_size
> >   register (the d71->max_line_size) 
> > 
> > So last as a conclusion:
> > 
> > - get_blk_line_size():
> >   is for the component that line_size can be indicated by HW line_size 
> > 

Re: [PATCH -next] treewide: remove unused argument in lock_release()

[Yuyang Du]

I didn't have the guts to do this, and I am glad you did it :)

Yuyang

On Fri, 20 Sep 2019 at 00:10, Qian Cai  wrote:
>
> Since the commit b4adfe8e05f1 ("locking/lockdep: Remove unused argument
> in __lock_release"), @nested is no longer used in lock_release(), so
> remove it from all lock_release() calls and friends.
>
> Signed-off-by: Qian Cai 
> ---
>  drivers/gpu/drm/drm_connector.c   |  2 +-
>  drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  |  6 +++---
>  drivers/gpu/drm/i915/gt/intel_engine_pm.c |  2 +-
>  drivers/gpu/drm/i915/i915_request.c   |  2 +-
>  drivers/tty/tty_ldsem.c   |  8 
>  fs/dcache.c   |  2 +-
>  fs/jbd2/transaction.c |  4 ++--
>  fs/kernfs/dir.c   |  4 ++--
>  fs/ocfs2/dlmglue.c|  2 +-
>  include/linux/jbd2.h  |  2 +-
>  include/linux/lockdep.h   | 21 ++---
>  include/linux/percpu-rwsem.h  |  4 ++--
>  include/linux/rcupdate.h  |  2 +-
>  include/linux/rwlock_api_smp.h| 16 
>  include/linux/seqlock.h   |  4 ++--
>  include/linux/spinlock_api_smp.h  |  8 
>  include/linux/ww_mutex.h  |  2 +-
>  include/net/sock.h|  2 +-
>  kernel/bpf/stackmap.c |  2 +-
>  kernel/cpu.c  |  2 +-
>  kernel/locking/lockdep.c  |  3 +--
>  kernel/locking/mutex.c|  4 ++--
>  kernel/locking/rtmutex.c  |  6 +++---
>  kernel/locking/rwsem.c| 10 +-
>  kernel/printk/printk.c| 10 +-
>  kernel/sched/core.c   |  2 +-
>  lib/locking-selftest.c| 24 
>  mm/memcontrol.c   |  2 +-
>  net/core/sock.c   |  2 +-
>  tools/lib/lockdep/include/liblockdep/common.h |  3 +--
>  tools/lib/lockdep/include/liblockdep/mutex.h  |  2 +-
>  tools/lib/lockdep/include/liblockdep/rwlock.h |  2 +-
>  tools/lib/lockdep/preload.c   | 16 
>  33 files changed, 90 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..4a8b2e5c2af6 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -719,7 +719,7 @@ void drm_connector_list_iter_end(struct 
> drm_connector_list_iter *iter)
> __drm_connector_put_safe(iter->conn);
> spin_unlock_irqrestore(>connector_list_lock, flags);
> }
> -   lock_release(_list_iter_dep_map, 0, _RET_IP_);
> +   lock_release(_list_iter_dep_map, _RET_IP_);
>  }
>  EXPORT_SYMBOL(drm_connector_list_iter_end);
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> index edd21d14e64f..1a51b3598d63 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> @@ -509,14 +509,14 @@ void i915_gem_shrinker_taints_mutex(struct 
> drm_i915_private *i915,
>   I915_MM_SHRINKER, 0, _RET_IP_);
>
> mutex_acquire(>dep_map, 0, 0, _RET_IP_);
> -   mutex_release(>dep_map, 0, _RET_IP_);
> +   mutex_release(>dep_map, _RET_IP_);
>
> -   mutex_release(>drm.struct_mutex.dep_map, 0, _RET_IP_);
> +   mutex_release(>drm.struct_mutex.dep_map, _RET_IP_);
>
> fs_reclaim_release(GFP_KERNEL);
>
> if (unlock)
> -   mutex_release(>drm.struct_mutex.dep_map, 0, _RET_IP_);
> +   mutex_release(>drm.struct_mutex.dep_map, _RET_IP_);
>  }
>
>  #define obj_to_i915(obj__) to_i915((obj__)->base.dev)
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c 
> b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index 65b5ca74b394..7f647243b3b9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -52,7 +52,7 @@ static inline unsigned long __timeline_mark_lock(struct 
> intel_context *ce)
>  static inline void __timeline_mark_unlock(struct intel_context *ce,
>   unsigned long flags)
>  {
> -   mutex_release(>timeline->mutex.dep_map, 0, _THIS_IP_);
> +   mutex_release(>timeline->mutex.dep_map, _THIS_IP_);
> local_irq_restore(flags);
>  }
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c 
> b/drivers/gpu/drm/i915/i915_request.c
> index a53777dd371c..e1f1be4d0531 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1456,7 +1456,7 @@ long i915_request_wait(struct i915_request *rq,
> dma_fence_remove_callback(>fence, );
>
>  out:
> -   

linux-next: build failure after merge of the drm-misc tree

[Stephen Rothwell]

Hi all,

After merging the drm-misc tree, today's linux-next build (x86_64
allmodconfig) failed like this:

In file included from drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_module.c:25:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_priv.h:40:10: fatal error: drm/drmP.h: 
No such file or directory
   40 | #include 
  |  ^~~~
In file included from drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_chardev.c:38:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_priv.h:40:10: fatal error: drm/drmP.h: 
No such file or directory
   40 | #include 
  |  ^~~~
In file included from drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device.c:26:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_priv.h:40:10: fatal error: drm/drmP.h: 
No such file or directory
   40 | #include 
  |  ^~~~
In file included from drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:34:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_priv.h:40:10: fatal error: drm/drmP.h: 
No such file or directory
   40 | #include 
  |  ^~~~


Caused by commit

  4e98f871bcff ("drm: delete drmP.h + drm_os_linux.h")

interacting with commit

  6b855f7b83d2 ("drm/amdkfd: Check against device cgroup")

from the amdgpu tree.

I added the following merge fix patch for today:

From: Stephen Rothwell 
Date: Wed, 9 Oct 2019 11:24:38 +1100
Subject: [PATCH] drm/amdkfd: update for drmP.h removal

Signed-off-by: Stephen Rothwell 
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index b8b4485c8f74..41bc0428bfc0 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -37,7 +37,9 @@
 #include 
 #include 
 #include 
-#include 
+#include 
+#include 
+#include 
 #include 
 
 #include "amd_shared.h"
@@ -49,8 +51,6 @@
 /* GPU ID hash width in bits */
 #define KFD_GPU_ID_HASH_WIDTH 16
 
-struct drm_device;
-
 /* Use upper bits of mmap offset to store KFD driver specific information.
  * BITS[63:62] - Encode MMAP type
  * BITS[61:46] - Encode gpu_id. To identify to which GPU the offset belongs to
-- 
2.23.0

-- 
Cheers,
Stephen Rothwell


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

[Bug 111913] AMD Navi10 GPU powerplay issues when using two DisplayPort connectors

[bugzilla-daemon]

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

--- Comment #9 from Andrew Sheldon  ---
(In reply to Stefan Rehm from comment #8)
> In my case the resolution of both monitors is 2560x1440

You could try overclocking (or underclocking) one or both monitors to see if
the bug still exists, using:
https://github.com/kevinlekiller/cvt_modeline_calculator_12

I recommend using the "-b" option which uses reduced blanking V2 mode, but you
could experiment with different options.

Then to use it:

xrandr --output  --newmode  

xrandr --output  --addmode  

xrandr --output  --mode 

Modeline name being whatever you like.

You'll probably have to launch X with one of the monitors disconnected (as the
bug may trigger before you can apply the modeline change). I believe the amdgpu
DDX has support for specifying modelines, but I don't know the syntax off the
top of my head.

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

linux-next: manual merge of the drm-misc tree with the drm tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the drm-misc tree got a conflict in:

  drivers/gpu/drm/i915/i915_gem_gtt.c

between commit:

  1e0a96e50882 ("drm/i915: export color_differs")

from the drm tree and commit:

  71724f708997 ("drm/mm: Use helpers for drm_mm_node booleans")

from the drm-misc tree.

I fixed it up (I used the former change) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell


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

linux-next: manual merge of the drm-misc tree with the drm tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the drm-misc tree got a conflict in:

  drivers/gpu/drm/i915/i915_vma.c

between commits:

  1e0a96e50882 ("drm/i915: export color_differs")
  33dd88992313 ("drm/i915: cleanup cache-coloring")
  b290a78b5c3d ("drm/i915: Use helpers for drm_mm_node booleans")
  5e053450c1c3 ("drm/i915: Only track bound elements of the GTT")
  2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex")

from the drm tree and commit:

  71724f708997 ("drm/mm: Use helpers for drm_mm_node booleans")

from the drm-misc tree.

I fixed it up (the subset of 71724f708997 affecting this file is in
b290a78b5c3d) and can carry the fix as necessary. This is now fixed as
far as linux-next is concerned, but any non trivial conflicts should be
mentioned to your upstream maintainer when your tree is submitted for
merging.  You may also want to consider cooperating with the maintainer
of the conflicting tree to minimise any particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell


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

linux-next: manual merge of the drm-misc tree with the drm tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the drm-misc tree got a conflict in:

  drivers/gpu/drm/i915/i915_gem.c

between commits:

  b290a78b5c3d ("drm/i915: Use helpers for drm_mm_node booleans")
  2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex")

from the drm tree and commit:

  71724f708997 ("drm/mm: Use helpers for drm_mm_node booleans")

from the drm-misc tree.

I fixed it up (b290a78b5c3d and 71724f708997 do basically identical
things to this file ...) and can carry the fix as necessary. This is
now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your
tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell


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

Re: [PATCH v3 1/2] dt-bindings: pwm: Convert PWM bindings to json-schema

[Paul Walmsley]

On Wed, 2 Oct 2019, Krzysztof Kozlowski wrote:

> Convert generic PWM bindings to DT schema format using json-schema.  The
> consumer bindings are split to separate file.
> 
> Signed-off-by: Krzysztof Kozlowski 
> 
> ---
> 

[ ... ]

> diff --git a/Documentation/devicetree/bindings/pwm/pwm-sifive.txt 
> b/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> index 36447e3c9378..3d1dd7b06efc 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm-sifive.txt
> @@ -17,7 +17,7 @@ Required properties:
>Please refer to sifive-blocks-ip-versioning.txt for details.
>  - reg: physical base address and length of the controller's registers
>  - clocks: Should contain a clock identifier for the PWM's parent clock.
> -- #pwm-cells: Should be 3. See pwm.txt in this directory
> +- #pwm-cells: Should be 3. See pwm.yaml in this directory
>for a description of the cell format.
>  - interrupts: one interrupt per PWM channel

For the SiFive PWM driver documentation:

Acked-by: Paul Walmsley 


- Paul

Re: liboutput: thoughts about shared library on top of DRM/KMS

[Keith Packard]

Neil Armstrong  writes:

> Seeing the description, it seems to be a libdrm with steroids,
> why libdrm doesn't handle all this already ?

That'd be a lot of steroids; we're talking about creating helper
functions all the way up to rendering images into scanout buffers
(presumably using Vulkan?) for format conversion or flattening.

> Is there a plan to maybe use it as a foundation for projects like
> wlroots or drm_hwcomposer for example ?

Yes, the goal is to start to share code across a wide range of DRM
users, instead of having everyone roll their own.

-- 
-keith


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

linux-next: manual merge of the drm-misc tree with the drm tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the drm-misc tree got a conflict in:

  drivers/gpu/drm/i915/i915_drv.c

between commit:

  2d6f6f359fd8 ("drm/i915: add i915_driver_modeset_remove()")

from the drm tree and commit:

  f2521f7731ed ("drm/i915: switch to 
drm_fb_helper_remove_conflicting_pci_framebuffers")

from the drm-misc tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/i915/i915_drv.c
index 15abad5c2d62,1c4ff8b5b0a2..
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@@ -350,44 -422,6 +350,19 @@@ out
return ret;
  }
  
- static int i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
- {
-   struct apertures_struct *ap;
-   struct pci_dev *pdev = dev_priv->drm.pdev;
-   struct i915_ggtt *ggtt = _priv->ggtt;
-   bool primary;
-   int ret;
- 
-   ap = alloc_apertures(1);
-   if (!ap)
-   return -ENOMEM;
- 
-   ap->ranges[0].base = ggtt->gmadr.start;
-   ap->ranges[0].size = ggtt->mappable_end;
- 
-   primary =
-   pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
- 
-   ret = drm_fb_helper_remove_conflicting_framebuffers(ap, "inteldrmfb", 
primary);
- 
-   kfree(ap);
- 
-   return ret;
- }
- 
 +static void i915_driver_modeset_remove(struct drm_i915_private *i915)
 +{
 +  intel_modeset_driver_remove(i915);
 +
 +  intel_bios_driver_remove(i915);
 +
 +  i915_switcheroo_unregister(i915);
 +
 +  intel_vga_unregister(i915);
 +
 +  intel_csr_ucode_fini(i915);
 +}
 +
  static void intel_init_dpio(struct drm_i915_private *dev_priv)
  {
/*


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

linux-next: manual merge of the drm-misc tree with the drm tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the drm-misc tree got a conflict in:

  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c

between commit:

  8a9a982767b7 ("drm/i915: use a separate context for gpu relocs")

from the drm tree and commit:

  4ee92c7149da ("drm/mm: Convert drm_mm_node booleans to bitops")

from the drm-misc tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index e8ddc2320efa,493f07806b08..
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@@ -908,8 -902,7 +908,8 @@@ static void reloc_cache_init(struct rel
cache->use_64bit_reloc = HAS_64BIT_RELOC(i915);
cache->has_fence = cache->gen < 4;
cache->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
-   cache->node.allocated = false;
+   cache->node.flags = 0;
 +  cache->ce = NULL;
cache->rq = NULL;
cache->rq_size = 0;
  }


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

Re: [PATCH] drm/msm: Use the correct dma_sync calls harder

[Fabio Estevam]

Hi Rob,

On Tue, Oct 8, 2019 at 8:08 PM Rob Clark  wrote:

> afaict this should be at least in v5.4-rc2.. am I missing something?

You are right, it is in 5.4-rc indeed, sorry.

It is 5.3.x stable that has this commit missing, but I guess it will
be backported at some point.

Thanks!

Re: [PATCH] drm/msm: Use the correct dma_sync calls harder

[Rob Clark]

On Tue, Oct 8, 2019 at 9:11 AM Fabio Estevam  wrote:
>
> Hi Rob,
>
> On Wed, Sep 4, 2019 at 2:19 PM Rob Clark  wrote:
> >
> > From: Rob Clark 
> >
> > Looks like the dma_sync calls don't do what we want on armv7 either.
> > Fixes:
> >
> >   Unable to handle kernel paging request at virtual address 50001000
> >   pgd = (ptrval)
> >   [50001000] *pgd=
> >   Internal error: Oops: 805 [#1] SMP ARM
> >   Modules linked in:
> >   CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.3.0-rc6-00271-g9f159ae07f07 #4
> >   Hardware name: Freescale i.MX53 (Device Tree Support)
> >   PC is at v7_dma_clean_range+0x20/0x38
> >   LR is at __dma_page_cpu_to_dev+0x28/0x90
> >   pc : []lr : []psr: 2013
> >   sp : d80b5a88  ip : de96c000  fp : d840ce6c
> >   r10:   r9 : 0001  r8 : d843e010
> >   r7 :   r6 : 8000  r5 : ddb6c000  r4 : 
> >   r3 : 003f  r2 : 0040  r1 : 50008000  r0 : 50001000
> >   Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> >   Control: 10c5387d  Table: 70004019  DAC: 0051
> >   Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
> >
> > Signed-off-by: Rob Clark 
> > Fixes: 3de433c5b38a ("drm/msm: Use the correct dma_sync calls in msm_gem")
> > Tested-by: Fabio Estevam 
>
> I see this one got applied in linux-next already.
> Could it be sent to 5.4-rc, please?

afaict this should be at least in v5.4-rc2.. am I missing something?

BR,
-R

>
> mx53 boards cannot boot in mainline because of this.
>
> Thanks
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/panfrost: Quiet shrinker messages

[Rob Clark]

On Tue, Oct 8, 2019 at 2:46 PM Rob Herring  wrote:
>
> On Tue, Oct 8, 2019 at 10:13 AM Robin Murphy  wrote:
> >
> > As brought up on IRC, logging a vague and unattributed message for a
> > normal and expected operation looks a bit spammy. Use a dev_* variant
> > to clarify it as a driver message, and downgrade the level to debug to
> > avoid cluttering up end users' logs.
>
> If it was good enough for msm, it's not good enough for us?
>

I suppose there are differing opinions about shrinker spam.. I kinda
like to know when it is happening, OTOH it is a much more normal
situation on devices with less RAM or android..

is there a ratelimited version of dev_dbg(), though?  You might want that..

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

Re: [PATCH v2 00/21] drm/dp: Various helper improvements and cleanups

[Lyude Paul]

Hi! a couple people poked me to take a look at this, hopefully I can provide
some helpful insight here.

So: I've tried a _lot_ of various redesigns with MST and some portions of the
DP helpers. I actually was going to try to write up some common helpers for
handling link training across drivers, but when I started trying to implement
them (ironically, I think it was in i915!) I realized I wasn't getting rid of
nearly as much code as I thought I was going to.

Now-I'd love to tell you if this series is good or bad, but I'm not entirely
sure myself either. Sometimes I wonder myself if I'm overcomplicating things
with MST. The only way I've really found of figuring out whether or not
helpers like this are overkill is to just implement them everywhere. With my
atomic VCPI helpers for MST, I tried implementing them everywhere I could
(except for amdgpu, but I _did_ try there originally!) to see how awkward they
were to use. I think if there's questions to how useful this series is, it'd
probably be good to try implementing these helpers in drivers like i915 to see
how things play out.

It should also be noted that I did actually try to come up with common link
rate helpers like this one, but I ended up realizing I was adding far more
code then I was getting rid of after I tried implementing them in i915
(ironic, huh?). Things got even more complicated when I looked at how
nouveau/nvidia hardware does link training.

On Fri, 2019-09-20 at 18:00 +0200, Thierry Reding wrote:
> On Mon, Sep 02, 2019 at 01:31:00PM +0200, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > Hi,
> > 
> > this series of patches improves the DP helpers a bit and cleans up some
> > inconsistencies along the way.
> > 
> > v2 incorporates all review comments add collects Reviewed-bys from v1.
> > 
> > Thierry
> > 
> > Thierry Reding (21):
> >   drm/dp: Sort includes alphabetically
> >   drm/dp: Add missing kerneldoc for struct drm_dp_link
> >   drm/dp: Add drm_dp_link_reset() implementation
> >   drm/dp: Track link capabilities alongside settings
> >   drm/dp: Turn link capabilities into booleans
> >   drm/dp: Probe link using existing parsing helpers
> >   drm/dp: Read fast training capability from link
> >   drm/dp: Read TPS3 capability from sink
> >   drm/dp: Read channel coding capability from sink
> >   drm/dp: Read alternate scrambler reset capability from sink
> >   drm/dp: Read eDP version from DPCD
> >   drm/dp: Read AUX read interval from DPCD
> >   drm/dp: Do not busy-loop during link training
> >   drm/dp: Use drm_dp_aux_rd_interval()
> >   drm/dp: Add helper to get post-cursor adjustments
> >   drm/dp: Set channel coding on link configuration
> >   drm/dp: Enable alternate scrambler reset when supported
> >   drm/dp: Add drm_dp_link_choose() helper
> >   drm/dp: Add support for eDP link rates
> >   drm/dp: Remove a gratuituous blank line
> >   drm/bridge: tc358767: Use DP nomenclature
> 
> Anyone interested in reviewing these?
> 
> Thierry
> 
> >  drivers/gpu/drm/bridge/tc358767.c  |  22 +-
> >  drivers/gpu/drm/drm_dp_helper.c| 327 ++---
> >  drivers/gpu/drm/msm/edp/edp_ctrl.c |  12 +-
> >  drivers/gpu/drm/rockchip/cdn-dp-core.c |   8 +-
> >  drivers/gpu/drm/rockchip/cdn-dp-reg.c  |  13 +-
> >  drivers/gpu/drm/tegra/dpaux.c  |   8 +-
> >  drivers/gpu/drm/tegra/sor.c|  32 +--
> >  include/drm/drm_dp_helper.h| 124 +-
> >  8 files changed, 459 insertions(+), 87 deletions(-)
> > 
> > -- 
> > 2.22.0
> > 
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Cheers,
Lyude Paul

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

[PATCH v4 2/3] drm/rockchip: Add optional support for CRTC gamma LUT

[Ezequiel Garcia]

Add an optional CRTC gamma LUT support, and enable it on RK3288.
This is currently enabled via a separate address resource,
which needs to be specified in the devicetree.

The address resource is required because on some SoCs, such as
RK3288, the LUT address is after the MMU address, and the latter
is supported by a different driver. This prevents the DRM driver
from requesting an entire register space.

The current implementation works for RGB 10-bit tables, as that
is what seems to work on RK3288.

Signed-off-by: Ezequiel Garcia 
---
Changes from v3:
* Move to atomic_enable and atomic_begin,
  as discussed with Sean Paul.
* Dropped the Reviewed-bys.
Changes from v2:
* None.
Changes from v1:
* drop explicit linear LUT after finding a proper
  way to disable gamma correction.
* avoid setting gamma is the CRTC is not active.
* s/int/unsigned int as suggested by Jacopo.
* only enable color management and set gamma size
  if gamma LUT is supported, suggested by Doug.
* drop the reg-names usage, and instead just use indexed reg
  specifiers, suggested by Doug.
Changes from RFC:
* Request (an optional) address resource for the LUT.
* Drop support for RK3399, which doesn't seem to work
  out of the box and needs more research.
* Support pass-thru setting when GAMMA_LUT is NULL.
* Add a check for the gamma size, as suggested by Ilia.
* Move gamma setting to atomic_commit_tail, as pointed
  out by Jacopo/Laurent, is the correct way.
---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |   1 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 125 
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   5 +
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   2 +
 4 files changed, 133 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index ca01234c037c..697ee04b85cf 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -17,6 +17,7 @@
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_fb.h"
 #include "rockchip_drm_gem.h"
+#include "rockchip_drm_vop.h"
 
 static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
.destroy   = drm_gem_fb_destroy,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 613404f86668..85c1269a1218 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -139,6 +139,7 @@ struct vop {
 
uint32_t *regsbak;
void __iomem *regs;
+   void __iomem *lut_regs;
 
/* physical map length of vop register */
uint32_t len;
@@ -1048,6 +1049,84 @@ static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
return true;
 }
 
+static bool vop_dsp_lut_is_enable(struct vop *vop)
+{
+   return vop_read_reg(vop, 0, >data->common->dsp_lut_en);
+}
+
+static void vop_crtc_write_gamma_lut(struct vop *vop, struct drm_crtc *crtc)
+{
+   struct drm_color_lut *lut = crtc->state->gamma_lut->data;
+   unsigned int i;
+
+   for (i = 0; i < crtc->gamma_size; i++) {
+   u32 word;
+
+   word = (drm_color_lut_extract(lut[i].red, 10) << 20) |
+  (drm_color_lut_extract(lut[i].green, 10) << 10) |
+   drm_color_lut_extract(lut[i].blue, 10);
+   writel(word, vop->lut_regs + i * 4);
+   }
+}
+
+static void vop_crtc_gamma_set(struct vop *vop, struct drm_crtc *crtc,
+  struct drm_crtc_state *old_crtc_state)
+{
+   unsigned int idle;
+   int ret;
+
+   /*
+* In order to write the LUT to the internal memory,
+* we need to first make sure the dsp_lut_en bit is cleared.
+*/
+   spin_lock(>reg_lock);
+   VOP_REG_SET(vop, common, dsp_lut_en, 0);
+   vop_cfg_done(vop);
+   spin_unlock(>reg_lock);
+
+   /*
+* If the CRTC is not active, dsp_lut_en will not get cleared.
+* Apparently we still need to do the above step to for
+* gamma correction to be disabled.
+*/
+   if (!crtc->state->active)
+   return;
+
+   ret = readx_poll_timeout(vop_dsp_lut_is_enable, vop,
+idle, !idle, 5, 30 * 1000);
+   if (ret) {
+   DRM_DEV_ERROR(vop->dev, "display LUT RAM enable timeout!\n");
+   return;
+   }
+
+   if (crtc->state->gamma_lut &&
+  (!old_crtc_state->gamma_lut || (crtc->state->gamma_lut->base.id !=
+   old_crtc_state->gamma_lut->base.id))) {
+
+   spin_lock(>reg_lock);
+
+   vop_crtc_write_gamma_lut(vop, crtc);
+   VOP_REG_SET(vop, common, dsp_lut_en, 1);
+   vop_cfg_done(vop);
+
+   spin_unlock(>reg_lock);
+   }
+}
+
+static void vop_crtc_atomic_begin(struct drm_crtc *crtc,
+  struct drm_crtc_state *old_crtc_state)
+{
+   

[PATCH v4 0/3] RK3288 Gamma LUT

[Ezequiel Garcia]

Following Sean's comments, here's a new version.

On this new iteration, we modify the GAMMA LUT
on .atomic_enable and .atomic_begin.

With this change, the GAMMA settings are effectively
re-applied after resuming the machine, so the previous
patch "RFC: drm/atomic-helper: Reapply color transformation after
resume" is now dropped.

Also, I dropped Reviewed-bys tags on patch 2, given
the implementation is a bit different now.

Thanks!

Ezequiel Garcia (3):
  dt-bindings: display: rockchip: document VOP gamma LUT address
  drm/rockchip: Add optional support for CRTC gamma LUT
  ARM: dts: rockchip: Add RK3288 VOP gamma LUT address

 .../display/rockchip/rockchip-vop.txt |   6 +-
 arch/arm/boot/dts/rk3288.dtsi |   4 +-
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c|   1 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   | 125 ++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h   |   5 +
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c   |   2 +
 6 files changed, 140 insertions(+), 3 deletions(-)

-- 
2.22.0

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

[PATCH v4 3/3] ARM: dts: rockchip: Add RK3288 VOP gamma LUT address

[Ezequiel Garcia]

RK3288 SoC VOPs have optional support Gamma LUT setting,
which requires specifying the Gamma LUT address in the devicetree.

Signed-off-by: Ezequiel Garcia 
Reviewed-by: Douglas Anderson 
---
Changes from v3:
* None.
Changes from v2:
* None.
Changes from v1:
* Drop reg-names, as suggested by Doug.
---
 arch/arm/boot/dts/rk3288.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index cc893e154fe5..c6fc633ace80 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -1023,7 +1023,7 @@
 
vopb: vop@ff93 {
compatible = "rockchip,rk3288-vop";
-   reg = <0x0 0xff93 0x0 0x19c>;
+   reg = <0x0 0xff93 0x0 0x19c>, <0x0 0xff931000 0x0 0x1000>;
interrupts = ;
clocks = < ACLK_VOP0>, < DCLK_VOP0>, < HCLK_VOP0>;
clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
@@ -1073,7 +1073,7 @@
 
vopl: vop@ff94 {
compatible = "rockchip,rk3288-vop";
-   reg = <0x0 0xff94 0x0 0x19c>;
+   reg = <0x0 0xff94 0x0 0x19c>, <0x0 0xff941000 0x0 0x1000>;
interrupts = ;
clocks = < ACLK_VOP1>, < DCLK_VOP1>, < HCLK_VOP1>;
clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
-- 
2.22.0

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

[PATCH v4 1/3] dt-bindings: display: rockchip: document VOP gamma LUT address

[Ezequiel Garcia]

Add the register specifier description for an
optional gamma LUT address.

Signed-off-by: Ezequiel Garcia 
Reviewed-by: Douglas Anderson 
Reviewed-by: Rob Herring 
---
Changes from v3:
* None.
Changes from v2:
* None.
Changes from v1:
* Drop reg-names, suggested by Doug.
---
 .../devicetree/bindings/display/rockchip/rockchip-vop.txt   | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git 
a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt 
b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt
index 4f58c5a2d195..8b3a5f514205 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt
@@ -20,6 +20,10 @@ Required properties:
"rockchip,rk3228-vop";
"rockchip,rk3328-vop";
 
+- reg: Must contain one entry corresponding to the base address and length
+   of the register space. Can optionally contain a second entry
+   corresponding to the CRTC gamma LUT address.
+
 - interrupts: should contain a list of all VOP IP block interrupts in the
 order: VSYNC, LCD_SYSTEM. The interrupt specifier
 format depends on the interrupt controller used.
@@ -48,7 +52,7 @@ Example:
 SoC specific DT entry:
vopb: vopb@ff93 {
compatible = "rockchip,rk3288-vop";
-   reg = <0xff93 0x19c>;
+   reg = <0x0 0xff93 0x0 0x19c>, <0x0 0xff931000 0x0 0x1000>;
interrupts = ;
clocks = < ACLK_VOP0>, < DCLK_VOP0>, < HCLK_VOP0>;
clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
-- 
2.22.0

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

Re: [PATCH v3 3/5] drm/rockchip: Add optional support for CRTC gamma LUT

[Ezequiel Garcia]

On Tue, 2019-10-08 at 16:03 -0400, Sean Paul wrote:
> On Tue, Oct 08, 2019 at 04:33:35PM -0300, Ezequiel Garcia wrote:
> > On Tue, 2019-10-08 at 16:23 -0300, Ezequiel Garcia wrote:
> > > Hello Sean,
> > > 
> > > On Mon, 2019-10-07 at 14:54 -0400, Sean Paul wrote:
> > > > On Mon, Sep 30, 2019 at 07:28:00PM -0300, Ezequiel Garcia wrote:
> > > > > Add an optional CRTC gamma LUT support, and enable it on RK3288.
> > > > > This is currently enabled via a separate address resource,
> > > > > which needs to be specified in the devicetree.
> > > > > 
> > > > > The address resource is required because on some SoCs, such as
> > > > > RK3288, the LUT address is after the MMU address, and the latter
> > > > > is supported by a different driver. This prevents the DRM driver
> > > > > from requesting an entire register space.
> > > > > 
> > > > > The current implementation works for RGB 10-bit tables, as that
> > > > > is what seems to work on RK3288.
> > > > > 
> > > > > Signed-off-by: Ezequiel Garcia 
> > > > > Reviewed-by: Douglas Anderson 
> > > > > Reviewed-by: Jacopo Mondi 
> > > > > ---
> > > > > Changes from v2:
> > > > > * None.
> > > > > 
> > > > > Changes from v1:
> > > > > * drop explicit linear LUT after finding a proper
> > > > >   way to disable gamma correction.
> > > > > * avoid setting gamma is the CRTC is not active.
> > > > > * s/int/unsigned int as suggested by Jacopo.
> > > > > * only enable color management and set gamma size
> > > > >   if gamma LUT is supported, suggested by Doug.
> > > > > * drop the reg-names usage, and instead just use indexed reg
> > > > >   specifiers, suggested by Doug.
> > > > > 
> > > > > Changes from RFC:
> > > > > * Request (an optional) address resource for the LUT.
> > > > > * Drop support for RK3399, which doesn't seem to work
> > > > >   out of the box and needs more research.
> > > > > * Support pass-thru setting when GAMMA_LUT is NULL.
> > > > > * Add a check for the gamma size, as suggested by Ilia.
> > > > > * Move gamma setting to atomic_commit_tail, as pointed
> > > > >   out by Jacopo/Laurent, is the correct way.
> > > > > ---
> > > > >  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |   3 +
> > > > >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 114 
> > > > > 
> > > > >  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   7 ++
> > > > >  drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   2 +
> > > > >  4 files changed, 126 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
> > > > > b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > > > > index dba352ec0ee3..fd1d987698ab 100644
> > > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > > > > @@ -17,6 +17,7 @@
> > > > >  #include "rockchip_drm_drv.h"
> > > > >  #include "rockchip_drm_fb.h"
> > > > >  #include "rockchip_drm_gem.h"
> > > > > +#include "rockchip_drm_vop.h"
> > > > >  
> > > > >  static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
> > > > >   .destroy   = drm_gem_fb_destroy,
> > > > > @@ -112,6 +113,8 @@ rockchip_atomic_helper_commit_tail_rpm(struct 
> > > > > drm_atomic_state *old_state)
> > > > >  
> > > > >   drm_atomic_helper_commit_modeset_disables(dev, old_state);
> > > > >  
> > > > > + rockchip_drm_vop_gamma_set(old_state);
> > > > > +
> > > > 
> > > > Instead of duplicating the commit_tail helper, could you just implement
> > > > .atomic_begin() and call this from there? I think the only hitch is if 
> > > > you
> > > > need this to be completed before crtc->atomic_enable(), at which point 
> > > > you
> > > > might need to call it from vop_crtc_atomic_enable() and then detect 
> > > > that in
> > > > atomic_begin()
> > > > 
> > > 
> > > I think moving this to .atomic_begin might be enough. Let me send a new
> > > series and we can see how that goes.
> > > 
> > 
> > Oh, before going forward, pleaste note that the first iteration
> > of this patch (as noted in the changelog) was applying the gamma lut
> > on .atomic_flush. However, Laurent and Jacopo pointed out that
> > it might add some tearing to do so, and that's why it was moved
> > to commit_tail.
> > 
> > I have to admit I'm not too sure about the difference between
> > applying this gamma LUT on atomic_begin or atomic_flush,
> > perhaps you can clarify that?
> 
> The only difference between what you have now and calling it in atomic_begin
> is that as you have it now, it's set before crtc->atomic_enable() is called.
> I think in order to address Ville's concerns on the other patch, you'll need
> to set it the lut in .atomic_enable() anyways, so here's what I would suggest:
> 
> - Set the LUT in .atomic_enable() wherever it makes sense (you have it at the
>   start now)
> - Add an .atomic_begin() implementation and check state->color_mgmt_changed 
> and
>   state->active_changed. color_mgmt_changed && !active_changed, set the lut
> - Remove patches 1 & 5
> 
> ...I think :-)
> 

OK, that 

Re: [PATCH] drm/panfrost: Quiet shrinker messages

[Rob Herring]

On Tue, Oct 8, 2019 at 10:13 AM Robin Murphy  wrote:
>
> As brought up on IRC, logging a vague and unattributed message for a
> normal and expected operation looks a bit spammy. Use a dev_* variant
> to clarify it as a driver message, and downgrade the level to debug to
> avoid cluttering up end users' logs.

If it was good enough for msm, it's not good enough for us?

> Signed-off-by: Robin Murphy 
> ---
>  drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/14] drm/amd/display: Recalculate VCPI slots for new DSC connectors

[Mikita Lipski]

On 08.10.2019 12:24, Lyude Paul wrote:
> ...
> yikes
> I need to apologize because I was going through my email and I realized you
> _did_ respond to me earlier regarding some of these questions, it just appears
> the reply fell through the cracks and somehow I didn't notice it until just
> now. Sorry about that!
> 
No worries, the patches got messy and are easily lost in here. I'll 
clean up on the next batch so it will be clearer what's happening

> I'm still not sure I understand why this is a future enhancement? Everything
> we need to implement these helpers should already be here, it's just a matter
> of keeping track who has dsc enabled where in the mst atomic state

As I've mentioned earlier our policy for DSC is to keep it disabled if 
not needed, because it is a lossy compression. Beside the fact that we 
don't want imperfect image on the screen, enabling DSC would also 
increase our power consumption. If we need it - we use the  greedy 
algorithm inside compute_mst_dsc_configs_for_link to only enable DSC on 
streams that can't fit into allowed bandwidth without compression.

I'm not exactly sure how it can be moved as helper to DRM.
We keep track on which stream has DSC enabled by raising DSC flag in 
dc_stream_state flags structure.
That it why pbn recalculation was moved to a separate function so only 
streams that have DSC flag raised get VCPI recalculated.

I guess if we would have dsc_desired flag on drm_connnector_state and it 
would then perform recalculation of PBN and VCPI for the connector. 
Would that be something applicable as a DRM helper?


> On Mon, 2019-10-07 at 17:52 -0400, Lyude Paul wrote:
>> Ok, let's stop and slow down for a minute here since I've repeated myself a
>> few times, and I'd like to figure out what's actually happening here and
>> whether I'm not being clear enough with my explanations, or if there's just
>> some misunderstanding here.
>>
>> I'll start of by trying my best to explain my hesistance in accepting these
>> patches. To be clear, MST with DSC is a big thing in terms of impact.
>> There's
>> a lot of things to consider:
>>   * What should the default DSC policy for MST be? Should we keep it on by
>> default so that we don't need to trigger a large number of PBN
>> recalculations and enable DSC on a per-case basis, or are there
>> legitimate
>> reasons to keep DSC off by default (such as potential issues with
>> lossiness
>> down the line).
>>   * We have other drivers in the tree that are planning on implementing MST
>> DSC
>> quite soon. Chances are they'll be implementing helpers to do this so
>> this
>> can be supported across the DRM tree, and chances are we'll just have to
>> rewrite and remove most of this code in that case once they do.
>>   * amdgpu already has a _lot_ of code that doesn't need to, and shouldn't be
>> there. This ranges from various DC specific helpers that are essentially
>> the same as the helpers that we already implement in the rest of DRM, to
>> misuses of various kernel APIs, and a complete lack of any kind of code
>> style (at least the last time that I checked) in or out of DC. This makes
>> the driver more difficult for people who don't work at AMD but are well
>> accustomed to working on the rest of the kernel, e.g. myself and others,
>> and it's a problem that seriously needs to be solved. Adding more
>> redundant
>> DP helpers that will inevitably get removed is just making the problem
>> worse.
>>
>> With all of this being said: I've been asking the same questions about this
>> patch throughout the whole patch series so far (since it got passed off to
>> you
>> from David's internship ending):
>>
>>   * Can we keep DSC enabled by default, so that these patches aren't needed?
>>   * If we can't, can we move this into common code so that other drivers can
>> use it?
>> The problem is I haven't received any responses to these questions, other
>> then
>> being told by David a month or two ago that it would be expedient to just
>> accept the patches and move on. Honestly, if discussion had been had about
>> the
>> changes I requested, I would have given my r-b a long time ago. In fact-I
>> really want to give it now! There's multiple patches in this series that
>> would
>> be very useful for other things that are being worked on at the moment, such
>> as the changes to drm_dp_dpcd_read/drm_dp_dpcd_write(). But I really think
>> this needs to be discussed first, and I'm worried that just going ahead and
>> giving my R-B before they have been discussed will further encourage rushing
>> changes like this in the future and continually building more tech debt for
>> ourselves.
>>
>> Please note as well: if anything I've asked for is confusing, or you don't
>> understand what I'm asking or looking for I am more then willing to help
>> explain things and help out as best as I can. I understand that a lot of the
>> developers working on DRM at AMD may have 

[Bug 111933] driconf-0.9.1-r2 - assert iter or allowNone

[bugzilla-daemon]

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

Bug ID: 111933
   Summary: driconf-0.9.1-r2 - assert iter or allowNone
   Product: DRI
   Version: XOrg git
  Hardware: Other
OS: All
Status: NEW
  Severity: not set
  Priority: not set
 Component: General
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: mmokr...@gmail.com

$ rm -rf ~/.driconf
$ driconf

[click on Reload selected configuration file]

Traceback (most recent call last):
  File "/usr/lib/driconf/driconf_complexui.py", line 699, in reloadConfig
node = self.getSelection()
  File "/usr/lib/driconf/driconf_complexui.py", line 551, in getSelection
assert iter or allowNone
AssertionError

-- 
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 RESEND] gpu: drm: bridge: sii9234: convert to devm_i2c_new_dummy_device

[Wolfram Sang]

Move from the deprecated i2c_new_dummy() to devm_i2c_new_dummy_device().
We now get an ERRPTR which we use in error handling and we can skip
removal of the created devices.

Signed-off-by: Wolfram Sang 
---

Rebased to v5.4-rc2 since last time. One of the last two users of the
old API, so please apply soon, so I can remove the old interface. Only
build tested.

 drivers/gpu/drm/bridge/sii9234.c | 36 +++-
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
index 25d4ad8c7ad6..8a6c85693a88 100644
--- a/drivers/gpu/drm/bridge/sii9234.c
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -841,39 +841,28 @@ static int sii9234_init_resources(struct sii9234 *ctx,
 
ctx->client[I2C_MHL] = client;
 
-   ctx->client[I2C_TPI] = i2c_new_dummy(adapter, I2C_TPI_ADDR);
-   if (!ctx->client[I2C_TPI]) {
+   ctx->client[I2C_TPI] = devm_i2c_new_dummy_device(>dev, adapter,
+I2C_TPI_ADDR);
+   if (IS_ERR(ctx->client[I2C_TPI])) {
dev_err(ctx->dev, "failed to create TPI client\n");
-   return -ENODEV;
+   return PTR_ERR(ctx->client[I2C_TPI]);
}
 
-   ctx->client[I2C_HDMI] = i2c_new_dummy(adapter, I2C_HDMI_ADDR);
-   if (!ctx->client[I2C_HDMI]) {
+   ctx->client[I2C_HDMI] = devm_i2c_new_dummy_device(>dev, adapter,
+ I2C_HDMI_ADDR);
+   if (IS_ERR(ctx->client[I2C_HDMI])) {
dev_err(ctx->dev, "failed to create HDMI RX client\n");
-   goto fail_tpi;
+   return PTR_ERR(ctx->client[I2C_HDMI]);
}
 
-   ctx->client[I2C_CBUS] = i2c_new_dummy(adapter, I2C_CBUS_ADDR);
-   if (!ctx->client[I2C_CBUS]) {
+   ctx->client[I2C_CBUS] = devm_i2c_new_dummy_device(>dev, adapter,
+ I2C_CBUS_ADDR);
+   if (IS_ERR(ctx->client[I2C_CBUS])) {
dev_err(ctx->dev, "failed to create CBUS client\n");
-   goto fail_hdmi;
+   return PTR_ERR(ctx->client[I2C_CBUS]);
}
 
return 0;
-
-fail_hdmi:
-   i2c_unregister_device(ctx->client[I2C_HDMI]);
-fail_tpi:
-   i2c_unregister_device(ctx->client[I2C_TPI]);
-
-   return -ENODEV;
-}
-
-static void sii9234_deinit_resources(struct sii9234 *ctx)
-{
-   i2c_unregister_device(ctx->client[I2C_CBUS]);
-   i2c_unregister_device(ctx->client[I2C_HDMI]);
-   i2c_unregister_device(ctx->client[I2C_TPI]);
 }
 
 static inline struct sii9234 *bridge_to_sii9234(struct drm_bridge *bridge)
@@ -950,7 +939,6 @@ static int sii9234_remove(struct i2c_client *client)
 
sii9234_cable_out(ctx);
drm_bridge_remove(>bridge);
-   sii9234_deinit_resources(ctx);
 
return 0;
 }
-- 
2.20.1

[Bug 111932] driconf: TypeError: cannot concatenate 'str' and 'int' objects

[bugzilla-daemon]

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

Bug ID: 111932
   Summary: driconf: TypeError: cannot concatenate 'str' and 'int'
objects
   Product: DRI
   Version: XOrg git
  Hardware: Other
OS: All
Status: NEW
  Severity: not set
  Priority: not set
 Component: General
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: mmokr...@gmail.com

$ driconf --version
Screen "0" is not direct rendering capable.
MESA-LOADER: failed to open radeon (search paths /usr/lib64/dri)
Traceback (most recent call last):
  File "/usr/lib/python-exec/python2.7/driconf", line 28, in 
driconf.main()
  File "/usr/lib/driconf/driconf.py", line 140, in main
complexui.start(configList)
  File "/usr/lib/driconf/driconf_complexui.py", line 1040, in start
mainWindow = MainWindow(configList)
  File "/usr/lib/driconf/driconf_complexui.py", line 830, in __init__
self.configTree = ConfigTreeView (configList)
  File "/usr/lib/driconf/driconf_complexui.py", line 525, in __init__
self.model = ConfigTreeModel (configList)
  File "/usr/lib/driconf/driconf_complexui.py", line 262, in __init__
self.addNode (config)
  File "/usr/lib/driconf/driconf_complexui.py", line 450, in addNode
self.initNode (node)
  File "/usr/lib/driconf/driconf_complexui.py", line 469, in initNode
self.initNode (device)
  File "/usr/lib/driconf/driconf_complexui.py", line 472, in initNode
self.initNode (app)
  File "/usr/lib/driconf/driconf_complexui.py", line 474, in initNode
self.validateAppNode (node)
  File "/usr/lib/driconf/driconf_complexui.py", line 515, in validateAppNode
driver = app.device.getDriver(commonui.dpy)
  File "/usr/lib/driconf/dri.py", line 490, in getDriver
driver = GetDriver (self.driver)
  File "/usr/lib/driconf/dri.py", line 430, in GetDriver
driver = DriverInfo (name)
  File "/usr/lib/driconf/dri.py", line 292, in __init__
driInfo = XDriInfo ("options " + name)
  File "/usr/lib/driconf/dri.py", line 54, in XDriInfo
raise DRIError ("XDriInfo killed by signal " + signal + ".")
TypeError: cannot concatenate 'str' and 'int' objects
$ equery belongs driconf
 * Searching for driconf ... 
x11-misc/driconf-0.9.1-r2 (/usr/share/driconf)
x11-misc/driconf-0.9.1-r2 (/usr/lib/python-exec/python2.7/driconf)
x11-misc/driconf-0.9.1-r2 (/usr/bin/driconf -> ../lib/python-exec/python-exec2)
x11-misc/driconf-0.9.1-r2 (/usr/lib/driconf)
$

This is a Gentoo Linux. Actually the '--version' argument does not matter, it
crashes even without it.

-- 
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 RESEND] gpu: drm: bridge: analogix-anx78xx: convert to i2c_new_dummy_device

[Wolfram Sang]

Move from the deprecated i2c_new_dummy() to i2c_new_dummy_device(). We
now get an ERRPTR which we use in error handling.

Signed-off-by: Wolfram Sang 
---

Rebased to v5.4-rc2 since last time. One of the last two users of the
old API, so please apply soon, so I can remove the old interface. Only
build tested.

 drivers/gpu/drm/bridge/analogix-anx78xx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c 
b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index 3c7cc5af735c..be7756280e41 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1350,10 +1350,10 @@ static int anx78xx_i2c_probe(struct i2c_client *client,
 
/* Map slave addresses of ANX7814 */
for (i = 0; i < I2C_NUM_ADDRESSES; i++) {
-   anx78xx->i2c_dummy[i] = i2c_new_dummy(client->adapter,
+   anx78xx->i2c_dummy[i] = i2c_new_dummy_device(client->adapter,
anx78xx_i2c_addresses[i] >> 1);
-   if (!anx78xx->i2c_dummy[i]) {
-   err = -ENOMEM;
+   if (IS_ERR(anx78xx->i2c_dummy[i])) {
+   err = PTR_ERR(anx78xx->i2c_dummy[i]);
DRM_ERROR("Failed to reserve I2C bus %02x\n",
  anx78xx_i2c_addresses[i]);
goto err_unregister_i2c;
-- 
2.20.1

Uninitialized Variable Use in video: fbdev: pm3fb

[Yizhuo Zhai]

Hi All:

drivers/video/fbdev/pm3fb.c:

Inside function pm3fb_write_mode(),  variable "m" "n" "p" could leave
uninitialized after pm3fb_calculate_clock(), however, they are used
later in PM3_WRITE_DAC_REG, which is potentially unsafe.


-- 
Kind Regards,

Yizhuo Zhai

Computer Science, Graduate Student
University of California, Riverside

[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=205089

--- Comment #5 from Alex Deucher (alexdeuc...@gmail.com) ---
If you could come up with a reproducible test case, that would help for
tracking down why it's hanging in the first place.

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

[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=205089

--- Comment #4 from Bruno Jacquet (maxi...@free.fr) ---
Okay, I got this, but should I investigate the initial GPU reset cause?

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

Re: [PATCH v3 3/5] drm/rockchip: Add optional support for CRTC gamma LUT

[Sean Paul]

On Tue, Oct 08, 2019 at 04:33:35PM -0300, Ezequiel Garcia wrote:
> On Tue, 2019-10-08 at 16:23 -0300, Ezequiel Garcia wrote:
> > Hello Sean,
> > 
> > On Mon, 2019-10-07 at 14:54 -0400, Sean Paul wrote:
> > > On Mon, Sep 30, 2019 at 07:28:00PM -0300, Ezequiel Garcia wrote:
> > > > Add an optional CRTC gamma LUT support, and enable it on RK3288.
> > > > This is currently enabled via a separate address resource,
> > > > which needs to be specified in the devicetree.
> > > > 
> > > > The address resource is required because on some SoCs, such as
> > > > RK3288, the LUT address is after the MMU address, and the latter
> > > > is supported by a different driver. This prevents the DRM driver
> > > > from requesting an entire register space.
> > > > 
> > > > The current implementation works for RGB 10-bit tables, as that
> > > > is what seems to work on RK3288.
> > > > 
> > > > Signed-off-by: Ezequiel Garcia 
> > > > Reviewed-by: Douglas Anderson 
> > > > Reviewed-by: Jacopo Mondi 
> > > > ---
> > > > Changes from v2:
> > > > * None.
> > > > 
> > > > Changes from v1:
> > > > * drop explicit linear LUT after finding a proper
> > > >   way to disable gamma correction.
> > > > * avoid setting gamma is the CRTC is not active.
> > > > * s/int/unsigned int as suggested by Jacopo.
> > > > * only enable color management and set gamma size
> > > >   if gamma LUT is supported, suggested by Doug.
> > > > * drop the reg-names usage, and instead just use indexed reg
> > > >   specifiers, suggested by Doug.
> > > > 
> > > > Changes from RFC:
> > > > * Request (an optional) address resource for the LUT.
> > > > * Drop support for RK3399, which doesn't seem to work
> > > >   out of the box and needs more research.
> > > > * Support pass-thru setting when GAMMA_LUT is NULL.
> > > > * Add a check for the gamma size, as suggested by Ilia.
> > > > * Move gamma setting to atomic_commit_tail, as pointed
> > > >   out by Jacopo/Laurent, is the correct way.
> > > > ---
> > > >  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |   3 +
> > > >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 114 
> > > >  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   7 ++
> > > >  drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   2 +
> > > >  4 files changed, 126 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
> > > > b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > > > index dba352ec0ee3..fd1d987698ab 100644
> > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > > > @@ -17,6 +17,7 @@
> > > >  #include "rockchip_drm_drv.h"
> > > >  #include "rockchip_drm_fb.h"
> > > >  #include "rockchip_drm_gem.h"
> > > > +#include "rockchip_drm_vop.h"
> > > >  
> > > >  static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
> > > > .destroy   = drm_gem_fb_destroy,
> > > > @@ -112,6 +113,8 @@ rockchip_atomic_helper_commit_tail_rpm(struct 
> > > > drm_atomic_state *old_state)
> > > >  
> > > > drm_atomic_helper_commit_modeset_disables(dev, old_state);
> > > >  
> > > > +   rockchip_drm_vop_gamma_set(old_state);
> > > > +
> > > 
> > > Instead of duplicating the commit_tail helper, could you just implement
> > > .atomic_begin() and call this from there? I think the only hitch is if you
> > > need this to be completed before crtc->atomic_enable(), at which point you
> > > might need to call it from vop_crtc_atomic_enable() and then detect that 
> > > in
> > > atomic_begin()
> > > 
> > 
> > I think moving this to .atomic_begin might be enough. Let me send a new
> > series and we can see how that goes.
> > 
> 
> Oh, before going forward, pleaste note that the first iteration
> of this patch (as noted in the changelog) was applying the gamma lut
> on .atomic_flush. However, Laurent and Jacopo pointed out that
> it might add some tearing to do so, and that's why it was moved
> to commit_tail.
> 
> I have to admit I'm not too sure about the difference between
> applying this gamma LUT on atomic_begin or atomic_flush,
> perhaps you can clarify that?

The only difference between what you have now and calling it in atomic_begin
is that as you have it now, it's set before crtc->atomic_enable() is called.
I think in order to address Ville's concerns on the other patch, you'll need
to set it the lut in .atomic_enable() anyways, so here's what I would suggest:

- Set the LUT in .atomic_enable() wherever it makes sense (you have it at the
  start now)
- Add an .atomic_begin() implementation and check state->color_mgmt_changed and
  state->active_changed. color_mgmt_changed && !active_changed, set the lut
- Remove patches 1 & 5

...I think :-)

Sean

> 
> Thanks!
> Ezequiel 
> 
> 

-- 
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 v9 4/5] dt-bindings: backlight: Add led-backlight binding

[Rob Herring]

On Tue, Oct 8, 2019 at 12:17 PM Jacek Anaszewski
 wrote:
>
> On 10/8/19 5:00 PM, Rob Herring wrote:
> > On Tue, Oct 8, 2019 at 8:30 AM Jean-Jacques Hiblot  wrote:
> >>
> >> Rob,
> >>
> >> On 08/10/2019 14:51, Jean-Jacques Hiblot wrote:
> >>> Hi Rob,
> >>>
> >>> On 07/10/2019 18:15, Rob Herring wrote:
>  Please send DT bindings to DT list or it's never in my queue. IOW,
>  send patches to the lists that get_maintainers.pl tells you to.
> 
>  On Mon, Oct 7, 2019 at 7:45 AM Jean-Jacques Hiblot 
>  wrote:
> > Add DT binding for led-backlight.
> >
> > Signed-off-by: Jean-Jacques Hiblot 
> > Reviewed-by: Daniel Thompson 
> > Reviewed-by: Sebastian Reichel 
> > ---
> >   .../bindings/leds/backlight/led-backlight.txt | 28
> > +++
> >   1 file changed, 28 insertions(+)
> >   create mode 100644
> > Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
>  Please make this a DT schema.
> >>>
> >>> OK.
> >>>
> >>> BTW I used "make dt_binding_check" but had to fix a couple of YAMLs
> >>> file to get it to work. Do you have a kernel tree with already all the
> >>> YAML files in good shape ? Or do you want me to post the changes to
> >>> devicet...@vger.kernel.org ?
> >>>
> >>>
> 
> > diff --git
> > a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> > b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> > new file mode 100644
> > index ..4c7dfbe7f67a
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> > @@ -0,0 +1,28 @@
> > +led-backlight bindings
> > +
> > +This binding is used to describe a basic backlight device made of
> > LEDs.
> > +It can also be used to describe a backlight device controlled by
> > the output of
> > +a LED driver.
> > +
> > +Required properties:
> > +  - compatible: "led-backlight"
> > +  - leds: a list of LEDs
>  'leds' is already used as a node name and mixing is not ideal.
>
> for the record: child node names (if that was what you had on mind)
> have singular form 'led'.

I did actually grep this and not rely on my somewhat faulty memory:

$ git grep '\sleds {' | wc -l
463

These are mostly gpio-leds I think.

Rob

Re: [PATCH v8 0/5] Add a generic driver for LED-based backlight

[Pavel Machek]

On Thu 2019-10-03 12:40:28, Lee Jones wrote:
> On Thu, 03 Oct 2019, Jean-Jacques Hiblot wrote:
> 
> > This series aims to add a led-backlight driver, similar to pwm-backlight,
> > but using a LED class device underneath.
> > 
> > A few years ago (2015), Tomi Valkeinen posted a series implementing a
> > backlight driver on top of a LED device:
> > https://patchwork.kernel.org/patch/7293991/
> > https://patchwork.kernel.org/patch/7294001/
> > https://patchwork.kernel.org/patch/7293981/
> > 
> > The discussion stopped because Tomi lacked the time to work on it.
> 
> [...]
> 
> >  .../bindings/leds/backlight/led-backlight.txt |  28 ++
> >  drivers/leds/led-class.c  |  98 ++-
> >  drivers/video/backlight/Kconfig   |   7 +
> >  drivers/video/backlight/Makefile  |   1 +
> >  drivers/video/backlight/led_bl.c  | 260 ++
> >  include/linux/leds.h  |   6 +
> >  6 files changed, 399 insertions(+), 1 deletion(-)
> >  create mode 100644 
> > Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> >  create mode 100644 drivers/video/backlight/led_bl.c
> 
> How should this set be processed?  I'm happy to take it through
> Backlight via an immutable branch and PR to be consumed by LED.

That would make sense to me.

For the record, series is Tested-by: Pavel Machek  .

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


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

Re: [PATCH v3 3/5] drm/rockchip: Add optional support for CRTC gamma LUT

[Ezequiel Garcia]

On Tue, 2019-10-08 at 16:23 -0300, Ezequiel Garcia wrote:
> Hello Sean,
> 
> On Mon, 2019-10-07 at 14:54 -0400, Sean Paul wrote:
> > On Mon, Sep 30, 2019 at 07:28:00PM -0300, Ezequiel Garcia wrote:
> > > Add an optional CRTC gamma LUT support, and enable it on RK3288.
> > > This is currently enabled via a separate address resource,
> > > which needs to be specified in the devicetree.
> > > 
> > > The address resource is required because on some SoCs, such as
> > > RK3288, the LUT address is after the MMU address, and the latter
> > > is supported by a different driver. This prevents the DRM driver
> > > from requesting an entire register space.
> > > 
> > > The current implementation works for RGB 10-bit tables, as that
> > > is what seems to work on RK3288.
> > > 
> > > Signed-off-by: Ezequiel Garcia 
> > > Reviewed-by: Douglas Anderson 
> > > Reviewed-by: Jacopo Mondi 
> > > ---
> > > Changes from v2:
> > > * None.
> > > 
> > > Changes from v1:
> > > * drop explicit linear LUT after finding a proper
> > >   way to disable gamma correction.
> > > * avoid setting gamma is the CRTC is not active.
> > > * s/int/unsigned int as suggested by Jacopo.
> > > * only enable color management and set gamma size
> > >   if gamma LUT is supported, suggested by Doug.
> > > * drop the reg-names usage, and instead just use indexed reg
> > >   specifiers, suggested by Doug.
> > > 
> > > Changes from RFC:
> > > * Request (an optional) address resource for the LUT.
> > > * Drop support for RK3399, which doesn't seem to work
> > >   out of the box and needs more research.
> > > * Support pass-thru setting when GAMMA_LUT is NULL.
> > > * Add a check for the gamma size, as suggested by Ilia.
> > > * Move gamma setting to atomic_commit_tail, as pointed
> > >   out by Jacopo/Laurent, is the correct way.
> > > ---
> > >  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |   3 +
> > >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 114 
> > >  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   7 ++
> > >  drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   2 +
> > >  4 files changed, 126 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
> > > b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > > index dba352ec0ee3..fd1d987698ab 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > > @@ -17,6 +17,7 @@
> > >  #include "rockchip_drm_drv.h"
> > >  #include "rockchip_drm_fb.h"
> > >  #include "rockchip_drm_gem.h"
> > > +#include "rockchip_drm_vop.h"
> > >  
> > >  static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
> > >   .destroy   = drm_gem_fb_destroy,
> > > @@ -112,6 +113,8 @@ rockchip_atomic_helper_commit_tail_rpm(struct 
> > > drm_atomic_state *old_state)
> > >  
> > >   drm_atomic_helper_commit_modeset_disables(dev, old_state);
> > >  
> > > + rockchip_drm_vop_gamma_set(old_state);
> > > +
> > 
> > Instead of duplicating the commit_tail helper, could you just implement
> > .atomic_begin() and call this from there? I think the only hitch is if you
> > need this to be completed before crtc->atomic_enable(), at which point you
> > might need to call it from vop_crtc_atomic_enable() and then detect that in
> > atomic_begin()
> > 
> 
> I think moving this to .atomic_begin might be enough. Let me send a new
> series and we can see how that goes.
> 

Oh, before going forward, pleaste note that the first iteration
of this patch (as noted in the changelog) was applying the gamma lut
on .atomic_flush. However, Laurent and Jacopo pointed out that
it might add some tearing to do so, and that's why it was moved
to commit_tail.

I have to admit I'm not too sure about the difference between
applying this gamma LUT on atomic_begin or atomic_flush,
perhaps you can clarify that?

Thanks!
Ezequiel 


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

Re: [PATCH v3 3/5] drm/rockchip: Add optional support for CRTC gamma LUT

[Ezequiel Garcia]

Hello Sean,

On Mon, 2019-10-07 at 14:54 -0400, Sean Paul wrote:
> On Mon, Sep 30, 2019 at 07:28:00PM -0300, Ezequiel Garcia wrote:
> > Add an optional CRTC gamma LUT support, and enable it on RK3288.
> > This is currently enabled via a separate address resource,
> > which needs to be specified in the devicetree.
> > 
> > The address resource is required because on some SoCs, such as
> > RK3288, the LUT address is after the MMU address, and the latter
> > is supported by a different driver. This prevents the DRM driver
> > from requesting an entire register space.
> > 
> > The current implementation works for RGB 10-bit tables, as that
> > is what seems to work on RK3288.
> > 
> > Signed-off-by: Ezequiel Garcia 
> > Reviewed-by: Douglas Anderson 
> > Reviewed-by: Jacopo Mondi 
> > ---
> > Changes from v2:
> > * None.
> > 
> > Changes from v1:
> > * drop explicit linear LUT after finding a proper
> >   way to disable gamma correction.
> > * avoid setting gamma is the CRTC is not active.
> > * s/int/unsigned int as suggested by Jacopo.
> > * only enable color management and set gamma size
> >   if gamma LUT is supported, suggested by Doug.
> > * drop the reg-names usage, and instead just use indexed reg
> >   specifiers, suggested by Doug.
> > 
> > Changes from RFC:
> > * Request (an optional) address resource for the LUT.
> > * Drop support for RK3399, which doesn't seem to work
> >   out of the box and needs more research.
> > * Support pass-thru setting when GAMMA_LUT is NULL.
> > * Add a check for the gamma size, as suggested by Ilia.
> > * Move gamma setting to atomic_commit_tail, as pointed
> >   out by Jacopo/Laurent, is the correct way.
> > ---
> >  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |   3 +
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 114 
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   7 ++
> >  drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   2 +
> >  4 files changed, 126 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
> > b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > index dba352ec0ee3..fd1d987698ab 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> > @@ -17,6 +17,7 @@
> >  #include "rockchip_drm_drv.h"
> >  #include "rockchip_drm_fb.h"
> >  #include "rockchip_drm_gem.h"
> > +#include "rockchip_drm_vop.h"
> >  
> >  static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
> > .destroy   = drm_gem_fb_destroy,
> > @@ -112,6 +113,8 @@ rockchip_atomic_helper_commit_tail_rpm(struct 
> > drm_atomic_state *old_state)
> >  
> > drm_atomic_helper_commit_modeset_disables(dev, old_state);
> >  
> > +   rockchip_drm_vop_gamma_set(old_state);
> > +
> 
> Instead of duplicating the commit_tail helper, could you just implement
> .atomic_begin() and call this from there? I think the only hitch is if you
> need this to be completed before crtc->atomic_enable(), at which point you
> might need to call it from vop_crtc_atomic_enable() and then detect that in
> atomic_begin()
> 

I think moving this to .atomic_begin might be enough. Let me send a new
series and we can see how that goes.

Thanks for reviewing,
Ezequiel

Re: [PATCH -next] treewide: remove unused argument in lock_release()

[Peter Zijlstra]

On Tue, Oct 08, 2019 at 06:33:51PM +0200, Daniel Vetter wrote:
> On Thu, Sep 19, 2019 at 12:09:40PM -0400, Qian Cai wrote:
> > Since the commit b4adfe8e05f1 ("locking/lockdep: Remove unused argument
> > in __lock_release"), @nested is no longer used in lock_release(), so
> > remove it from all lock_release() calls and friends.
> > 
> > Signed-off-by: Qian Cai 
> 
> Ack on the concept and for the drm parts (and feel free to keep the ack if
> you inevitably have to respin this later on). Might result in some
> conflicts, but welp we need to keep Linus busy :-)
> 
> Acked-by: Daniel Vetter 

Thanks Daniel!

Re: [PATCH RFC v4 16/16] drm/amdgpu: Integrate with DRM cgroup

[Kuehling, Felix]

On 2019-08-29 2:05 a.m., Kenny Ho wrote:
> The number of logical gpu (lgpu) is defined to be the number of compute
> unit (CU) for a device.  The lgpu allocation limit only applies to
> compute workload for the moment (enforced via kfd queue creation.)  Any
> cu_mask update is validated against the availability of the compute unit
> as defined by the drmcg the kfd process belongs to.

There is something missing here. There is an API for the application to 
specify a CU mask. Right now it looks like the application-specified and 
CGroup-specified CU masks would clobber each other. Instead the two 
should be merged.

The CGroup-specified mask should specify a subset of CUs available for 
application-specified CU masks. When the cgroup CU mask changes, you'd 
need to take any application-specified CU masks into account before 
updating the hardware.

The KFD topology APIs report the number of available CUs to the 
application. CGroups would change that number at runtime and 
applications would not expect that. I think the best way to deal with 
that would be to have multiple bits in the application-specified CU mask 
map to the same CU. How to do that in a fair way is not obvious. I guess 
a more coarse-grain division of the GPU into LGPUs would make this 
somewhat easier.

How is this problem handled for CPU cores and the interaction with CPU 
pthread_setaffinity_np?

Regards,
   Felix


>
> Change-Id: I69a57452c549173a1cd623c30dc57195b3b6563e
> Signed-off-by: Kenny Ho 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h|   4 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  21 +++
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c  |   6 +
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h |   3 +
>   .../amd/amdkfd/kfd_process_queue_manager.c| 140 ++
>   5 files changed, 174 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 55cb1b2094fd..369915337213 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -198,6 +198,10 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev 
> *dst, struct kgd_dev *s
>   valid;  \
>   })
>   
> +int amdgpu_amdkfd_update_cu_mask_for_process(struct task_struct *task,
> + struct amdgpu_device *adev, unsigned long *lgpu_bitmap,
> + unsigned int nbits);
> +
>   /* GPUVM API */
>   int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, unsigned int 
> pasid,
>   void **vm, void **process_info,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 163a4fbf0611..8abeffdd2e5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1398,9 +1398,29 @@ amdgpu_get_crtc_scanout_position(struct drm_device 
> *dev, unsigned int pipe,
>   static void amdgpu_drmcg_custom_init(struct drm_device *dev,
>   struct drmcg_props *props)
>   {
> + struct amdgpu_device *adev = dev->dev_private;
> +
> + props->lgpu_capacity = adev->gfx.cu_info.number;
> +
>   props->limit_enforced = true;
>   }
>   
> +static void amdgpu_drmcg_limit_updated(struct drm_device *dev,
> + struct task_struct *task, struct drmcg_device_resource *ddr,
> + enum drmcg_res_type res_type)
> +{
> + struct amdgpu_device *adev = dev->dev_private;
> +
> + switch (res_type) {
> + case DRMCG_TYPE_LGPU:
> + amdgpu_amdkfd_update_cu_mask_for_process(task, adev,
> +ddr->lgpu_allocated, dev->drmcg_props.lgpu_capacity);
> + break;
> + default:
> + break;
> + }
> +}
> +
>   static struct drm_driver kms_driver = {
>   .driver_features =
>   DRIVER_USE_AGP | DRIVER_ATOMIC |
> @@ -1438,6 +1458,7 @@ static struct drm_driver kms_driver = {
>   .gem_prime_mmap = amdgpu_gem_prime_mmap,
>   
>   .drmcg_custom_init = amdgpu_drmcg_custom_init,
> + .drmcg_limit_updated = amdgpu_drmcg_limit_updated,
>   
>   .name = DRIVER_NAME,
>   .desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 138c70454e2b..fa765b803f97 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -450,6 +450,12 @@ static int kfd_ioctl_set_cu_mask(struct file *filp, 
> struct kfd_process *p,
>   return -EFAULT;
>   }
>   
> + if (!pqm_drmcg_lgpu_validate(p, args->queue_id, properties.cu_mask, 
> cu_mask_size)) {
> + pr_debug("CU mask not permitted by DRM Cgroup");
> + kfree(properties.cu_mask);
> + return -EACCES;
> + }
> +
>   mutex_lock(>mutex);
>   
>   retval = pqm_set_cu_mask(>pqm, args->queue_id, );
> diff --git 

[Bug 111922] amdgpu fails to resume on 5.2 kernel [regression]

[bugzilla-daemon]

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

--- Comment #1 from Alex Deucher  ---
Can you bisect?

-- 
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 RFC v4 14/16] drm, cgroup: Introduce lgpu as DRM cgroup resource

[Kuehling, Felix]

On 2019-08-29 2:05 a.m., Kenny Ho wrote:
> drm.lgpu
>  A read-write nested-keyed file which exists on all cgroups.
>  Each entry is keyed by the DRM device's major:minor.
>
>  lgpu stands for logical GPU, it is an abstraction used to
>  subdivide a physical DRM device for the purpose of resource
>  management.
>
>  The lgpu is a discrete quantity that is device specific (i.e.
>  some DRM devices may have 64 lgpus while others may have 100
>  lgpus.)  The lgpu is a single quantity with two representations
>  denoted by the following nested keys.
>
>= 
>count Representing lgpu as anonymous resource
>list  Representing lgpu as named resource
>= 
>
>  For example:
>  226:0 count=256 list=0-255
>  226:1 count=4 list=0,2,4,6
>  226:2 count=32 list=32-63
>
>  lgpu is represented by a bitmap and uses the bitmap_parselist
>  kernel function so the list key input format is a
>  comma-separated list of decimal numbers and ranges.
>
>  Consecutively set bits are shown as two hyphen-separated decimal
>  numbers, the smallest and largest bit numbers set in the range.
>  Optionally each range can be postfixed to denote that only parts
>  of it should be set.  The range will divided to groups of
>  specific size.
>  Syntax: range:used_size/group_size
>  Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
>
>  The count key is the hamming weight / hweight of the bitmap.
>
>  Both count and list accept the max and default keywords.
>
>  Some DRM devices may only support lgpu as anonymous resources.
>  In such case, the significance of the position of the set bits
>  in list will be ignored.
>
>  This lgpu resource supports the 'allocation' resource
>  distribution model.
>
> Change-Id: I1afcacf356770930c7f925df043e51ad06ceb98e
> Signed-off-by: Kenny Ho 

The description sounds reasonable to me and maps well to the CU masking 
feature in our GPUs.

It would also allow us to do more coarse-grained masking for example to 
guarantee balanced allocation of CUs across shader engines or 
partitioning of memory bandwidth or CP pipes (if that is supported by 
the hardware/firmware).

I can't comment on the code as I'm unfamiliar with the details of the 
cgroup code.

Acked-by: Felix Kuehling 


> ---
>   Documentation/admin-guide/cgroup-v2.rst |  46 
>   include/drm/drm_cgroup.h|   4 +
>   include/linux/cgroup_drm.h  |   6 ++
>   kernel/cgroup/drm.c | 135 
>   4 files changed, 191 insertions(+)
>
> diff --git a/Documentation/admin-guide/cgroup-v2.rst 
> b/Documentation/admin-guide/cgroup-v2.rst
> index 87a195133eaa..57f18469bd76 100644
> --- a/Documentation/admin-guide/cgroup-v2.rst
> +++ b/Documentation/admin-guide/cgroup-v2.rst
> @@ -1958,6 +1958,52 @@ DRM Interface Files
>   Set largest allocation for /dev/dri/card1 to 4MB
>   echo "226:1 4m" > drm.buffer.peak.max
>   
> +  drm.lgpu
> + A read-write nested-keyed file which exists on all cgroups.
> + Each entry is keyed by the DRM device's major:minor.
> +
> + lgpu stands for logical GPU, it is an abstraction used to
> + subdivide a physical DRM device for the purpose of resource
> + management.
> +
> + The lgpu is a discrete quantity that is device specific (i.e.
> + some DRM devices may have 64 lgpus while others may have 100
> + lgpus.)  The lgpu is a single quantity with two representations
> + denoted by the following nested keys.
> +
> +   = 
> +   count Representing lgpu as anonymous resource
> +   list  Representing lgpu as named resource
> +   = 
> +
> + For example:
> + 226:0 count=256 list=0-255
> + 226:1 count=4 list=0,2,4,6
> + 226:2 count=32 list=32-63
> +
> + lgpu is represented by a bitmap and uses the bitmap_parselist
> + kernel function so the list key input format is a
> + comma-separated list of decimal numbers and ranges.
> +
> + Consecutively set bits are shown as two hyphen-separated decimal
> + numbers, the smallest and largest bit numbers set in the range.
> + Optionally each range can be postfixed to denote that only parts
> + of it should be set.  The range will divided to groups of
> + specific size.
> + Syntax: range:used_size/group_size
> + Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> +
> + The count key is the hamming weight / hweight of the bitmap.
> +
> + Both count and list accept the max and default keywords.
> +
> + Some DRM devices may only 

[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=205089

--- Comment #3 from Alex Deucher (alexdeuc...@gmail.com) ---
(In reply to Bruno Jacquet from comment #2)
> If I understand you right this means there is still another issue that
> caused the GPU reset. And this issue in particular is just a consequence of
> the reset not being properly handled?

The GPU reset succeeded.  However, since the GPU has been reset, the contents
of the memory (e.g, vram) that the application was using is undefined.  So the
application needs to use an API level (e.g., OpenGL robustness extensions or
vulkan context lost) interface to query whether the GPU was reset and
re-initialize it's buffers if so.

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

Re: [PATCH] drm/komeda: Adds power management support

[Sean Paul]

On Tue, Oct 8, 2019 at 1:47 PM Sean Paul  wrote:
>
> On Mon, Sep 23, 2019 at 01:59:25AM +, Lowry Li (Arm Technology China) 
> wrote:
> > From: "Lowry Li (Arm Technology China)" 
> >
> > Adds system power management support in KMS kernel driver.
> >
> > Depends on:
> > https://patchwork.freedesktop.org/series/62377/
> >
> > Changes since v1:
> > Since we have unified mclk/pclk/pipeline->aclk to one mclk, which will
> > be turned on/off when crtc atomic enable/disable, removed runtime power
> > management.
> > Removes run time get/put related flow.
> > Adds to disable the aclk when register access finished.
> >
> > Changes since v2:
> > Rebases to the drm-misc-next branch.
> >
> > Signed-off-by: Lowry Li (Arm Technology China) 
> > ---
> >  .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  1 -
> >  .../gpu/drm/arm/display/komeda/komeda_dev.c   | 65 +--
> >  .../gpu/drm/arm/display/komeda/komeda_dev.h   |  3 +
> >  .../gpu/drm/arm/display/komeda/komeda_drv.c   | 30 -
> >  4 files changed, 91 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c 
> > b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > index 38d5cb20e908..b47c0dabd0d1 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > @@ -5,7 +5,6 @@
> >   *
> >   */
> >  #include 
> > -#include 
> >  #include 
> >
> >  #include 
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c 
> > b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > index bee4633cdd9f..8a03324f02a5 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> > @@ -258,7 +258,7 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
> > product->product_id,
> > MALIDP_CORE_ID_PRODUCT_ID(mdev->chip.core_id));
> >   err = -ENODEV;
> > - goto err_cleanup;
> > + goto disable_clk;
> >   }
> >
> >   DRM_INFO("Found ARM Mali-D%x version r%dp%d\n",
> > @@ -271,19 +271,19 @@ struct komeda_dev *komeda_dev_create(struct device 
> > *dev)
> >   err = mdev->funcs->enum_resources(mdev);
> >   if (err) {
> >   DRM_ERROR("enumerate display resource failed.\n");
> > - goto err_cleanup;
> > + goto disable_clk;
> >   }
> >
> >   err = komeda_parse_dt(dev, mdev);
> >   if (err) {
> >   DRM_ERROR("parse device tree failed.\n");
> > - goto err_cleanup;
> > + goto disable_clk;
> >   }
> >
> >   err = komeda_assemble_pipelines(mdev);
> >   if (err) {
> >   DRM_ERROR("assemble display pipelines failed.\n");
> > - goto err_cleanup;
> > + goto disable_clk;
> >   }
> >
> >   dev->dma_parms = >dma_parms;
> > @@ -296,11 +296,14 @@ struct komeda_dev *komeda_dev_create(struct device 
> > *dev)
> >   if (mdev->iommu && mdev->funcs->connect_iommu) {
> >   err = mdev->funcs->connect_iommu(mdev);
> >   if (err) {
> > + DRM_ERROR("connect iommu failed.\n");
> >   mdev->iommu = NULL;
> > - goto err_cleanup;
> > + goto disable_clk;
> >   }
> >   }
> >
> > + clk_disable_unprepare(mdev->aclk);
> > +
> >   err = sysfs_create_group(>kobj, _sysfs_attr_group);
> >   if (err) {
> >   DRM_ERROR("create sysfs group failed.\n");
> > @@ -313,6 +316,8 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
> >
> >   return mdev;
> >
> > +disable_clk:
> > + clk_disable_unprepare(mdev->aclk);
> >  err_cleanup:
> >   komeda_dev_destroy(mdev);
> >   return ERR_PTR(err);
> > @@ -330,8 +335,12 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
> >   debugfs_remove_recursive(mdev->debugfs_root);
> >  #endif
> >
> > + if (mdev->aclk)
> > + clk_prepare_enable(mdev->aclk);
> > +
> >   if (mdev->iommu && mdev->funcs->disconnect_iommu)
> > - mdev->funcs->disconnect_iommu(mdev);
> > + if (mdev->funcs->disconnect_iommu(mdev))
> > + DRM_ERROR("disconnect iommu failed.\n");
> >   mdev->iommu = NULL;
> >
> >   for (i = 0; i < mdev->n_pipelines; i++) {
> > @@ -359,3 +368,47 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
> >
> >   devm_kfree(dev, mdev);
> >  }
> > +
> > +int komeda_dev_resume(struct komeda_dev *mdev)
> > +{
> > + int ret = 0;
> > +
> > + clk_prepare_enable(mdev->aclk);
> > +
> > + if (mdev->iommu && mdev->funcs->connect_iommu) {
> > + ret = mdev->funcs->connect_iommu(mdev);
> > + if (ret < 0) {
> > + DRM_ERROR("connect iommu failed.\n");
> > + goto disable_clk;
> > + }
> > + }
> > +
> > + ret = mdev->funcs->enable_irq(mdev);
> > +

[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=205089

--- Comment #2 from Bruno Jacquet (maxi...@free.fr) ---
If I understand you right this means there is still another issue that caused
the GPU reset. And this issue in particular is just a consequence of the reset
not being properly handled?

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

Re: [PATCH] drm/komeda: Adds power management support

[Sean Paul]

On Mon, Sep 23, 2019 at 01:59:25AM +, Lowry Li (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)" 
> 
> Adds system power management support in KMS kernel driver.
> 
> Depends on:
> https://patchwork.freedesktop.org/series/62377/
> 
> Changes since v1:
> Since we have unified mclk/pclk/pipeline->aclk to one mclk, which will
> be turned on/off when crtc atomic enable/disable, removed runtime power
> management.
> Removes run time get/put related flow.
> Adds to disable the aclk when register access finished.
> 
> Changes since v2:
> Rebases to the drm-misc-next branch.
> 
> Signed-off-by: Lowry Li (Arm Technology China) 
> ---
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  1 -
>  .../gpu/drm/arm/display/komeda/komeda_dev.c   | 65 +--
>  .../gpu/drm/arm/display/komeda/komeda_dev.h   |  3 +
>  .../gpu/drm/arm/display/komeda/komeda_drv.c   | 30 -
>  4 files changed, 91 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c 
> b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 38d5cb20e908..b47c0dabd0d1 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -5,7 +5,6 @@
>   *
>   */
>  #include 
> -#include 
>  #include 
>  
>  #include 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c 
> b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index bee4633cdd9f..8a03324f02a5 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -258,7 +258,7 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
> product->product_id,
> MALIDP_CORE_ID_PRODUCT_ID(mdev->chip.core_id));
>   err = -ENODEV;
> - goto err_cleanup;
> + goto disable_clk;
>   }
>  
>   DRM_INFO("Found ARM Mali-D%x version r%dp%d\n",
> @@ -271,19 +271,19 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
>   err = mdev->funcs->enum_resources(mdev);
>   if (err) {
>   DRM_ERROR("enumerate display resource failed.\n");
> - goto err_cleanup;
> + goto disable_clk;
>   }
>  
>   err = komeda_parse_dt(dev, mdev);
>   if (err) {
>   DRM_ERROR("parse device tree failed.\n");
> - goto err_cleanup;
> + goto disable_clk;
>   }
>  
>   err = komeda_assemble_pipelines(mdev);
>   if (err) {
>   DRM_ERROR("assemble display pipelines failed.\n");
> - goto err_cleanup;
> + goto disable_clk;
>   }
>  
>   dev->dma_parms = >dma_parms;
> @@ -296,11 +296,14 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
>   if (mdev->iommu && mdev->funcs->connect_iommu) {
>   err = mdev->funcs->connect_iommu(mdev);
>   if (err) {
> + DRM_ERROR("connect iommu failed.\n");
>   mdev->iommu = NULL;
> - goto err_cleanup;
> + goto disable_clk;
>   }
>   }
>  
> + clk_disable_unprepare(mdev->aclk);
> +
>   err = sysfs_create_group(>kobj, _sysfs_attr_group);
>   if (err) {
>   DRM_ERROR("create sysfs group failed.\n");
> @@ -313,6 +316,8 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
>  
>   return mdev;
>  
> +disable_clk:
> + clk_disable_unprepare(mdev->aclk);
>  err_cleanup:
>   komeda_dev_destroy(mdev);
>   return ERR_PTR(err);
> @@ -330,8 +335,12 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
>   debugfs_remove_recursive(mdev->debugfs_root);
>  #endif
>  
> + if (mdev->aclk)
> + clk_prepare_enable(mdev->aclk);
> +
>   if (mdev->iommu && mdev->funcs->disconnect_iommu)
> - mdev->funcs->disconnect_iommu(mdev);
> + if (mdev->funcs->disconnect_iommu(mdev))
> + DRM_ERROR("disconnect iommu failed.\n");
>   mdev->iommu = NULL;
>  
>   for (i = 0; i < mdev->n_pipelines; i++) {
> @@ -359,3 +368,47 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
>  
>   devm_kfree(dev, mdev);
>  }
> +
> +int komeda_dev_resume(struct komeda_dev *mdev)
> +{
> + int ret = 0;
> +
> + clk_prepare_enable(mdev->aclk);
> +
> + if (mdev->iommu && mdev->funcs->connect_iommu) {
> + ret = mdev->funcs->connect_iommu(mdev);
> + if (ret < 0) {
> + DRM_ERROR("connect iommu failed.\n");
> + goto disable_clk;
> + }
> + }
> +
> + ret = mdev->funcs->enable_irq(mdev);
> +
> +disable_clk:
> + clk_disable_unprepare(mdev->aclk);
> +
> + return ret;
> +}
> +
> +int komeda_dev_suspend(struct komeda_dev *mdev)
> +{
> + int ret = 0;
> +
> + clk_prepare_enable(mdev->aclk);
> +
> + if (mdev->iommu && mdev->funcs->disconnect_iommu) {
> + ret = 

[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

[bugzilla-daemon]

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

--- Comment #81 from Shmerl  ---
Also opened Firefox specific bug here in case it's radeonsi issue:
https://gitlab.freedesktop.org/mesa/mesa/issues/1910

-- 
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 v9 4/5] dt-bindings: backlight: Add led-backlight binding

[Jacek Anaszewski]

On 10/8/19 5:00 PM, Rob Herring wrote:
> On Tue, Oct 8, 2019 at 8:30 AM Jean-Jacques Hiblot  wrote:
>>
>> Rob,
>>
>> On 08/10/2019 14:51, Jean-Jacques Hiblot wrote:
>>> Hi Rob,
>>>
>>> On 07/10/2019 18:15, Rob Herring wrote:
 Please send DT bindings to DT list or it's never in my queue. IOW,
 send patches to the lists that get_maintainers.pl tells you to.

 On Mon, Oct 7, 2019 at 7:45 AM Jean-Jacques Hiblot 
 wrote:
> Add DT binding for led-backlight.
>
> Signed-off-by: Jean-Jacques Hiblot 
> Reviewed-by: Daniel Thompson 
> Reviewed-by: Sebastian Reichel 
> ---
>   .../bindings/leds/backlight/led-backlight.txt | 28
> +++
>   1 file changed, 28 insertions(+)
>   create mode 100644
> Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
 Please make this a DT schema.
>>>
>>> OK.
>>>
>>> BTW I used "make dt_binding_check" but had to fix a couple of YAMLs
>>> file to get it to work. Do you have a kernel tree with already all the
>>> YAML files in good shape ? Or do you want me to post the changes to
>>> devicet...@vger.kernel.org ?
>>>
>>>

> diff --git
> a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> new file mode 100644
> index ..4c7dfbe7f67a
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> @@ -0,0 +1,28 @@
> +led-backlight bindings
> +
> +This binding is used to describe a basic backlight device made of
> LEDs.
> +It can also be used to describe a backlight device controlled by
> the output of
> +a LED driver.
> +
> +Required properties:
> +  - compatible: "led-backlight"
> +  - leds: a list of LEDs
 'leds' is already used as a node name and mixing is not ideal.

for the record: child node names (if that was what you had on mind)
have singular form 'led'.

 We already have 'flash-leds' in use and with the same definition, so
 lets continue that and use 'backlight-leds'.
>>> OK
>>
>> I am taking that back. I have added of_get_led() and devm_of_get_led()
>> to the LED core to make it easier to get a LED from the DT. I modeled
>> the interface like it is done for PWM, PHYs or clocks. The property
>> containing list/array of phandle  is always named the same. To get one
>> particular PWM/PHY/clock, a identifier (name or integer) must be provided.
> 
> It can be done as we do support that with '-gpios', but yes, it is a
> bit more painful to deal with.
> 
>> So unless there is a strong incentive to do otherwise I's rather keep
>> this name here.
> 
> In that case, this needs to be documented as a common LED binding, not
> something hidden away in this binding.
> 
> Rob
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v9 4/5] dt-bindings: backlight: Add led-backlight binding

[Jacek Anaszewski]

On 10/8/19 5:00 PM, Rob Herring wrote:
> On Tue, Oct 8, 2019 at 8:30 AM Jean-Jacques Hiblot  wrote:
>>
>> Rob,
>>
>> On 08/10/2019 14:51, Jean-Jacques Hiblot wrote:
>>> Hi Rob,
>>>
>>> On 07/10/2019 18:15, Rob Herring wrote:
 Please send DT bindings to DT list or it's never in my queue. IOW,
 send patches to the lists that get_maintainers.pl tells you to.

 On Mon, Oct 7, 2019 at 7:45 AM Jean-Jacques Hiblot 
 wrote:
> Add DT binding for led-backlight.
>
> Signed-off-by: Jean-Jacques Hiblot 
> Reviewed-by: Daniel Thompson 
> Reviewed-by: Sebastian Reichel 
> ---
>   .../bindings/leds/backlight/led-backlight.txt | 28
> +++
>   1 file changed, 28 insertions(+)
>   create mode 100644
> Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
 Please make this a DT schema.
>>>
>>> OK.
>>>
>>> BTW I used "make dt_binding_check" but had to fix a couple of YAMLs
>>> file to get it to work. Do you have a kernel tree with already all the
>>> YAML files in good shape ? Or do you want me to post the changes to
>>> devicet...@vger.kernel.org ?
>>>
>>>

> diff --git
> a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> new file mode 100644
> index ..4c7dfbe7f67a
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> @@ -0,0 +1,28 @@
> +led-backlight bindings
> +
> +This binding is used to describe a basic backlight device made of
> LEDs.
> +It can also be used to describe a backlight device controlled by
> the output of
> +a LED driver.
> +
> +Required properties:
> +  - compatible: "led-backlight"
> +  - leds: a list of LEDs
 'leds' is already used as a node name and mixing is not ideal.

for the record: child node names (if that was what you had on mind)
have singular form 'led'.

 We already have 'flash-leds' in use and with the same definition, so
 lets continue that and use 'backlight-leds'.
>>> OK
>>
>> I am taking that back. I have added of_get_led() and devm_of_get_led()
>> to the LED core to make it easier to get a LED from the DT. I modeled
>> the interface like it is done for PWM, PHYs or clocks. The property
>> containing list/array of phandle  is always named the same. To get one
>> particular PWM/PHY/clock, a identifier (name or integer) must be provided.
> 
> It can be done as we do support that with '-gpios', but yes, it is a
> bit more painful to deal with.
> 
>> So unless there is a strong incentive to do otherwise I's rather keep
>> this name here.
> 
> In that case, this needs to be documented as a common LED binding, not
> something hidden away in this binding.
> 
> Rob
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH 09/11] lib/interval-tree: convert interval_tree to half closed intervals

[Davidlohr Bueso]

On Fri, 04 Oct 2019, Koenig, Christian wrote:


Am 04.10.19 um 08:57 schrieb Christian König:

Am 03.10.19 um 22:18 schrieb Davidlohr Bueso:

The generic tree tree really wants [a, b) intervals, not fully closed.
As such convert it to use the new interval_tree_gen.h. Most of the
conversions are straightforward, with the exception of perhaps
radeon_vm_bo_set_addr(), but semantics have been tried to be left
untouched.


NAK, the whole thing won't work.

See we need to handle the full device address space which means we
have values in the range of 0x0-0x.

If you make this a closed interval then the end would wrap around to
0x0 if long is only 32bit.


Well I've just now re-read the subject line. From that it sounds like
you are actually trying to fix the interval tree to use a half closed
interval, e.g. something like [a, b[


Correct.



But your code changes sometimes doesn't seem to reflect that.


Hmm the change simply aims at avoiding the end - 1 trick when dealing
with interval_tree insertions and lookups; the rest of the series
converts other interval tree users in a similar way, albeit some errors
which will be updated. What are your concerns about this patch?

Thanks,
Davidlohr

[Bug 111481] AMD Navi GPU frequent freezes on both Manjaro/Ubuntu with kernel 5.3 and mesa 19.2 -git/llvm9

[bugzilla-daemon]

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

--- Comment #80 from Shmerl  ---
Looks like there are a bunch of fixes related to powerplay here:
https://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-next

So if anyone has time to test, may be some of them fix that concurrent sensor
access bug that still happens in 5.4-rc2 despite previous added mutex fix
already being there.

-- 
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 v1 0/2]: Finally delete drmP.h

[Sam Ravnborg]

On Mon, Oct 07, 2019 at 07:12:22PM +0200, Sam Ravnborg wrote:
> One user of drmP.h sneaked in after the merge window.
> Drop the use of drmP.h and delete the header file for good.
> 
> Small band-aid on top of not going to xdc :-)
> 
> Build tested with various architectures and configs.

Applied and pushed to drm-misc-next.

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

[PATCH 1/2] drm/edid: Add drm_hdmi_avi_infoframe_bars()

[Ville Syrjala]

From: Ville Syrjälä 

Add a function to fill the AVI infoframe bar information from
the standard tv margin properties.

Cc: Eric Anholt 
Cc: Boris Brezillon 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_edid.c | 17 +
 include/drm/drm_edid.h |  4 
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0552175313cb..4af184b07cc1 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5382,6 +5382,23 @@ drm_hdmi_avi_infoframe_quant_range(struct 
hdmi_avi_infoframe *frame,
 }
 EXPORT_SYMBOL(drm_hdmi_avi_infoframe_quant_range);
 
+/**
+ * drm_hdmi_avi_infoframe_bars() - fill the HDMI AVI infoframe
+ * bar information
+ * @frame: HDMI AVI infoframe
+ * @conn_state: connector state
+ */
+void
+drm_hdmi_avi_infoframe_bars(struct hdmi_avi_infoframe *frame,
+   const struct drm_connector_state *conn_state)
+{
+   frame->right_bar = conn_state->tv.margins.right;
+   frame->left_bar = conn_state->tv.margins.left;
+   frame->top_bar = conn_state->tv.margins.top;
+   frame->bottom_bar = conn_state->tv.margins.bottom;
+}
+EXPORT_SYMBOL(drm_hdmi_avi_infoframe_bars);
+
 static enum hdmi_3d_structure
 s3d_structure_from_display_mode(const struct drm_display_mode *mode)
 {
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index b9719418c3d2..e0701b3d3194 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -367,6 +367,10 @@ void
 drm_hdmi_avi_infoframe_colorspace(struct hdmi_avi_infoframe *frame,
  const struct drm_connector_state *conn_state);
 
+void
+drm_hdmi_avi_infoframe_bars(struct hdmi_avi_infoframe *frame,
+   const struct drm_connector_state *conn_state);
+
 void
 drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
   struct drm_connector *connector,
-- 
2.21.0

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

[PATCH 2/2] drm/vc4: Use drm_hdmi_avi_infoframe_bars()

[Ville Syrjala]

From: Ville Syrjälä 

Use the new drm_hdmi_avi_infoframe_bars() helper instead
of hand rolling it.

Cc: Eric Anholt 
Cc: Boris Brezillon 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 0853b980bcb3..1c62c6c9244b 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -398,10 +398,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder 
*encoder)
   HDMI_QUANTIZATION_RANGE_LIMITED :
   HDMI_QUANTIZATION_RANGE_FULL);
 
-   frame.avi.right_bar = cstate->tv.margins.right;
-   frame.avi.left_bar = cstate->tv.margins.left;
-   frame.avi.top_bar = cstate->tv.margins.top;
-   frame.avi.bottom_bar = cstate->tv.margins.bottom;
+   drm_hdmi_avi_infoframe_bars(, cstate);
 
vc4_hdmi_write_infoframe(encoder, );
 }
-- 
2.21.0

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

[Bug 111921] GPU crash on VegaM (amdgpu: The CS has been rejected)

[bugzilla-daemon]

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

--- Comment #4 from Andrey Grodzovsky  ---
What happens if you disable GPU reset by loading the kernel with
amdgpu.gpu_recovery=0 ?

-- 
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] dma-buf/resv: fix exclusive fence get

[Daniel Vetter]

On Mon, Sep 30, 2019 at 08:57:32AM +, Koenig, Christian wrote:
> Am 30.09.19 um 09:22 schrieb Daniel Vetter:
> > On Sun, Sep 22, 2019 at 2:08 PM Qiang Yu  wrote:
> >> This causes kernel crash when testing lima driver.
> >>
> >> Cc: Christian König 
> >> Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu 
> >> a bit")
> >> Signed-off-by: Qiang Yu 
> > Selftest for this would be lovely, now that the basic infrastructure
> > is in place ...
> 
> What do you have in mind? I wouldn't even know where to start to write 
> an unit test for this.

1. set a few fences (both excl + shared) in a dma_resv
2. get them
3. check that we got them all
4. notice that the exlusive fence isn't actually in the array (because we
increment the index before storing, so the exclusive fence ended past the
array). For robustness the test should check that the fences are listed in
any order, not just the one the current implementation gives you.

I guess the actual crash happens when we're unlucky and overflow the
allocation, which is probably more rare. But KASAN should help catch that
too (run that in your CI if you don't do that yet, it's pretty
impressive).

Or am I totally misunderstanding what's going wrong here?
-Daniel
> 
> Christian.
> 
> > -Daniel
> >
> >> ---
> >>   drivers/dma-buf/dma-resv.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> >> index 42a8f3f11681..709002515550 100644
> >> --- a/drivers/dma-buf/dma-resv.c
> >> +++ b/drivers/dma-buf/dma-resv.c
> >> @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
> >>  if (pfence_excl)
> >>  *pfence_excl = fence_excl;
> >>  else if (fence_excl)
> >> -   shared[++shared_count] = fence_excl;
> >> +   shared[shared_count++] = fence_excl;
> >>
> >>  if (!shared_count) {
> >>  kfree(shared);
> >> --
> >> 2.17.1
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> 

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

Re: [PATCH 13/14] drm/amd/display: Recalculate VCPI slots for new DSC connectors

[Lyude Paul]

On Tue, 2019-10-08 at 12:24 -0400, Lyude Paul wrote:
> ...
> yikes
> I need to apologize because I was going through my email and I realized you
> _did_ respond to me earlier regarding some of these questions, it just
> appears
> the reply fell through the cracks and somehow I didn't notice it until just
> now. Sorry about that!

Referring to this response, jfyi:
https://lists.freedesktop.org/archives/amd-gfx/2019-October/040683.html

(sorry again for replying before reading that!)
> 
> I'm still not sure I understand why this is a future enhancement? Everything
> we need to implement these helpers should already be here, it's just a
> matter
> of keeping track who has dsc enabled where in the mst atomic state
> 
> On Mon, 2019-10-07 at 17:52 -0400, Lyude Paul wrote:
> > Ok, let's stop and slow down for a minute here since I've repeated myself
> > a
> > few times, and I'd like to figure out what's actually happening here and
> > whether I'm not being clear enough with my explanations, or if there's
> > just
> > some misunderstanding here.
> > 
> > I'll start of by trying my best to explain my hesistance in accepting
> > these
> > patches. To be clear, MST with DSC is a big thing in terms of impact.
> > There's
> > a lot of things to consider:
> >  * What should the default DSC policy for MST be? Should we keep it on by
> >default so that we don't need to trigger a large number of PBN
> >recalculations and enable DSC on a per-case basis, or are there
> > legitimate
> >reasons to keep DSC off by default (such as potential issues with
> > lossiness
> >down the line).
> >  * We have other drivers in the tree that are planning on implementing MST
> > DSC
> >quite soon. Chances are they'll be implementing helpers to do this so
> > this
> >can be supported across the DRM tree, and chances are we'll just have
> > to
> >rewrite and remove most of this code in that case once they do.
> >  * amdgpu already has a _lot_ of code that doesn't need to, and shouldn't
> > be
> >there. This ranges from various DC specific helpers that are
> > essentially
> >the same as the helpers that we already implement in the rest of DRM,
> > to
> >misuses of various kernel APIs, and a complete lack of any kind of code
> >style (at least the last time that I checked) in or out of DC. This
> > makes
> >the driver more difficult for people who don't work at AMD but are well
> >accustomed to working on the rest of the kernel, e.g. myself and
> > others,
> >and it's a problem that seriously needs to be solved. Adding more
> > redundant
> >DP helpers that will inevitably get removed is just making the problem
> >worse.
> > 
> > With all of this being said: I've been asking the same questions about
> > this
> > patch throughout the whole patch series so far (since it got passed off to
> > you
> > from David's internship ending):
> > 
> >  * Can we keep DSC enabled by default, so that these patches aren't
> > needed?
> >  * If we can't, can we move this into common code so that other drivers
> > can
> >use it?
> > The problem is I haven't received any responses to these questions, other
> > then
> > being told by David a month or two ago that it would be expedient to just
> > accept the patches and move on. Honestly, if discussion had been had about
> > the
> > changes I requested, I would have given my r-b a long time ago. In fact-I
> > really want to give it now! There's multiple patches in this series that
> > would
> > be very useful for other things that are being worked on at the moment,
> > such
> > as the changes to drm_dp_dpcd_read/drm_dp_dpcd_write(). But I really think
> > this needs to be discussed first, and I'm worried that just going ahead
> > and
> > giving my R-B before they have been discussed will further encourage
> > rushing
> > changes like this in the future and continually building more tech debt
> > for
> > ourselves.
> > 
> > Please note as well: if anything I've asked for is confusing, or you don't
> > understand what I'm asking or looking for I am more then willing to help
> > explain things and help out as best as I can. I understand that a lot of
> > the
> > developers working on DRM at AMD may have more experience in Windows and
> > Mac
> > land and as a result, trying to get used to the way that we do things in
> > the
> > Linux kernel can be a confusing endeavor. I'm more then happy to help out
> > with
> > this wherever I can, all you need to do is ask. Asking a few questions
> > about
> > something you aren't sure you understand can save both of us a lot of
> > time,
> > and avoid having to go through this many patch respins.
> > 
> > In the mean time, I'd be willing to look at what patches from this series
> > that
> > have already been reviewed which could be pushed to drm-misc or friends in
> > the
> > mean time to speed things up a bit.
> > 
> > On Tue, 2019-10-01 at 12:17 -0400, mikita.lip...@amd.com wrote:
> > > From: Mikita Lipski 
> > 

Re: [PATCH 2/2] Documentation/gpu: Fix no structured comments warning for drm_gem_ttm_helper.h

[Daniel Vetter]

On Mon, Sep 23, 2019 at 09:03:01AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 20.09.19 um 21:35 schrieb Sean Paul:
> > From: Sean Paul 
> > 
> > Fixes
> > include/drm/drm_gem_ttm_helper.h:1: warning: no structured comments found
> 
> That missing documentation looks like an oversight to me.
> 
> Acked-by: Thomas Zimmermann 
> 
> under the premise that there's not currently some patch with the missing
> documentation floating around.

There's no struct or inline functions in that header file, so really
nothing to document. Just need to make sure that if we add anything, we
re-add the include directive.
-Daniel

> 
> Best regards
> Thomas
> 
> > Fixes: ff540b76f14a ("drm/ttm: add drm gem ttm helpers, starting with 
> > drm_gem_ttm_print_info()")
> > Cc: Gerd Hoffmann 
> > Cc: Thomas Zimmermann 
> > Cc: Daniel Vetter 
> > Cc: Maarten Lankhorst 
> > Cc: Maxime Ripard 
> > Cc: Sean Paul 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Sean Paul 
> > ---
> >  Documentation/gpu/drm-mm.rst | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
> > index 99d56015e077..59619296c84b 100644
> > --- a/Documentation/gpu/drm-mm.rst
> > +++ b/Documentation/gpu/drm-mm.rst
> > @@ -406,9 +406,6 @@ GEM TTM Helper Functions Reference
> >  .. kernel-doc:: drivers/gpu/drm/drm_gem_ttm_helper.c
> > :doc: overview
> >  
> > -.. kernel-doc:: include/drm/drm_gem_ttm_helper.h
> > -   :internal:
> > -
> >  .. kernel-doc:: drivers/gpu/drm/drm_gem_ttm_helper.c
> > :export:
> >  
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
> HRB 21284 (AG Nürnberg)
> 




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

Re: [PATCH -next] treewide: remove unused argument in lock_release()

[Daniel Vetter]

On Thu, Sep 19, 2019 at 12:09:40PM -0400, Qian Cai wrote:
> Since the commit b4adfe8e05f1 ("locking/lockdep: Remove unused argument
> in __lock_release"), @nested is no longer used in lock_release(), so
> remove it from all lock_release() calls and friends.
> 
> Signed-off-by: Qian Cai 

Ack on the concept and for the drm parts (and feel free to keep the ack if
you inevitably have to respin this later on). Might result in some
conflicts, but welp we need to keep Linus busy :-)

Acked-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_connector.c   |  2 +-
>  drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  |  6 +++---
>  drivers/gpu/drm/i915/gt/intel_engine_pm.c |  2 +-
>  drivers/gpu/drm/i915/i915_request.c   |  2 +-
>  drivers/tty/tty_ldsem.c   |  8 
>  fs/dcache.c   |  2 +-
>  fs/jbd2/transaction.c |  4 ++--
>  fs/kernfs/dir.c   |  4 ++--
>  fs/ocfs2/dlmglue.c|  2 +-
>  include/linux/jbd2.h  |  2 +-
>  include/linux/lockdep.h   | 21 ++---
>  include/linux/percpu-rwsem.h  |  4 ++--
>  include/linux/rcupdate.h  |  2 +-
>  include/linux/rwlock_api_smp.h| 16 
>  include/linux/seqlock.h   |  4 ++--
>  include/linux/spinlock_api_smp.h  |  8 
>  include/linux/ww_mutex.h  |  2 +-
>  include/net/sock.h|  2 +-
>  kernel/bpf/stackmap.c |  2 +-
>  kernel/cpu.c  |  2 +-
>  kernel/locking/lockdep.c  |  3 +--
>  kernel/locking/mutex.c|  4 ++--
>  kernel/locking/rtmutex.c  |  6 +++---
>  kernel/locking/rwsem.c| 10 +-
>  kernel/printk/printk.c| 10 +-
>  kernel/sched/core.c   |  2 +-
>  lib/locking-selftest.c| 24 
>  mm/memcontrol.c   |  2 +-
>  net/core/sock.c   |  2 +-
>  tools/lib/lockdep/include/liblockdep/common.h |  3 +--
>  tools/lib/lockdep/include/liblockdep/mutex.h  |  2 +-
>  tools/lib/lockdep/include/liblockdep/rwlock.h |  2 +-
>  tools/lib/lockdep/preload.c   | 16 
>  33 files changed, 90 insertions(+), 93 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..4a8b2e5c2af6 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -719,7 +719,7 @@ void drm_connector_list_iter_end(struct 
> drm_connector_list_iter *iter)
>   __drm_connector_put_safe(iter->conn);
>   spin_unlock_irqrestore(>connector_list_lock, flags);
>   }
> - lock_release(_list_iter_dep_map, 0, _RET_IP_);
> + lock_release(_list_iter_dep_map, _RET_IP_);
>  }
>  EXPORT_SYMBOL(drm_connector_list_iter_end);
>  
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> index edd21d14e64f..1a51b3598d63 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> @@ -509,14 +509,14 @@ void i915_gem_shrinker_taints_mutex(struct 
> drm_i915_private *i915,
> I915_MM_SHRINKER, 0, _RET_IP_);
>  
>   mutex_acquire(>dep_map, 0, 0, _RET_IP_);
> - mutex_release(>dep_map, 0, _RET_IP_);
> + mutex_release(>dep_map, _RET_IP_);
>  
> - mutex_release(>drm.struct_mutex.dep_map, 0, _RET_IP_);
> + mutex_release(>drm.struct_mutex.dep_map, _RET_IP_);
>  
>   fs_reclaim_release(GFP_KERNEL);
>  
>   if (unlock)
> - mutex_release(>drm.struct_mutex.dep_map, 0, _RET_IP_);
> + mutex_release(>drm.struct_mutex.dep_map, _RET_IP_);
>  }
>  
>  #define obj_to_i915(obj__) to_i915((obj__)->base.dev)
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c 
> b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index 65b5ca74b394..7f647243b3b9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -52,7 +52,7 @@ static inline unsigned long __timeline_mark_lock(struct 
> intel_context *ce)
>  static inline void __timeline_mark_unlock(struct intel_context *ce,
> unsigned long flags)
>  {
> - mutex_release(>timeline->mutex.dep_map, 0, _THIS_IP_);
> + mutex_release(>timeline->mutex.dep_map, _THIS_IP_);
>   local_irq_restore(flags);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_request.c 
> b/drivers/gpu/drm/i915/i915_request.c
> index a53777dd371c..e1f1be4d0531 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1456,7 

Re: [PATCH v2] drm/ioctl: Add a ioctl to label GEM objects

[Daniel Vetter]

On Thu, Sep 26, 2019 at 10:47:35AM +0200, Rohan Garg wrote:
> On viernes, 20 de septiembre de 2019 17:25:10 (CEST) Thierry Reding wrote:
> > On Thu, Sep 19, 2019 at 02:53:21PM +0200, Rohan Garg wrote:
> > > DRM_IOCTL_BO_SET_LABEL lets you label GEM objects, making it
> > > easier to debug issues in userspace applications.
> > > 
> > > Changes in v2:
> > >   - Hoist the IOCTL up into the drm_driver framework
> > > 
> > > Signed-off-by: Rohan Garg 
> > > ---
> > > 
> > >  drivers/gpu/drm/drm_gem.c  | 64 ++
> > >  drivers/gpu/drm/drm_internal.h |  4 +++
> > >  drivers/gpu/drm/drm_ioctl.c|  1 +
> > >  include/drm/drm_drv.h  | 18 ++
> > >  include/drm/drm_gem.h  |  7 
> > >  include/uapi/drm/drm.h | 20 +++
> > >  6 files changed, 114 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > > index 6854f5867d51..785ac561619a 100644
> > > --- a/drivers/gpu/drm/drm_gem.c
> > > +++ b/drivers/gpu/drm/drm_gem.c
> > > @@ -941,6 +941,65 @@ drm_gem_release(struct drm_device *dev, struct
> > > drm_file *file_private)> 
> > >   idr_destroy(_private->object_idr);
> > >  
> > >  }
> > > 
> > > +int drm_bo_set_label_ioctl(struct drm_device *dev, void *data,
> > > +struct drm_file *file_priv)
> > 
> > Perhaps call this drm_gem_set_label_ioctl() so that it's consistent with
> > the other GEM related IOCTLs?
> > 
> > > +{
> > > + struct drm_label_object *args = data;
> > 
> > Similarly, perhaps make this struct drm_gem_set_label for consistency.
> > 
> > > +
> > > + if (!args->len || !args->name)
> > > + return -EINVAL;
> > > +
> > > + if (dev->driver->label)
> > > + return dev->driver->label(dev, args, file_priv);
> > > +
> > > + return -EOPNOTSUPP;
> > > +}
> > > +
> > > +/**
> > > + * drm_gem_label - label a given GEM object
> > > + * @dev: drm_device for the associated GEM object
> > > + * @data: drm_label_bo struct with a label, label length and any relevant
> > > flags + * @file_private: drm file-private structure to clean up
> > > + */
> > > +
> > > +int drm_gem_set_label(struct drm_device *dev, struct drm_label_object
> > > *args, struct drm_file *file_priv)
> > Function name doesn't match the kerneldoc.
> > 
> > > +{
> > > + struct drm_gem_object *gem_obj;
> > > + int err = 0;
> > > + char *label;
> > > +
> > > + if (args->len > 0)
> > > + label = strndup_user(u64_to_user_ptr(args->name), args-
> >len);
> > > +
> > > + gem_obj = drm_gem_object_lookup(file_priv, args->handle);
> > > + if (!gem_obj) {
> > > + DRM_ERROR("Failed to look up GEM BO %d\n", args-
> >handle);
> > 
> > People could abuse this to spam the system logs with these error
> > messages. Better make this DRM_DEBUG() or leave it out completely.
> > 
> > > + err = -ENOENT;
> > > + goto err;
> > 
> > I think you're leaking the label string here.
> > 
> > > + }
> > > +
> > > + if ((args->len == 0 && args->name == NULL) || gem_obj->label) {
> > > + kfree(gem_obj->label);
> > > + gem_obj->label = NULL;
> > > + }
> > > +
> > > + gem_obj->label = label;
> > > +
> > > + drm_gem_object_put_unlocked(gem_obj);
> > > +
> > > + if (IS_ERR(gem_obj->label)) {
> > > + err = PTR_ERR(gem_obj->label);
> > > + goto err;
> > > + }
> > 
> > Why don't you check for this earlier? That seems suboptimal because now
> > you've got some error value in gem_obj->label that you're going to have
> > to special case at every step of the way.
> > 
> > > +
> > > +err:
> > > + if (err != 0)
> > > + args->flags = err;
> > 
> > Why the flags? We typically just return the error...
> > 
> > > +
> > > + return err;
> > 
> > ... like here.
> > 
> > > +}
> > 
> > Do we want to export this so that drivers can use it?
> > 
> > > +
> > > +
> > > 
> > >  /**
> > >  
> > >   * drm_gem_object_release - release GEM buffer object resources
> > >   * @obj: GEM buffer object
> > > 
> > > @@ -958,6 +1017,11 @@ drm_gem_object_release(struct drm_gem_object *obj)
> > > 
> > >   dma_resv_fini(>_resv);
> > >   drm_gem_free_mmap_offset(obj);
> > > 
> > > +
> > > + if (obj->label) {
> > > + kfree(obj->label);
> > > + obj->label = NULL;
> > > + }
> > > 
> > >  }
> > >  EXPORT_SYMBOL(drm_gem_object_release);
> > > 
> > > diff --git a/drivers/gpu/drm/drm_internal.h
> > > b/drivers/gpu/drm/drm_internal.h index 51a2055c8f18..8259622f9ab6 100644
> > > --- a/drivers/gpu/drm/drm_internal.h
> > > +++ b/drivers/gpu/drm/drm_internal.h
> > > @@ -137,6 +137,10 @@ int drm_gem_pin(struct drm_gem_object *obj);
> > > 
> > >  void drm_gem_unpin(struct drm_gem_object *obj);
> > >  void *drm_gem_vmap(struct drm_gem_object *obj);
> > >  void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr);
> > > 
> > > +int drm_bo_set_label_ioctl(struct drm_device *dev, void *data,
> > > + struct drm_file *file_priv);
> > > +int drm_gem_set_label(struct drm_device *dev, struct 

Re: [PATCH 13/14] drm/amd/display: Recalculate VCPI slots for new DSC connectors

[Lyude Paul]

...
yikes
I need to apologize because I was going through my email and I realized you
_did_ respond to me earlier regarding some of these questions, it just appears
the reply fell through the cracks and somehow I didn't notice it until just
now. Sorry about that!

I'm still not sure I understand why this is a future enhancement? Everything
we need to implement these helpers should already be here, it's just a matter
of keeping track who has dsc enabled where in the mst atomic state

On Mon, 2019-10-07 at 17:52 -0400, Lyude Paul wrote:
> Ok, let's stop and slow down for a minute here since I've repeated myself a
> few times, and I'd like to figure out what's actually happening here and
> whether I'm not being clear enough with my explanations, or if there's just
> some misunderstanding here.
> 
> I'll start of by trying my best to explain my hesistance in accepting these
> patches. To be clear, MST with DSC is a big thing in terms of impact.
> There's
> a lot of things to consider:
>  * What should the default DSC policy for MST be? Should we keep it on by
>default so that we don't need to trigger a large number of PBN
>recalculations and enable DSC on a per-case basis, or are there
> legitimate
>reasons to keep DSC off by default (such as potential issues with
> lossiness
>down the line).
>  * We have other drivers in the tree that are planning on implementing MST
> DSC
>quite soon. Chances are they'll be implementing helpers to do this so
> this
>can be supported across the DRM tree, and chances are we'll just have to
>rewrite and remove most of this code in that case once they do.
>  * amdgpu already has a _lot_ of code that doesn't need to, and shouldn't be
>there. This ranges from various DC specific helpers that are essentially
>the same as the helpers that we already implement in the rest of DRM, to
>misuses of various kernel APIs, and a complete lack of any kind of code
>style (at least the last time that I checked) in or out of DC. This makes
>the driver more difficult for people who don't work at AMD but are well
>accustomed to working on the rest of the kernel, e.g. myself and others,
>and it's a problem that seriously needs to be solved. Adding more
> redundant
>DP helpers that will inevitably get removed is just making the problem
>worse.
> 
> With all of this being said: I've been asking the same questions about this
> patch throughout the whole patch series so far (since it got passed off to
> you
> from David's internship ending):
> 
>  * Can we keep DSC enabled by default, so that these patches aren't needed?
>  * If we can't, can we move this into common code so that other drivers can
>use it?
> The problem is I haven't received any responses to these questions, other
> then
> being told by David a month or two ago that it would be expedient to just
> accept the patches and move on. Honestly, if discussion had been had about
> the
> changes I requested, I would have given my r-b a long time ago. In fact-I
> really want to give it now! There's multiple patches in this series that
> would
> be very useful for other things that are being worked on at the moment, such
> as the changes to drm_dp_dpcd_read/drm_dp_dpcd_write(). But I really think
> this needs to be discussed first, and I'm worried that just going ahead and
> giving my R-B before they have been discussed will further encourage rushing
> changes like this in the future and continually building more tech debt for
> ourselves.
> 
> Please note as well: if anything I've asked for is confusing, or you don't
> understand what I'm asking or looking for I am more then willing to help
> explain things and help out as best as I can. I understand that a lot of the
> developers working on DRM at AMD may have more experience in Windows and Mac
> land and as a result, trying to get used to the way that we do things in the
> Linux kernel can be a confusing endeavor. I'm more then happy to help out
> with
> this wherever I can, all you need to do is ask. Asking a few questions about
> something you aren't sure you understand can save both of us a lot of time,
> and avoid having to go through this many patch respins.
> 
> In the mean time, I'd be willing to look at what patches from this series
> that
> have already been reviewed which could be pushed to drm-misc or friends in
> the
> mean time to speed things up a bit.
> 
> On Tue, 2019-10-01 at 12:17 -0400, mikita.lip...@amd.com wrote:
> > From: Mikita Lipski 
> > 
> > Since for DSC MST connector's PBN is claculated differently
> > due to compression, we have to recalculate both PBN and
> > VCPI slots for that connector.
> > 
> > This patch proposes to use similair logic as in
> > dm_encoder_helper_atomic_chek, but since we do not know which
> > connectors will have DSC enabled we have to recalculate PBN only
> > after that's determined, which is done in
> > compute_mst_dsc_configs_for_state.
> > 
> > Cc: Jerry Zuo 
> > Cc: 

Re: [PATCH] drm/msm: Use the correct dma_sync calls harder

[Fabio Estevam]

Hi Rob,

On Wed, Sep 4, 2019 at 2:19 PM Rob Clark  wrote:
>
> From: Rob Clark 
>
> Looks like the dma_sync calls don't do what we want on armv7 either.
> Fixes:
>
>   Unable to handle kernel paging request at virtual address 50001000
>   pgd = (ptrval)
>   [50001000] *pgd=
>   Internal error: Oops: 805 [#1] SMP ARM
>   Modules linked in:
>   CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.3.0-rc6-00271-g9f159ae07f07 #4
>   Hardware name: Freescale i.MX53 (Device Tree Support)
>   PC is at v7_dma_clean_range+0x20/0x38
>   LR is at __dma_page_cpu_to_dev+0x28/0x90
>   pc : []lr : []psr: 2013
>   sp : d80b5a88  ip : de96c000  fp : d840ce6c
>   r10:   r9 : 0001  r8 : d843e010
>   r7 :   r6 : 8000  r5 : ddb6c000  r4 : 
>   r3 : 003f  r2 : 0040  r1 : 50008000  r0 : 50001000
>   Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
>   Control: 10c5387d  Table: 70004019  DAC: 0051
>   Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
>
> Signed-off-by: Rob Clark 
> Fixes: 3de433c5b38a ("drm/msm: Use the correct dma_sync calls in msm_gem")
> Tested-by: Fabio Estevam 

I see this one got applied in linux-next already.
Could it be sent to 5.4-rc, please?

mx53 boards cannot boot in mainline because of this.

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

Re: [PATCH v3 0/8] drm/omap: OMAP_BO flags

[Jean-Jacques Hiblot]

On 07/10/2019 14:16, Tomi Valkeinen wrote:

On 07/10/2019 14:25, Jean-Jacques Hiblot wrote:

A first version of this work had been sent by Tomi Valkeinen in may 2017
(https://www.spinics.net/lists/dri-devel/msg140663.html).

This series adds a few new OMAP_BO flags to help the userspace manage
situations where it needs to use lots of buffers, and would currently 
run
out of TILER space. The TILER space is limited to mapping 128MB at 
any given

time and some applications might need more.

This seres is also the opportunity to do some cleanup in the flags and
improve the comments describing them.

The user-space patches for libdrm, although ready, haven't been 
posted yet.

It will be be done when this series have been discussed and hopefully in
the process of getting merged.

JJ

changes in v3:
- rebase on top of v5.4-rc2
- Document omap_gem_new() and the new flags using the kernel-doc format

changes in v2:
- fixed build error that crept in during rebase before sending (sorry
   about that)
- rework the refcount part a bit.

Jean-Jacques Hiblot (1):
   drm/omap: use refcount API to track the number of users of dma_addr

Tomi Valkeinen (7):
   drm/omap: add omap_gem_unpin_locked()
   drm/omap: accept NULL for dma_addr in omap_gem_pin
   drm/omap: cleanup OMAP_BO flags
   drm/omap: remove OMAP_BO_TILED define
   drm/omap: cleanup OMAP_BO_SCANOUT use
   drm/omap: add omap_gem_validate_flags()
   drm/omap: add OMAP_BO flags to affect buffer allocation

  drivers/gpu/drm/omapdrm/omap_dmm_tiler.h  |   2 +-
  drivers/gpu/drm/omapdrm/omap_fb.c |   6 +-
  drivers/gpu/drm/omapdrm/omap_gem.c    | 191 --
  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |   2 +-
  include/uapi/drm/omap_drm.h   |  27 ++-
  5 files changed, 164 insertions(+), 64 deletions(-)


Thanks! This looks good to me. One comment, though:

Some people may have different opinions on how to manage other 
people's patches, but here's mine:


If you have made changes to a patch from someone else (me, in this 
case), other than really trivial typo fixes or such, you should add 
your signed-off-by.


Also, if you change the patch in a way that might make it behave 
differently than the original, you should change the ownership to 
yourself, drop the original signed-off-by, and perhaps say in the desc 
that the original was written by xyz. I don't want "my" patch to cause 
kernel crashes, if it's really not my code =).
Actually I didn't touch those patches a lot. Apart from the first one 
(the atomic stuff) and the kernel-doc style comment in the last patch, 
it is pretty much the stuff that you authored and is now part of TI's tree.


Actually, I see we now have "Co-developed-by" documented in 
Documentation/process/5.Posting.rst. That may be suitable here.


And for the patches that you didn't touch, I'm sure you've still gone 
through those, so you could add your reviewed-by.


Done.

Thanks

JJ



 Tomi


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

Re: [v3,8/8] drm/omap: add OMAP_BO flags to affect buffer allocation

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:

From: Tomi Valkeinen 

On SoCs with DMM/TILER, we have two ways to allocate buffers: normal
dma_alloc or via DMM (which basically functions as an IOMMU). DMM can
map 128MB at a time, and we only map the DMM buffers when they are used
(i.e. not at alloc time). If DMM is present, omapdrm always uses DMM.

There are use cases that require lots of big buffers that are being used
at the same time by different IPs. At the moment the userspace has a
hard maximum of 128MB.

This patch adds three new flags that can be used by the userspace to
solve the situation:

OMAP_BO_MEM_CONTIG: The driver will use dma_alloc to get the memory.
This can be used to avoid DMM if the userspace knows it needs more than
128M of memory at the same time.

OMAP_BO_MEM_DMM: The driver will use DMM to get the memory. There's not
much use for this flag at the moment, as on platforms with DMM it is
used by default, but it's here for completeness.

OMAP_BO_MEM_PIN: The driver will pin the memory at alloc time, and keep
it pinned. This can be used to 1) get an error at alloc time if DMM
space is full, and 2) get rid of the constant pin/unpin operations which
may have some effect on performance.

If none of the flags are given, the behavior is the same as currently.

Signed-off-by: Tomi Valkeinen 
---
  drivers/gpu/drm/omapdrm/omap_gem.c | 54 --
  include/uapi/drm/omap_drm.h|  9 +
  2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index e518d93ca6df..bf18dfe2b689 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1097,6 +1097,9 @@ void omap_gem_free_object(struct drm_gem_object *obj)
list_del(_obj->mm_list);
mutex_unlock(>list_lock);
  
+	if (omap_obj->flags & OMAP_BO_MEM_PIN)

+   omap_gem_unpin_locked(obj);
+
/*
 * We own the sole reference to the object at this point, but to keep
 * lockdep happy, we must still take the omap_obj_lock to call
@@ -1147,10 +1150,19 @@ static bool omap_gem_validate_flags(struct drm_device 
*dev, u32 flags)
return false;
}
  
+	if ((flags & OMAP_BO_MEM_CONTIG) && (flags & OMAP_BO_MEM_DMM))

+   return false;
+
+   if ((flags & OMAP_BO_MEM_DMM) && !priv->usergart)
+   return false;
+
if (flags & OMAP_BO_TILED_MASK) {
if (!priv->usergart)
return false;
  
+		if (flags & OMAP_BO_MEM_CONTIG)

+   return false;
+
switch (flags & OMAP_BO_TILED_MASK) {
case OMAP_BO_TILED_8:
case OMAP_BO_TILED_16:
@@ -1165,7 +1177,34 @@ static bool omap_gem_validate_flags(struct drm_device 
*dev, u32 flags)
return true;
  }
  
-/* GEM buffer object constructor */

+/**
+ * omap_gem_new() - Create a new GEM buffer
+ * @dev: The DRM device
+ * @gsize: The requested size for the GEM buffer. If the buffer is tiled
+ * (2D buffer), the size is a pair of values: height and width
+ * expressed in pixels. If the buffers is not tiled, it is expressed
+ * in bytes.
+ * @flags: Flags give additionnal information about the allocation:
+ * OMAP_BO_TILED_x: use the TILER (2D buffers). The TILER container
+ *  unit can be 8, 16 or 32 bits. Cache is always disabled for
+ *  tiled buffers.
+ * OMAP_BO_SCANOUT: Scannout buffer, consummable by the DSS
+ * OMAP_BO_CACHED: Buffer CPU caching mode: cached
+ * OMAP_BO_WC: Buffer CPU caching mode: write-combined
+ * OMAP_BO_UNCACHED: Buffer CPU caching mode: uncached
+ * OMAP_BO_MEM_CONTIG: The driver will use dma_alloc to get the memory.
+ *  This can be used to avoid DMM if the userspace knows it needs
+ *  more than 128M of memory at the same time.
+ * OMAP_BO_MEM_DMM: The driver will use DMM to get the memory. There's
+ *  not much use for this flag at the moment, as on platforms with
+ *  DMM it is used by default, but it's here for completeness.
+ * OMAP_BO_MEM_PIN: The driver will pin the memory at alloc time, and
+ *  keep it pinned. This can be used to 1) get an error at alloc
+ *  time if DMM space is full, and 2) get rid of the constant
+ *  pin/unpin operations which may have some effect on performance.
+ *
+ * Return: The GEM buffer or NULL if the allocation failed
+ */
  struct drm_gem_object *omap_gem_new(struct drm_device *dev,
union omap_gem_size gsize, u32 flags)
  {
@@ -1193,7 +1232,8 @@ struct drm_gem_object *omap_gem_new(struct drm_device 
*dev,
 */
flags &= ~(OMAP_BO_CACHED|OMAP_BO_WC|OMAP_BO_UNCACHED);
flags |= tiler_get_cpu_cache_flags();
-   } else if ((flags & OMAP_BO_SCANOUT) && 

Re: [v3,7/8] drm/omap: add omap_gem_validate_flags()

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:

From: Tomi Valkeinen 

Add a helper function omap_gem_validate_flags() which validates the
omap_bo flags passed from the userspace.

Also drop the dev_err() message, as the userspace can cause that at
will.

Signed-off-by: Tomi Valkeinen 
---
  drivers/gpu/drm/omapdrm/omap_gem.c | 40 ++
  1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index 27e0a2f8508a..e518d93ca6df 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1133,6 +1133,38 @@ void omap_gem_free_object(struct drm_gem_object *obj)
kfree(omap_obj);
  }
  
+static bool omap_gem_validate_flags(struct drm_device *dev, u32 flags)

+{
+   struct omap_drm_private *priv = dev->dev_private;
+
+   switch (flags & OMAP_BO_CACHE_MASK) {
+   case OMAP_BO_CACHED:
+   case OMAP_BO_WC:
+   case OMAP_BO_CACHE_MASK:
+   break;
+
+   default:
+   return false;
+   }
+
+   if (flags & OMAP_BO_TILED_MASK) {
+   if (!priv->usergart)
+   return false;
+
+   switch (flags & OMAP_BO_TILED_MASK) {
+   case OMAP_BO_TILED_8:
+   case OMAP_BO_TILED_16:
+   case OMAP_BO_TILED_32:
+   break;
+
+   default:
+   return false;
+   }
+   }
+
+   return true;
+}
+
  /* GEM buffer object constructor */
  struct drm_gem_object *omap_gem_new(struct drm_device *dev,
union omap_gem_size gsize, u32 flags)
@@ -1144,13 +1176,11 @@ struct drm_gem_object *omap_gem_new(struct drm_device 
*dev,
size_t size;
int ret;
  
+	if (!omap_gem_validate_flags(dev, flags))

+   return NULL;
+
/* Validate the flags and compute the memory and cache flags. */
if (flags & OMAP_BO_TILED_MASK) {
-   if (!priv->usergart) {
-   dev_err(dev->dev, "Tiled buffers require DMM\n");
-   return NULL;
-   }
-
/*
 * Tiled buffers are always shmem paged backed. When they are
 * scanned out, they are remapped into DMM/TILER.

Reviewed-by: Jean-Jacques Hiblot 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,6/8] drm/omap: cleanup OMAP_BO_SCANOUT use

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:

From: Tomi Valkeinen 

omap_gem_new() has a comment about OMAP_BO_SCANOUT which does not make
sense. Also, for the TILER case, we drop OMAP_BO_SCANOUT flag for some
reason.

It's not clear what the original purpose of OMAP_BO_SCANOUT is, but
presuming it means "scanout buffer, something that can be consumed by
DSS", this patch cleans up the above issues.

Signed-off-by: Tomi Valkeinen 
---
  drivers/gpu/drm/omapdrm/omap_gem.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index 4e8fcfdff3a0..27e0a2f8508a 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -1155,7 +1155,6 @@ struct drm_gem_object *omap_gem_new(struct drm_device 
*dev,
 * Tiled buffers are always shmem paged backed. When they are
 * scanned out, they are remapped into DMM/TILER.
 */
-   flags &= ~OMAP_BO_SCANOUT;
flags |= OMAP_BO_MEM_SHMEM;
  
  		/*

@@ -1166,9 +1165,8 @@ struct drm_gem_object *omap_gem_new(struct drm_device 
*dev,
flags |= tiler_get_cpu_cache_flags();
} else if ((flags & OMAP_BO_SCANOUT) && !priv->has_dmm) {
/*
-* OMAP_BO_SCANOUT hints that the buffer doesn't need to be
-* tiled. However, to lower the pressure on memory allocation,
-* use contiguous memory only if no TILER is available.
+* If we don't have DMM, we must allocate scanout buffers
+* from contiguous DMA memory.
 */
flags |= OMAP_BO_MEM_DMA_API;
} else if (!(flags & OMAP_BO_MEM_DMABUF)) {

Reviewed-by: Jean-Jacques Hiblot 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,5/8] drm/omap: remove OMAP_BO_TILED define

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:

From: Tomi Valkeinen 

OMAP_BO_TILED does not make sense, as OMAP_BO_TILED_* values are not
bitmasks but normal values. As we already have OMAP_BO_TILED_MASK for
the mask, we can remove OMAP_BO_TILED and use OMAP_BO_TILED_MASK
instead.

Signed-off-by: Tomi Valkeinen 
---
  drivers/gpu/drm/omapdrm/omap_dmm_tiler.h  |  2 +-
  drivers/gpu/drm/omapdrm/omap_fb.c |  6 +++---
  drivers/gpu/drm/omapdrm/omap_gem.c| 18 +-
  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |  2 +-
  include/uapi/drm/omap_drm.h   |  1 -
  5 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h 
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
index 835e6654fa82..43c1d096b021 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.h
@@ -113,7 +113,7 @@ extern struct platform_driver omap_dmm_driver;
  /* GEM bo flags -> tiler fmt */
  static inline enum tiler_fmt gem2fmt(u32 flags)
  {
-   switch (flags & OMAP_BO_TILED) {
+   switch (flags & OMAP_BO_TILED_MASK) {
case OMAP_BO_TILED_8:
return TILFMT_8BIT;
case OMAP_BO_TILED_16:
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c 
b/drivers/gpu/drm/omapdrm/omap_fb.c
index 1b8b5108caf8..7403316088b8 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -95,7 +95,7 @@ static u32 get_linear_addr(struct drm_framebuffer *fb,
  
  bool omap_framebuffer_supports_rotation(struct drm_framebuffer *fb)

  {
-   return omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED;
+   return omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED_MASK;
  }
  
  /* Note: DRM rotates counter-clockwise, TILER & DSS rotates clockwise */

@@ -154,7 +154,7 @@ void omap_framebuffer_update_scanout(struct drm_framebuffer 
*fb,
x = state->src_x >> 16;
y = state->src_y >> 16;
  
-	if (omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED) {

+   if (omap_gem_flags(fb->obj[0]) & OMAP_BO_TILED_MASK) {
u32 w = state->src_w >> 16;
u32 h = state->src_h >> 16;
  
@@ -212,7 +212,7 @@ void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,

plane = _fb->planes[1];
  
  		if (info->rotation_type == OMAP_DSS_ROT_TILER) {

-   WARN_ON(!(omap_gem_flags(fb->obj[1]) & OMAP_BO_TILED));
+   WARN_ON(!(omap_gem_flags(fb->obj[1]) & 
OMAP_BO_TILED_MASK));
omap_gem_rotated_dma_addr(fb->obj[1], orient, x/2, y/2,
  >p_uv_addr);
} else {
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index a6562d23d314..4e8fcfdff3a0 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -196,7 +196,7 @@ static void omap_gem_evict(struct drm_gem_object *obj)
struct omap_gem_object *omap_obj = to_omap_bo(obj);
struct omap_drm_private *priv = obj->dev->dev_private;
  
-	if (omap_obj->flags & OMAP_BO_TILED) {

+   if (omap_obj->flags & OMAP_BO_TILED_MASK) {
enum tiler_fmt fmt = gem2fmt(omap_obj->flags);
int i;
  
@@ -324,7 +324,7 @@ size_t omap_gem_mmap_size(struct drm_gem_object *obj)

struct omap_gem_object *omap_obj = to_omap_bo(obj);
size_t size = obj->size;
  
-	if (omap_obj->flags & OMAP_BO_TILED) {

+   if (omap_obj->flags & OMAP_BO_TILED_MASK) {
/* for tiled buffers, the virtual size has stride rounded up
 * to 4kb.. (to hide the fact that row n+1 might start 16kb or
 * 32kb later!).  But we don't back the entire buffer with
@@ -513,7 +513,7 @@ vm_fault_t omap_gem_fault(struct vm_fault *vmf)
 * probably trigger put_pages()?
 */
  
-	if (omap_obj->flags & OMAP_BO_TILED)

+   if (omap_obj->flags & OMAP_BO_TILED_MASK)
ret = omap_gem_fault_2d(obj, vma, vmf);
else
ret = omap_gem_fault_1d(obj, vma, vmf);
@@ -786,7 +786,7 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t 
*dma_addr)
if (ret)
goto fail;
  
-			if (omap_obj->flags & OMAP_BO_TILED) {

+   if (omap_obj->flags & OMAP_BO_TILED_MASK) {
block = tiler_reserve_2d(fmt,
omap_obj->width,
omap_obj->height, 0);
@@ -892,7 +892,7 @@ int omap_gem_rotated_dma_addr(struct drm_gem_object *obj, 
u32 orient,
mutex_lock(_obj->lock);
  
  	if ((refcount_read(_obj->dma_addr_cnt) > 0) && omap_obj->block &&

-   (omap_obj->flags & OMAP_BO_TILED)) {
+   (omap_obj->flags & OMAP_BO_TILED_MASK)) {
*dma_addr = tiler_tsptr(omap_obj->block, orient, x, y);
ret = 0;
}

Re: [v3,4/8] drm/omap: cleanup OMAP_BO flags

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:

From: Tomi Valkeinen 

Reorder OMAP_BO flags and improve the comments.

Signed-off-by: Tomi Valkeinen 
---
  include/uapi/drm/omap_drm.h | 17 +
  1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/uapi/drm/omap_drm.h b/include/uapi/drm/omap_drm.h
index 1fccffef9e27..d8ee2f840697 100644
--- a/include/uapi/drm/omap_drm.h
+++ b/include/uapi/drm/omap_drm.h
@@ -38,19 +38,20 @@ struct drm_omap_param {
__u64 value;/* in (set_param), out (get_param) */
  };
  
-#define OMAP_BO_SCANOUT		0x0001	/* scanout capable (phys contiguous) */

-#define OMAP_BO_CACHE_MASK 0x0006  /* cache type mask, see cache 
modes */
-#define OMAP_BO_TILED_MASK 0x0f00  /* tiled mapping mask, see 
tiled modes */
+/* Scanout buffer, consumable by DSS */
+#define OMAP_BO_SCANOUT0x0001
  
-/* cache modes */

-#define OMAP_BO_CACHED 0x  /* default */
-#define OMAP_BO_WC 0x0002  /* write-combine */
-#define OMAP_BO_UNCACHED   0x0004  /* strongly-ordered (uncached) 
*/
+/* Buffer CPU caching mode: cached, write-combining or uncached. */
+#define OMAP_BO_CACHED 0x
+#define OMAP_BO_WC 0x0002
+#define OMAP_BO_UNCACHED   0x0004
+#define OMAP_BO_CACHE_MASK 0x0006
  
-/* tiled modes */

+/* Use TILER for the buffer. The TILER container unit can be 8, 16 or 32 bits. 
*/
  #define OMAP_BO_TILED_8   0x0100
  #define OMAP_BO_TILED_16  0x0200
  #define OMAP_BO_TILED_32  0x0300
+#define OMAP_BO_TILED_MASK 0x0f00
  #define OMAP_BO_TILED (OMAP_BO_TILED_8 | OMAP_BO_TILED_16 | 
OMAP_BO_TILED_32)
  
  union omap_gem_size {

Reviewed-by: Jean-Jacques Hiblot 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,3/8] drm/omap: accept NULL for dma_addr in omap_gem_pin

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:

From: Tomi Valkeinen 

Allow NULL to be passed in 'dma_addr' for omap_gem_pin(), in case the
caller does not need the dma_addr.

Signed-off-by: Tomi Valkeinen 
---
  drivers/gpu/drm/omapdrm/omap_gem.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index 9201c21e206f..a6562d23d314 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -819,9 +819,11 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t 
*dma_addr)
refcount_inc(_obj->dma_addr_cnt);
}
  
-		*dma_addr = omap_obj->dma_addr;

+   if (dma_addr)
+   *dma_addr = omap_obj->dma_addr;
} else if (omap_gem_is_contiguous(omap_obj)) {
-   *dma_addr = omap_obj->dma_addr;
+   if (dma_addr)
+   *dma_addr = omap_obj->dma_addr;
} else {
ret = -EINVAL;
goto fail;

Reviewed-by: Jean-Jacques Hiblot 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3,2/8] drm/omap: add omap_gem_unpin_locked()

[Jean-Jacques Hiblot]

On 07/10/2019 13:25, Jean-Jacques Hiblot wrote:

From: Tomi Valkeinen 

Add omap_gem_unpin_locked() which is a version of omap_gem_unpin() that
expects the caller to hold the omap_obj lock.

Signed-off-by: Tomi Valkeinen 
---
  drivers/gpu/drm/omapdrm/omap_gem.c | 24 +---
  1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c 
b/drivers/gpu/drm/omapdrm/omap_gem.c
index 51ede083..9201c21e206f 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -834,20 +834,16 @@ int omap_gem_pin(struct drm_gem_object *obj, dma_addr_t 
*dma_addr)
  }
  
  /**

- * omap_gem_unpin() - Unpin a GEM object from memory
+ * omap_gem_unpin_locked() - Unpin a GEM object from memory
   * @obj: the GEM object
   *
- * Unpin the given GEM object previously pinned with omap_gem_pin(). Pins are
- * reference-counted, the actualy unpin will only be performed when the number
- * of calls to this function matches the number of calls to omap_gem_pin().
+ * omap_gem_unpin() without locking.
   */
-void omap_gem_unpin(struct drm_gem_object *obj)
+static void omap_gem_unpin_locked(struct drm_gem_object *obj)
  {
struct omap_gem_object *omap_obj = to_omap_bo(obj);
int ret;
  
-	mutex_lock(_obj->lock);

-
if (refcount_dec_and_test(_obj->dma_addr_cnt)) {
ret = tiler_unpin(omap_obj->block);
if (ret) {
@@ -864,6 +860,20 @@ void omap_gem_unpin(struct drm_gem_object *obj)
}
  }
  
+/**

+ * omap_gem_unpin() - Unpin a GEM object from memory
+ * @obj: the GEM object
+ *
+ * Unpin the given GEM object previously pinned with omap_gem_pin(). Pins are
+ * reference-counted, the actual unpin will only be performed when the number
+ * of calls to this function matches the number of calls to omap_gem_pin().
+ */
+void omap_gem_unpin(struct drm_gem_object *obj)
+{
+   struct omap_gem_object *omap_obj = to_omap_bo(obj);
+
+   mutex_lock(_obj->lock);
+   omap_gem_unpin_locked(obj);
mutex_unlock(_obj->lock);
  }
  


Reviewed-by: Jean-Jacques Hiblot 

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

[Bug 111921] GPU crash on VegaM (amdgpu: The CS has been rejected)

[bugzilla-daemon]

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

--- Comment #3 from Rémi Verschelde  ---
I don't reset the GPU manually, no. I'm not sure why this happens, but I've had
such output in dmesg as far as I can remember (since I got this laptop in
March).

For the reference, I've been using kernel 5.1.20 and did not experience this
crash. I'm not sure yet it's conclusive to say it's a regression though, I will
test more in coming days.

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

[Bug 111913] AMD Navi10 GPU powerplay issues when using two DisplayPort connectors

[bugzilla-daemon]

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

--- Comment #8 from Stefan Rehm  ---
In my case the resolution of both monitors is 2560x1440

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

[Bug 111921] GPU crash on VegaM (amdgpu: The CS has been rejected)

[bugzilla-daemon]

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

--- Comment #2 from Andrey Grodzovsky  ---
Hey, I noticed a lot of 'amdgpu :01:00.0: GPU pci config reset' there.
Since I see no command submissions timeout errors it looks like you manually
tried to reset the GPU multiple times - on one of them there was a failure
after which the errors you described appeared. IS this correct ?

-- 
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: [v4] drm: two planes with the same zpos have undefined ordering

[Simon Ser]

On Tuesday, October 8, 2019 6:16 PM, Daniel Vetter  wrote:

> On Tue, Oct 8, 2019 at 5:11 PM Simon Ser cont...@emersion.fr wrote:
>
> > On Tuesday, October 8, 2019 6:03 PM, Daniel Vetter dan...@ffwll.ch wrote:
> >
> > > > > > > In any case, this doesn't change the patch itself. Probably 
> > > > > > > something worth
> > > > > > > mentionning in the commit message.
> > > > > >
> > > > > > Yes, recording these use cases would be very nice.
> > > > >
> > > > > There's a lot more hw that does the same tricks (qualcom is one for 
> > > > > sure).
> > > > > Imo before we encode this we should make sure that:
> > > > > a) everyone is happy with this new uapi
> > > >
> > > > Sorry, what new UAPI?
> > > > We're just trying to make the documentation match what currently
> > > > happens, right?
> > >
> > > It's so much new uapi that I've sent out a few reverts for 5.4 (it
> > > landed in that merge window) to undo the stuff the arm driver team has
> > > done (it didn't come with userspace, proper discussion on dri-devel,
> > > docs or testcases in igt). I also just spotted that a leftover is that
> > > arm/komeda still computes its own version of normalized_zpos, which
> > > probably should be ditched too (it's not actually different from the
> > > one in helpers without the reverted uapi).
> > > So yeah, separate patch :-)
> >
> > I don't get it. Do you want to split the docs changes for user-space,
> > only keeping the doc changes for drivers in this patch?
> > User-space could already see duplicate zpos because of the non-atomic
> > API. I don't think this introduces a new uAPI.
>
> I'm talking specifically about the "duplicated zpos values indicate
> special cloned planes like in the arm example" clarification. Not
> about splitting the zpos documentation any more, that's indeed just
> documenting existing uapi. But assigning the special meaning to
> duplicated zpos values (instead of just "can happen because non-atomic
> legacy userspace"), that is new uapi. Especially if we allow
> duplicated zpos for immutable properties (afaik that doesn't exist
> yet).

Oh, I see. That makes sense, will send a new version.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 111922] amdgpu fails to resume on 5.2 kernel [regression]

[bugzilla-daemon]

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

Bug ID: 111922
   Summary: amdgpu fails to resume on 5.2 kernel [regression]
   Product: DRI
   Version: DRI git
  Hardware: Other
OS: All
Status: NEW
  Severity: not set
  Priority: not set
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: pierre-bugzi...@ossman.eu

The upgrade to the 5.2 series of the kernel has unfortunately made my system
unusable. After resuming the system from suspend the display goes green and is
unresponsive locally. It is still up though so I was able to reach it via the
network and get some logs out of it.

Tested bad kernels:

5.2.15-200.fc30
5.2.9-200.fc30

Known good kernels:

5.0.17-300.fc30
5.1.18-300.fc30
5.1.20-300.fc30

Found some other bug reports both here and with Red Hat with similar warnings
in dmesg, but they seem to be failing right away and not after a suspend. So
I'm not sure if it's the same issue.

But report at Red Hat:

https://bugzilla.redhat.com/show_bug.cgi?id=1754252

(includes dmesg)

-- 
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/3] dma-buf: add dma_resv_ctx for deadlock handling

[Daniel Vetter]

On Wed, Sep 18, 2019 at 07:55:23PM +0200, Christian König wrote:
> The ww_mutex framework allows for detecting deadlocks when multiple
> threads try to acquire the same set of locks in different order.
> 
> The problem is that handling those deadlocks was the burden of the user of
> the ww_mutex implementation and at least some users didn't got that right
> on the first try.
> 
> So introduce a new dma_resv_ctx object which can be used to
> simplify the deadlock handling. This is done by tracking all locked
> dma_resv objects in the context as well as the last contended object.
> 
> When a deadlock occurse we now unlock all previously locked objects and
> acquire the contended lock in the slow path. After this is done -EDEADLK
> is still returned to signal that all other locks now need to be
> re-acquired again.
> 
> Signed-off-by: Christian König 

I pondered this quite a bit, some thoughts:

- doing this over dma-buf means we can only use this for the ww_mutx
  dance, not for anything else. Which means we need another layer on top
  for shared execbuf utils (which Gerd has started looking into, but I
  felt like not quite the right approach yet in his first draft series).

- With the ttm/gem merger we could just put this into drm_gem_object, and
  ttm/gem helpers could still use it. Plus we could build some shared
  execbuf utils on top of this, by essentially merging ttm_operation_ctx
  into drm_gem_operation_ctx. I think this would also simplify the ttm
  parameters a bit, since currently the ttm_operation_ctx doesn't have an
  explicit pointer to the ww_acquire_ctx (which feels like an oversight to
  me).

- Aside, quick question: Any reason why struct amdgpu_cs_parser doesn't
  subclass ttm_operation_ctx? From my naive understanding this would make
  tons of sense ...

- Maybe we could even build some lru/eviction helpers on top, perhaps with
  two lists, one for the set of buffers in the execbuf, the other for
  additional buffers we've reserved as part of the eviction dance (to
  solve the TODO in ttm_mem_evict_wait_busy).

- Only downside would be that drivers outside of drivers/gpu won't be able
  to use these helpers. I don't see any immediate nor near-future need for
  that. All other drivers use so little memory they don't need to
  participate in the multi-lock dance, they just pin the few buffers they
  need and call it a day.

Ofc not everything above would need to be done right away, that's more
ideas for todo.rst entries to make sure we all agree on the rough
direction.

Thoughts?

Also adding Gerd Hoffmann, since he looked into this.

Cheers, Daniel
> ---
>  drivers/dma-buf/Makefile   |   2 +-
>  drivers/dma-buf/dma-resv-ctx.c | 108 +
>  include/linux/dma-resv-ctx.h   |  68 +
>  include/linux/dma-resv.h   |   1 +
>  4 files changed, 178 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/dma-buf/dma-resv-ctx.c
>  create mode 100644 include/linux/dma-resv-ctx.h
> 
> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
> index 03479da06422..da7701c85de2 100644
> --- a/drivers/dma-buf/Makefile
> +++ b/drivers/dma-buf/Makefile
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
> -  dma-resv.o seqno-fence.o
> +  dma-resv.o dma-resv-ctx.o seqno-fence.o
>  obj-$(CONFIG_SYNC_FILE)  += sync_file.o
>  obj-$(CONFIG_SW_SYNC)+= sw_sync.o sync_debug.o
>  obj-$(CONFIG_UDMABUF)+= udmabuf.o
> diff --git a/drivers/dma-buf/dma-resv-ctx.c b/drivers/dma-buf/dma-resv-ctx.c
> new file mode 100644
> index ..cad10fa6f80b
> --- /dev/null
> +++ b/drivers/dma-buf/dma-resv-ctx.c
> @@ -0,0 +1,108 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2019 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN 

Re: [v4] drm: two planes with the same zpos have undefined ordering

[Daniel Vetter]

On Tue, Oct 8, 2019 at 5:11 PM Simon Ser  wrote:
>
> On Tuesday, October 8, 2019 6:03 PM, Daniel Vetter  wrote:
>
> > > > > > In any case, this doesn't change the patch itself. Probably 
> > > > > > something worth
> > > > > > mentionning in the commit message.
> > > > >
> > > > > Yes, recording these use cases would be very nice.
> > > >
> > > > There's a lot more hw that does the same tricks (qualcom is one for 
> > > > sure).
> > > > Imo before we encode this we should make sure that:
> > > > a) everyone is happy with this new uapi
> > >
> > > Sorry, what new UAPI?
> > > We're just trying to make the documentation match what currently
> > > happens, right?
> >
> > It's so much new uapi that I've sent out a few reverts for 5.4 (it
> > landed in that merge window) to undo the stuff the arm driver team has
> > done (it didn't come with userspace, proper discussion on dri-devel,
> > docs or testcases in igt). I also just spotted that a leftover is that
> > arm/komeda still computes its own version of normalized_zpos, which
> > probably should be ditched too (it's not actually different from the
> > one in helpers without the reverted uapi).
> >
> > So yeah, separate patch :-)
>
> I don't get it. Do you want to split the docs changes for user-space,
> only keeping the doc changes for drivers in this patch?
>
> User-space could already see duplicate zpos because of the non-atomic
> API. I don't think this introduces a new uAPI.

I'm talking specifically about the "duplicated zpos values indicate
special cloned planes like in the arm example" clarification. Not
about splitting the zpos documentation any more, that's indeed just
documenting existing uapi. But assigning the special meaning to
duplicated zpos values (instead of just "can happen because non-atomic
legacy userspace"), that is new uapi. Especially if we allow
duplicated zpos for immutable properties (afaik that doesn't exist
yet).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/panfrost: Quiet shrinker messages

[Robin Murphy]

As brought up on IRC, logging a vague and unattributed message for a
normal and expected operation looks a bit spammy. Use a dev_* variant
to clarify it as a driver message, and downgrade the level to debug to
avoid cluttering up end users' logs.

Signed-off-by: Robin Murphy 
---
 drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c 
b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
index 458f0fa68111..d1c0bd81c9f7 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
@@ -74,7 +74,7 @@ panfrost_gem_shrinker_scan(struct shrinker *shrinker, struct 
shrink_control *sc)
mutex_unlock(>shrinker_lock);
 
if (freed > 0)
-   pr_info_ratelimited("Purging %lu bytes\n", freed << PAGE_SHIFT);
+   dev_dbg(pfdev->dev, "Purging %lu bytes\n", freed << PAGE_SHIFT);
 
return freed;
 }
-- 
2.21.0.dirty

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

Re: [v4] drm: two planes with the same zpos have undefined ordering

[Simon Ser]

On Tuesday, October 8, 2019 6:03 PM, Daniel Vetter  wrote:

> > > > > In any case, this doesn't change the patch itself. Probably something 
> > > > > worth
> > > > > mentionning in the commit message.
> > > >
> > > > Yes, recording these use cases would be very nice.
> > >
> > > There's a lot more hw that does the same tricks (qualcom is one for sure).
> > > Imo before we encode this we should make sure that:
> > > a) everyone is happy with this new uapi
> >
> > Sorry, what new UAPI?
> > We're just trying to make the documentation match what currently
> > happens, right?
>
> It's so much new uapi that I've sent out a few reverts for 5.4 (it
> landed in that merge window) to undo the stuff the arm driver team has
> done (it didn't come with userspace, proper discussion on dri-devel,
> docs or testcases in igt). I also just spotted that a leftover is that
> arm/komeda still computes its own version of normalized_zpos, which
> probably should be ditched too (it's not actually different from the
> one in helpers without the reverted uapi).
>
> So yeah, separate patch :-)

I don't get it. Do you want to split the docs changes for user-space,
only keeping the doc changes for drivers in this patch?

User-space could already see duplicate zpos because of the non-atomic
API. I don't think this introduces a new uAPI.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] dt-bindings: arm: samsung: Force clkoutN names to be unique in PMU

[Krzysztof Kozlowski]

On Tue, Oct 08, 2019 at 05:04:56PM +0200, Krzysztof Kozlowski wrote:
> On Tue, Oct 08, 2019 at 09:38:15AM -0500, Rob Herring wrote:
> > Are you running using DT_SCHEMA_FILES? If so, you won't get the core schema.
> 
> Ah, yes, now I see proper errors. Thanks for pointing this.
> 
> I'll send next version of this patch only (if others are ok).
> 

Of course, there will be no v2, this patch can be just skipped.

Best regards,
Krzysztof

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

Re: [PATCH 1/3] dt-bindings: arm: samsung: Force clkoutN names to be unique in PMU

[Krzysztof Kozlowski]

On Tue, Oct 08, 2019 at 09:38:15AM -0500, Rob Herring wrote:
> Are you running using DT_SCHEMA_FILES? If so, you won't get the core schema.

Ah, yes, now I see proper errors. Thanks for pointing this.

I'll send next version of this patch only (if others are ok).

Best regards,
Krzysztof

Re: [RFC PATCH] drm/virtio: Export resource handles via DMA-buf API

[Daniel Vetter]

On Tue, Oct 08, 2019 at 07:49:39PM +0900, Tomasz Figa wrote:
> On Tue, Oct 8, 2019 at 7:03 PM Daniel Vetter  wrote:
> >
> > On Sat, Oct 05, 2019 at 02:41:54PM +0900, Tomasz Figa wrote:
> > > Hi Daniel, Gerd,
> > >
> > > On Tue, Sep 17, 2019 at 10:23 PM Daniel Vetter  wrote:
> > > >
> > > > On Thu, Sep 12, 2019 at 06:41:21PM +0900, Tomasz Figa wrote:
> > > > > This patch is an early RFC to judge the direction we are following in
> > > > > our virtualization efforts in Chrome OS. The purpose is to start a
> > > > > discussion on how to handle buffer sharing between multiple virtio
> > > > > devices.
> > > > >
> > > > > On a side note, we are also working on a virtio video decoder 
> > > > > interface
> > > > > and implementation, with a V4L2 driver for Linux. Those will be posted
> > > > > for review in the near future as well.
> > > > >
> > > > > Any feedback will be appreciated! Thanks in advance.
> > > > >
> > > > > ===
> > > > >
> > > > > With the range of use cases for virtualization expanding, there is 
> > > > > going
> > > > > to be more virtio devices added to the ecosystem. Devices such as 
> > > > > video
> > > > > decoders, encoders, cameras, etc. typically work together with the
> > > > > display and GPU in a pipeline manner, which can only be implemented
> > > > > efficiently by sharing the buffers between producers and consumers.
> > > > >
> > > > > Existing buffer management framework in Linux, such as the videobuf2
> > > > > framework in V4L2, implements all the DMA-buf handling inside generic
> > > > > code and do not expose any low level information about the buffers to
> > > > > the drivers.
> > > > >
> > > > > To seamlessly enable buffer sharing with drivers using such 
> > > > > frameworks,
> > > > > make the virtio-gpu driver expose the resource handle as the DMA 
> > > > > address
> > > > > of the buffer returned from the DMA-buf mapping operation. Arguably, 
> > > > > the
> > > > > resource handle is a kind of DMA address already, as it is the buffer
> > > > > identifier that the device needs to access the backing memory, which 
> > > > > is
> > > > > exactly the same role a DMA address provides for native devices.
> > > > >
> > > > > A virtio driver that does memory management fully on its own would 
> > > > > have
> > > > > code similar to following. The code is identical to what a regular
> > > > > driver for real hardware would do to import a DMA-buf.
> > > > >
> > > > > static int virtio_foo_get_resource_handle(struct virtio_foo *foo,
> > > > > struct dma_buf *dma_buf, u32 
> > > > > *id)
> > > > > {
> > > > >   struct dma_buf_attachment *attach;
> > > > >   struct sg_table *sgt;
> > > > >   int ret = 0;
> > > > >
> > > > >   attach = dma_buf_attach(dma_buf, foo->dev);
> > > > >   if (IS_ERR(attach))
> > > > >   return PTR_ERR(attach);
> > > > >
> > > > >   sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> > > > >   if (IS_ERR(sgt)) {
> > > > >   ret = PTR_ERR(sgt);
> > > > >   goto err_detach;
> > > > >   }
> > > > >
> > > > >   if (sgt->nents != 1) {
> > > > >   ret = -EINVAL;
> > > > >   goto err_unmap;
> > > > >   }
> > > > >
> > > > >   *id = sg_dma_address(sgt->sgl);
> > > >
> > > > I agree with Gerd, this looks pretty horrible to me.
> > > >
> > > > The usual way we've done these kind of special dma-bufs is:
> > > >
> > > > - They all get allocated at the same place, through some library or
> > > >   whatever.
> > > >
> > > > - You add a dma_buf_is_virtio(dma_buf) function, or maybe something that
> > > >   also upcasts or returns NULL, which checks for dma_buf->ops.
> > > >
> > >
> > > Thanks for a lot of valuable feedback and sorry for the late reply.
> > >
> > > While I agree that stuffing the resource ID in sg_dma_address() is
> > > quite ugly (for example, the regular address arithmetic doesn't work),
> > > I still believe we need to convey information about these buffers
> > > using regular kernel interfaces.
> > >
> > > Drivers in some subsystems like DRM tend to open code any buffer
> > > management and then it wouldn't be any problem to do what you
> > > suggested. However, other subsystems have generic frameworks for
> > > buffer management, like videobuf2 for V4L2. Those assume regular
> > > DMA-bufs that can be handled with regular dma_buf_() API and described
> > > using sgtables and/or pfn vectors and/or DMA addresses.
> >
> > "other subsystem sucks" doesn't sound like a good design paradigm to me.
> > Forced midlayers are a bad design decision isn't really new at all ...
> >
> 
> Sorry, I don't think that's an argument. There are various design
> aspects and for the scenarios for which V4L2 was designed, the other
> subsystems may actually "suck". Let's not derail the discussion into
> judging which subsystems are better or worse.
> 
> Those mid layers are not forced, you don't have to use videobuf2, but
> 

Re: [v4] drm: two planes with the same zpos have undefined ordering

[Daniel Vetter]

On Tue, Oct 8, 2019 at 1:39 PM Pekka Paalanen  wrote:
>
> On Tue, 8 Oct 2019 11:59:04 +0200
> Daniel Vetter  wrote:
>
> > On Mon, Sep 30, 2019 at 10:07:07AM +0300, Pekka Paalanen wrote:
> > > On Sun, 29 Sep 2019 20:30:44 +
> > > Simon Ser  wrote:
> > >
> > > > Hi,
> > > >
> > > > > On Mon, Sep 23, 2019 at 12:39:20PM +, Simon Ser wrote:
> > > > > > Currently the property docs don't specify whether it's okay for two 
> > > > > > planes to
> > > > > > have the same zpos value and what user-space should expect in this 
> > > > > > case.
> > > > > >
> > > > > > The rule mentionned in the past was to disambiguate with object 
> > > > > > IDs. However
> > > > > > some drivers break this rule (that's why the ordering is documented 
> > > > > > as
> > > > > > unspecified in case the zpos property is missing). Additionally it 
> > > > > > doesn't
> > > > > > really make sense for a driver to user identical zpos values if it 
> > > > > > knows their
> > > > > > relative position: the driver can just pick different values 
> > > > > > instead.
> > > > > >
> > > > > > So two solutions would make sense: either disallow completely 
> > > > > > identical zpos
> > > > > > values for two different planes, either make the ordering 
> > > > > > unspecified. To allow
> > > > > > drivers that don't know the relative ordering between two planes to 
> > > > > > still
> > > > > > expose the zpos property, choose the latter solution.
> > > > >
> > > > > Some Arm's usage cases about two planes with same zpos.
> > > > >
> > > > > - "Smart layer"
> > > > > which can accepts 4 different fbs with different src/display rect,
> > > > > but this smart layer has one restriction:
> > > > >
> > > > > 4 display rects must have no overlaps in V direction
> > > > > (A optimization for android window like Toolbar/Navigation bar)
> > > > >
> > > > > So when map this Smart-layer to drm world, it might be 4 different
> > > > > drm-planes, but with same zorder to indicate that these 4 planes are
> > > > > smart-laye planes.
> > > > >
> > > > > - "VR-Layer"
> > > > > One VR-layer comprises two different viewports which can be configured
> > > > > with totoally different fbs, src/display rect.
> > > > > we use two differernt drm-planes to drive on HW "VR-Layer", and these
> > > > > two drm-planes must be configured with same zpos.
> > > >
> > > > Thanks a lot for your feedback James, that's exactly what I was looking 
> > > > for.
> > > > Both of these use-cases make sense to me.
> > > >
> > > > I think user-space should be prepared to handle identical immutable 
> > > > zpos values.
> > > > Pekka and Daniel, thoughts?
> > >
> > > Hi,
> > >
> > > given those explained use cases, sure, I agree.
> > >
> > > > In any case, this doesn't change the patch itself. Probably something 
> > > > worth
> > > > mentionning in the commit message.
> > >
> > > Yes, recording these use cases would be very nice.
> >
> > There's a lot more hw that does the same tricks (qualcom is one for sure).
> > Imo before we encode this we should make sure that:
> > a) everyone is happy with this new uapi
>
> Sorry, what new UAPI?
>
> We're just trying to make the documentation match what currently
> happens, right?

It's so much new uapi that I've sent out a few reverts for 5.4 (it
landed in that merge window) to undo the stuff the arm driver team has
done (it didn't come with userspace, proper discussion on dri-devel,
docs or testcases in igt). I also just spotted that a leftover is that
arm/komeda still computes its own version of normalized_zpos, which
probably should be ditched too (it's not actually different from the
one in helpers without the reverted uapi).

So yeah, separate patch :-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v8,2/4] drm/panel: set display info in panel attach

[Sean Paul]

/snip

> > > > > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> > > > > index d16158deacdc..f3587a54b8ac 100644
> > > > > --- a/include/drm/drm_panel.h
> > > > > +++ b/include/drm/drm_panel.h
> > > > > @@ -141,6 +141,56 @@ struct drm_panel {
> > > > >*/
> > > > >   const struct drm_panel_funcs *funcs;
> > > > >
> > > >
> > > > All these new added members seems dupliated with drm_display_info,
> > > > So I think, can we add a new drm_plane_funcs func:
> > > >
> > > > int (*set_display_info)(struct drm_panel *panel,
> > > > struct drm_display_info *info);
> > >
> > > I don't disagree personally, since I originally wrote it this way, but
> > > 2 maintainers, Daniel Vetter and Thierry Reding, requested that it be
> > > changed. Unless that decision is reversed, I won't be changing this.
> > >
> >
> > Reading back the feedback on v1, I don't think anyone said they were against
> > storing a drm_display_info struct in drm_panel (no one really weighed in on
> > it one way or another). IMO duplicating a bunch of fields feels pretty icky.
> 
> Thanks for the review. Should we duplicate the entire struct, or just
> create a sub-struct, say, physical_properties that will be part of
> drm_display_info and drm_panel?

That's a good idea, but I think you can use the entire struct. Everything has
reasonable default values and I think it might be difficult to draw a line in
the sand as to which fields will or won't be useful for panels going forward.
Best for drivers to just treat the info in drm_display_info as best effort and
have good defaults IMO.

Sean

> 
> >
> > I think most fields in drm_display_info have pretty reasonable defaults, so 
> > I'd
> > recommend just storing a copy in drm_panel. As Thierry suggests, we can
> > populate the dt parts in probe (orientation) and then copy over all or a 
> > subset
> > of the struct to connector on attach.
> >
> > Sean
> >
> > > >
> > > > Then in drm_panel_attach(), via this interface the specific panel
> > > > driver can directly set connector->display_info. like
> > > >
> > > >...
> > > >if (panel->funcs && panel->funcs->set_display_info)
> > > > panel->funcs->unprepare(panel, connector->display_info);
> > > >...
> > > >
> > > > Thanks
> > > > James
> > > >
> > > > > + /**
> > > > > +  * @width_mm:
> > > > > +  *
> > > > > +  * Physical width in mm.
> > > > > +  */
> > > > > + unsigned int width_mm;
> > > > > +
> > > > > + /**
> > > > > +  * @height_mm:
> > > > > +  *
> > > > > +  * Physical height in mm.
> > > > > +  */
> > > > > + unsigned int height_mm;
> > > > > +
> > > > > + /**
> > > > > +  * @bpc:
> > > > > +  *
> > > > > +  * Maximum bits per color channel. Used by HDMI and DP outputs.
> > > > > +  */
> > > > > + unsigned int bpc;
> > > > > +
> > > > > + /**
> > > > > +  * @orientation
> > > > > +  *
> > > > > +  * Installation orientation of the panel with respect to the 
> > > > > chassis.
> > > > > +  */
> > > > > + int orientation;
> > > > > +
> > > > > + /**
> > > > > +  * @bus_formats
> > > > > +  *
> > > > > +  * Pixel data format on the wire.
> > > > > +  */
> > > > > + const u32 *bus_formats;
> > > > > +
> > > > > + /**
> > > > > +  * @num_bus_formats:
> > > > > +  *
> > > > > +  * Number of elements pointed to by @bus_formats
> > > > > +  */
> > > > > + unsigned int num_bus_formats;
> > > > > +
> > > > > + /**
> > > > > +  * @bus_flags:
> > > > > +  *
> > > > > +  * Additional information (like pixel signal polarity) for the 
> > > > > pixel
> > > > > +  * data on the bus.
> > > > > +  */
> > > > > + u32 bus_flags;
> > > > > +
> > > > >   /**
> > > > >* @list:
> > > > >*
> > >
> > > Thanks for the review
> >
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS

-- 
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] drm/komeda: Adds zorder support

[Daniel Vetter]

On Mon, May 20, 2019 at 5:33 AM Lowry Li (Arm Technology China)
 wrote:
>
> - Creates the zpos property.
> - Implement komeda_crtc_normalize_zpos to replace
> drm_atomic_normalize_zpos, reasons as the following:
>
> 1. The drm_atomic_normalize_zpos allows to configure same zpos for
> different planes, but komeda doesn't support such configuration.

Just stumbled over your custom normalized_zpos calculation, and it
looks very fishy. You seem to reinvent the normalized zpos
computation, which has the entire job of resolving duplicated zpos
values (which can happen with legacy kms). So the above is definitely
wrong.

Can you pls do a patch to remove your own code, and replace it with
helper usage? Or at least explain why exactly you can't use the
standard normalized zpos stuff and need your own (since it really
looks like just reinventing the same thing).
-Daniel

> 2. For further slave pipline case, Komeda need to calculate the
> max_slave_zorder, we will merge such calculation into
> komed_crtc_normalize_zpos to save a separated plane_state loop.
> 3. For feature none-scaling layer_split, which a plane_state will be
> assigned to two individual layers(left/right), which requires two
> normalize_zpos for this plane, plane_st->normalize_zpos will be used
> by left layer, normalize_zpos + 1 for right_layer.
>
> This patch series depends on:
> - https://patchwork.freedesktop.org/series/58710/
> - https://patchwork.freedesktop.org/series/59000/
> - https://patchwork.freedesktop.org/series/59002/
> - https://patchwork.freedesktop.org/series/59747/
> - https://patchwork.freedesktop.org/series/59915/
> - https://patchwork.freedesktop.org/series/60083/
> - https://patchwork.freedesktop.org/series/60698/
>
> Signed-off-by: Lowry Li (Arm Technology China) 
> ---
>  drivers/gpu/drm/arm/display/komeda/komeda_kms.c   | 90 
> ++-
>  drivers/gpu/drm/arm/display/komeda/komeda_kms.h   |  3 +
>  drivers/gpu/drm/arm/display/komeda/komeda_plane.c |  6 +-
>  3 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c 
> b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index 306ea06..0ec7665 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -100,6 +100,90 @@ static void komeda_kms_commit_tail(struct 
> drm_atomic_state *old_state)
> .atomic_commit_tail = komeda_kms_commit_tail,
>  };
>
> +static int komeda_plane_state_list_add(struct drm_plane_state *plane_st,
> +  struct list_head *zorder_list)
> +{
> +   struct komeda_plane_state *new = to_kplane_st(plane_st);
> +   struct komeda_plane_state *node, *last;
> +
> +   last = list_empty(zorder_list) ?
> +  NULL : list_last_entry(zorder_list, typeof(*last), zlist_node);
> +
> +   /* Considering the list sequence is zpos increasing, so if list is 
> empty
> +* or the zpos of new node bigger than the last node in list, no need
> +* loop and just insert the new one to the tail of the list.
> +*/
> +   if (!last || (new->base.zpos > last->base.zpos)) {
> +   list_add_tail(>zlist_node, zorder_list);
> +   return 0;
> +   }
> +
> +   /* Build the list by zpos increasing */
> +   list_for_each_entry(node, zorder_list, zlist_node) {
> +   if (new->base.zpos < node->base.zpos) {
> +   list_add_tail(>zlist_node, >zlist_node);
> +   break;
> +   } else if (node->base.zpos == new->base.zpos) {
> +   struct drm_plane *a = node->base.plane;
> +   struct drm_plane *b = new->base.plane;
> +
> +   /* Komeda doesn't support setting a same zpos for
> +* different planes.
> +*/
> +   DRM_DEBUG_ATOMIC("PLANE: %s and PLANE: %s are 
> configured same zpos: %d.\n",
> +a->name, b->name, node->base.zpos);
> +   return -EINVAL;
> +   }
> +   }
> +
> +   return 0;
> +}
> +
> +static int komeda_crtc_normalize_zpos(struct drm_crtc *crtc,
> + struct drm_crtc_state *crtc_st)
> +{
> +   struct drm_atomic_state *state = crtc_st->state;
> +   struct komeda_plane_state *kplane_st;
> +   struct drm_plane_state *plane_st;
> +   struct drm_framebuffer *fb;
> +   struct drm_plane *plane;
> +   struct list_head zorder_list;
> +   int order = 0, err;
> +
> +   DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> +crtc->base.id, crtc->name);
> +
> +   INIT_LIST_HEAD(_list);
> +
> +   /* This loop also added all effected planes into the new state */
> +   drm_for_each_plane_mask(plane, crtc->dev, crtc_st->plane_mask) {
> +   plane_st = 

Re: [PATCH v9 4/5] dt-bindings: backlight: Add led-backlight binding

[Rob Herring]

On Tue, Oct 8, 2019 at 8:30 AM Jean-Jacques Hiblot  wrote:
>
> Rob,
>
> On 08/10/2019 14:51, Jean-Jacques Hiblot wrote:
> > Hi Rob,
> >
> > On 07/10/2019 18:15, Rob Herring wrote:
> >> Please send DT bindings to DT list or it's never in my queue. IOW,
> >> send patches to the lists that get_maintainers.pl tells you to.
> >>
> >> On Mon, Oct 7, 2019 at 7:45 AM Jean-Jacques Hiblot 
> >> wrote:
> >>> Add DT binding for led-backlight.
> >>>
> >>> Signed-off-by: Jean-Jacques Hiblot 
> >>> Reviewed-by: Daniel Thompson 
> >>> Reviewed-by: Sebastian Reichel 
> >>> ---
> >>>   .../bindings/leds/backlight/led-backlight.txt | 28
> >>> +++
> >>>   1 file changed, 28 insertions(+)
> >>>   create mode 100644
> >>> Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> >> Please make this a DT schema.
> >
> > OK.
> >
> > BTW I used "make dt_binding_check" but had to fix a couple of YAMLs
> > file to get it to work. Do you have a kernel tree with already all the
> > YAML files in good shape ? Or do you want me to post the changes to
> > devicet...@vger.kernel.org ?
> >
> >
> >>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> >>> b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> >>> new file mode 100644
> >>> index ..4c7dfbe7f67a
> >>> --- /dev/null
> >>> +++
> >>> b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> >>> @@ -0,0 +1,28 @@
> >>> +led-backlight bindings
> >>> +
> >>> +This binding is used to describe a basic backlight device made of
> >>> LEDs.
> >>> +It can also be used to describe a backlight device controlled by
> >>> the output of
> >>> +a LED driver.
> >>> +
> >>> +Required properties:
> >>> +  - compatible: "led-backlight"
> >>> +  - leds: a list of LEDs
> >> 'leds' is already used as a node name and mixing is not ideal.
> >>
> >> We already have 'flash-leds' in use and with the same definition, so
> >> lets continue that and use 'backlight-leds'.
> > OK
>
> I am taking that back. I have added of_get_led() and devm_of_get_led()
> to the LED core to make it easier to get a LED from the DT. I modeled
> the interface like it is done for PWM, PHYs or clocks. The property
> containing list/array of phandle  is always named the same. To get one
> particular PWM/PHY/clock, a identifier (name or integer) must be provided.

It can be done as we do support that with '-gpios', but yes, it is a
bit more painful to deal with.

> So unless there is a strong incentive to do otherwise I's rather keep
> this name here.

In that case, this needs to be documented as a common LED binding, not
something hidden away in this binding.

Rob

Re: [PATCH v8 1/4] drm/panel: Add helper for reading DT rotation

[Sean Paul]

On Mon, Oct 07, 2019 at 03:12:00PM -0700, dbasehore . wrote:
> On Mon, Oct 7, 2019 at 9:38 AM Sean Paul  wrote:
> >
> > On Wed, Sep 25, 2019 at 03:58:30PM -0700, Derek Basehore wrote:
> > > This adds a helper function for reading the rotation (panel
> > > orientation) from the device tree.
> > >
> > > Signed-off-by: Derek Basehore 
> > > Reviewed-by: Sam Ravnborg 
> >
> > The patch LGTM, but I don't see it used anywhere later in the patch? Is 
> > there a
> > panel driver incoming?
> 
> Yeah, the boe-tv101wum-nl6 panel will use it. It's not in the patch
> currently sent upstream since I don't want to step on their toes, but
> I plan on sending a patch to add it as soon as that is merged.
> 
> I could hold back on this patch until that panel driver is merged too.

Yeah, I think it's probably best. I don't anticipate any changes will be
required, but it's always best to review the code end-to-end.

I haven't checked in on that review, but if it's close to landing, you can also
add a patch to this stack that is based on the in-flight patches. That way we 
can
get all the review out of the way and then when the panel lands, we can apply
your add-on with the rest of the series.

Sean

> Another alternative is to throw this into the generic drm_panel code,
> but there's no obvious place to put it since DRM seems to leave
> reading the DTS up to the panel drivers themselves.
> 
> >
> > Sean
> >
> > > ---
> > >  drivers/gpu/drm/drm_panel.c | 43 +
> > >  include/drm/drm_panel.h |  9 
> > >  2 files changed, 52 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> > > index 6b0bf42039cf..0909b53b74e6 100644
> > > --- a/drivers/gpu/drm/drm_panel.c
> > > +++ b/drivers/gpu/drm/drm_panel.c
> > > @@ -264,6 +264,49 @@ struct drm_panel *of_drm_find_panel(const struct 
> > > device_node *np)
> > >   return ERR_PTR(-EPROBE_DEFER);
> > >  }
> > >  EXPORT_SYMBOL(of_drm_find_panel);
> > > +
> > > +/**
> > > + * of_drm_get_panel_orientation - look up the orientation of the panel 
> > > through
> > > + * the "rotation" binding from a device tree node
> > > + * @np: device tree node of the panel
> > > + * @orientation: orientation enum to be filled in
> > > + *
> > > + * Looks up the rotation of a panel in the device tree. The orientation 
> > > of the
> > > + * panel is expressed as a property name "rotation" in the device tree. 
> > > The
> > > + * rotation in the device tree is counter clockwise.
> > > + *
> > > + * Return: 0 when a valid rotation value (0, 90, 180, or 270) is read or 
> > > the
> > > + * rotation property doesn't exist. -EERROR otherwise.
> > > + */
> > > +int of_drm_get_panel_orientation(const struct device_node *np,
> > > +  enum drm_panel_orientation *orientation)
> > > +{
> > > + int rotation, ret;
> > > +
> > > + ret = of_property_read_u32(np, "rotation", );
> > > + if (ret == -EINVAL) {
> > > + /* Don't return an error if there's no rotation property. */
> > > + *orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
> > > + return 0;
> > > + }
> > > +
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + if (rotation == 0)
> > > + *orientation = DRM_MODE_PANEL_ORIENTATION_NORMAL;
> > > + else if (rotation == 90)
> > > + *orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP;
> > > + else if (rotation == 180)
> > > + *orientation = DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP;
> > > + else if (rotation == 270)
> > > + *orientation = DRM_MODE_PANEL_ORIENTATION_LEFT_UP;
> > > + else
> > > + return -EINVAL;
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL(of_drm_get_panel_orientation);
> > >  #endif
> > >
> > >  MODULE_AUTHOR("Thierry Reding ");
> > > diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> > > index 624bd15ecfab..d16158deacdc 100644
> > > --- a/include/drm/drm_panel.h
> > > +++ b/include/drm/drm_panel.h
> > > @@ -34,6 +34,8 @@ struct drm_device;
> > >  struct drm_panel;
> > >  struct display_timing;
> > >
> > > +enum drm_panel_orientation;
> > > +
> > >  /**
> > >   * struct drm_panel_funcs - perform operations on a given panel
> > >   *
> > > @@ -165,11 +167,18 @@ int drm_panel_get_modes(struct drm_panel *panel);
> > >
> > >  #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
> > >  struct drm_panel *of_drm_find_panel(const struct device_node *np);
> > > +int of_drm_get_panel_orientation(const struct device_node *np,
> > > +  enum drm_panel_orientation *orientation);
> > >  #else
> > >  static inline struct drm_panel *of_drm_find_panel(const struct 
> > > device_node *np)
> > >  {
> > >   return ERR_PTR(-ENODEV);
> > >  }
> > > +static inline int of_drm_get_panel_orientation(const struct device_node 
> > > *np,
> > > + enum drm_panel_orientation *orientation)
> > > +{
> 

Re: [PATCH v9 4/5] dt-bindings: backlight: Add led-backlight binding

[Rob Herring]

On Tue, Oct 8, 2019 at 7:51 AM Jean-Jacques Hiblot  wrote:
>
> Hi Rob,
>
> On 07/10/2019 18:15, Rob Herring wrote:
> > Please send DT bindings to DT list or it's never in my queue. IOW,
> > send patches to the lists that get_maintainers.pl tells you to.
> >
> > On Mon, Oct 7, 2019 at 7:45 AM Jean-Jacques Hiblot  wrote:
> >> Add DT binding for led-backlight.
> >>
> >> Signed-off-by: Jean-Jacques Hiblot 
> >> Reviewed-by: Daniel Thompson 
> >> Reviewed-by: Sebastian Reichel 
> >> ---
> >>   .../bindings/leds/backlight/led-backlight.txt | 28 +++
> >>   1 file changed, 28 insertions(+)
> >>   create mode 100644 
> >> Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> > Please make this a DT schema.
>
> OK.
>
> BTW I used "make dt_binding_check" but had to fix a couple of YAMLs file
> to get it to work. Do you have a kernel tree with already all the YAML
> files in good shape ? Or do you want me to post the changes to
> devicet...@vger.kernel.org ?

linux-next is fixed now.

> >> diff --git 
> >> a/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt 
> >> b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> >> new file mode 100644
> >> index ..4c7dfbe7f67a
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.txt
> >> @@ -0,0 +1,28 @@
> >> +led-backlight bindings
> >> +
> >> +This binding is used to describe a basic backlight device made of LEDs.
> >> +It can also be used to describe a backlight device controlled by the 
> >> output of
> >> +a LED driver.
> >> +
> >> +Required properties:
> >> +  - compatible: "led-backlight"
> >> +  - leds: a list of LEDs
> > 'leds' is already used as a node name and mixing is not ideal.
> >
> > We already have 'flash-leds' in use and with the same definition, so
> > lets continue that and use 'backlight-leds'.
> OK
> >
> >> +
> >> +Optional properties:
> >> +  - brightness-levels: Array of distinct brightness levels. The levels 
> >> must be
> >> +   in the range accepted by the underlying LED 
> >> devices.
> >> +   This is used to translate a backlight brightness 
> >> level
> >> +   into a LED brightness level. If it is not 
> >> provided, the
> >> +   identity mapping is used.
> >> +
> >> +  - default-brightness-level: The default brightness level.
> > You can just assume these 2 get a common schema at some point. So just
> > need to define any additional constraints if possible.
>
> Maybe we should keep them until such a common schema is written ?

I'm not saying to remove them, but you can just have a description if
there are no other constraints.

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

Re: liboutput: thoughts about shared library on top of DRM/KMS

[Daniel Vetter]

On Tue, Oct 8, 2019 at 4:36 PM Alex Deucher  wrote:
>
> On Tue, Oct 8, 2019 at 5:32 AM Daniel Stone  wrote:
> >
> > Hi,
> >
> > On Mon, 7 Oct 2019 at 19:16, Keith Packard  wrote:
> > > Daniel Stone  writes:
> > > > I think there would be a load of value in starting with simple helpers
> > > > which can be used independently of any larger scheme, tackling that
> > > > list above.
> > >
> > > Yeah, a helper library that didn't enforce at tonne of policy and just
> > > let the user glue things together on their own is probably going to be
> > > more generally usable by existing and new systems.
> >
> > To elaborate a little bit, one of the reasons I'm loath to hide
> > complexity like transforms, colour management, and timing away in an
> > encapsulated lower layer, is because I have to expose all those
> > details anyway. Ultimately to make those work properly, we'll require
> > awareness not just in the compositor itself, but pushed through to
> > clients.
> >
> > Wayland already has facility for informing clients about output
> > transforms so they can render pre-rotated and avoid the
> > compositor-side transform; in order to make HDR and other colour
> > management (e.g. just simple calibration) properly we need to have
> > full plumbing back through to clients; doing timing properly,
> > particularly for multiple simultaneous clients, also requires a fair
> > bit of mechanics and back-and-forth.
> >
> > There's a lot that we could usefully share between all the users, and
> > having a shared library to help with that would be great. But the
> > thought of tucking it all away in an opaque layer which (*waves
> > hands*) just does it, gives me cold EGLStreams sweats.
> >
> > Maybe a good place to start is if we all listed the bits of code which
> > we'd be delighted to jettison?
>
> On the flipside, it would be nice to have one set of code to do
> modesets from userspace.  Certainly simplifies QA since in theory we
> should be seeing the same sequences from all apps using the helpers
> rather than every app rolling their own and getting it subtly
> different enough that strange things happen with different
> compositors.

For atomic drivers, and atomic userspace this really doesn't matter
anymore. Because userspace just assembles an overall update, and the
kernel applies that update no matter in which order userspace listed
the property/value/object triples.

It does somewhat matter for legacy kms (but not from a driver
validation pov, anything legacy can do can also be done as a legit
request through atomic), but I really can't care about that one much.
Imo better to just cut everyone over to atomic. The only thing that
might make sense is the fake atomic support, where userspace tries to
not be too dumb about a transition on drivers without atomic (i.e.
shut down crtcs first, then light up the new ones, to avoid in-between
state that oversubscribes available hw resources). But that's really a
niche thing.

> While we are on the topic, it would be nice to have a central place
> for robustness/context lost handling for compositors so we can
> properly handle GPU resets.  Not sure that is really possible though.

I think this blows up the scope too much and will break the project.
In the past there's been attempts at a libcompositor (and you need
that much to recover from context loss), and that never got anywhere
at all. I think the modular helpers approach has a much bigger chance
at success.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] dt-bindings: arm: samsung: Force clkoutN names to be unique in PMU

[Rob Herring]

On Tue, Oct 8, 2019 at 9:29 AM Krzysztof Kozlowski  wrote:
>
> On Tue, Oct 08, 2019 at 09:17:16AM -0500, Rob Herring wrote:
> > On Tue, Oct 8, 2019 at 9:05 AM Rob Herring  wrote:
> > >
> > > On Tue, Oct 8, 2019 at 7:50 AM Krzysztof Kozlowski  
> > > wrote:
> > > >
> > > > On Tue, Oct 08, 2019 at 07:38:14AM -0500, Rob Herring wrote:
> > > > > On Fri, Oct 4, 2019 at 10:14 AM Krzysztof Kozlowski  
> > > > > wrote:
> > > > > >
> > > > > > The clkoutN names of clocks must be unique because they represent
> > > > > > unique inputs of clock multiplexer.
> > > > > >
> > > > > > Signed-off-by: Krzysztof Kozlowski 
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/arm/samsung/pmu.yaml | 6 --
> > > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml 
> > > > > > b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > > > index 73b56fc5bf58..d8e03716f5d2 100644
> > > > > > --- a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > > > @@ -53,8 +53,10 @@ properties:
> > > > > >List of clock names for particular CLKOUT mux inputs
> > > > > >  minItems: 1
> > > > > >  maxItems: 32
> > > > > > -items:
> > > > > > -  pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> > > > > > +allOf:
> > > > > > +  - items:
> > > > > > +  pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> > > > > > +  - uniqueItems: true
> > > > >
> > > > > You shouldn't need the 'allOf', just add uniqueItems at the same 
> > > > > level as items.
> > > >
> > > > If you mean something like:
> > > >   56 uniqueItems: true
> > > >   57 items:
> > > >   58   pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> > > >
> > > > Then the dt_binding_check fails:
> > > >
> > > > dev/linux/Documentation/devicetree/bindings/arm/samsung/pmu.yaml: 
> > > > properties:clock-names:
> > > > 'uniqueItems' is not one of ['$ref', 'additionalItems', 
> > > > 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 
> > > > 'default', 'dependencies', 'deprecated', 'description', 'else', 'enum', 
> > > > 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'not', 
> > > > 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 
> > > > 'then', 'type', 'typeSize', 'unevaluatedProperties']
> > >
> > > I can add it.
> > >
> > > The other option is to fix this in the clock schema. I'm not sure if
> > > there's a need for duplicate clock-names. Seems unlikely. I'll test
> > > that.
> >
> > Actually, clock-names is already set to be unique. Did you see otherwise?
>
> Yeah, I duplicated on purpose "clkout1" and it was not reported as an
> error. That's why I added "uniqueItems".

Are you running using DT_SCHEMA_FILES? If so, you won't get the core schema.

Rob

Re: liboutput: thoughts about shared library on top of DRM/KMS

[Alex Deucher]

On Tue, Oct 8, 2019 at 5:32 AM Daniel Stone  wrote:
>
> Hi,
>
> On Mon, 7 Oct 2019 at 19:16, Keith Packard  wrote:
> > Daniel Stone  writes:
> > > I think there would be a load of value in starting with simple helpers
> > > which can be used independently of any larger scheme, tackling that
> > > list above.
> >
> > Yeah, a helper library that didn't enforce at tonne of policy and just
> > let the user glue things together on their own is probably going to be
> > more generally usable by existing and new systems.
>
> To elaborate a little bit, one of the reasons I'm loath to hide
> complexity like transforms, colour management, and timing away in an
> encapsulated lower layer, is because I have to expose all those
> details anyway. Ultimately to make those work properly, we'll require
> awareness not just in the compositor itself, but pushed through to
> clients.
>
> Wayland already has facility for informing clients about output
> transforms so they can render pre-rotated and avoid the
> compositor-side transform; in order to make HDR and other colour
> management (e.g. just simple calibration) properly we need to have
> full plumbing back through to clients; doing timing properly,
> particularly for multiple simultaneous clients, also requires a fair
> bit of mechanics and back-and-forth.
>
> There's a lot that we could usefully share between all the users, and
> having a shared library to help with that would be great. But the
> thought of tucking it all away in an opaque layer which (*waves
> hands*) just does it, gives me cold EGLStreams sweats.
>
> Maybe a good place to start is if we all listed the bits of code which
> we'd be delighted to jettison?

On the flipside, it would be nice to have one set of code to do
modesets from userspace.  Certainly simplifies QA since in theory we
should be seeing the same sequences from all apps using the helpers
rather than every app rolling their own and getting it subtly
different enough that strange things happen with different
compositors.

While we are on the topic, it would be nice to have a central place
for robustness/context lost handling for compositors so we can
properly handle GPU resets.  Not sure that is really possible though.

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

Re: [PATCH 1/3] dt-bindings: arm: samsung: Force clkoutN names to be unique in PMU

[Krzysztof Kozlowski]

On Tue, Oct 08, 2019 at 09:17:16AM -0500, Rob Herring wrote:
> On Tue, Oct 8, 2019 at 9:05 AM Rob Herring  wrote:
> >
> > On Tue, Oct 8, 2019 at 7:50 AM Krzysztof Kozlowski  wrote:
> > >
> > > On Tue, Oct 08, 2019 at 07:38:14AM -0500, Rob Herring wrote:
> > > > On Fri, Oct 4, 2019 at 10:14 AM Krzysztof Kozlowski  
> > > > wrote:
> > > > >
> > > > > The clkoutN names of clocks must be unique because they represent
> > > > > unique inputs of clock multiplexer.
> > > > >
> > > > > Signed-off-by: Krzysztof Kozlowski 
> > > > > ---
> > > > >  Documentation/devicetree/bindings/arm/samsung/pmu.yaml | 6 --
> > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml 
> > > > > b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > > index 73b56fc5bf58..d8e03716f5d2 100644
> > > > > --- a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > > +++ b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > > @@ -53,8 +53,10 @@ properties:
> > > > >List of clock names for particular CLKOUT mux inputs
> > > > >  minItems: 1
> > > > >  maxItems: 32
> > > > > -items:
> > > > > -  pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> > > > > +allOf:
> > > > > +  - items:
> > > > > +  pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> > > > > +  - uniqueItems: true
> > > >
> > > > You shouldn't need the 'allOf', just add uniqueItems at the same level 
> > > > as items.
> > >
> > > If you mean something like:
> > >   56 uniqueItems: true
> > >   57 items:
> > >   58   pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> > >
> > > Then the dt_binding_check fails:
> > >
> > > dev/linux/Documentation/devicetree/bindings/arm/samsung/pmu.yaml: 
> > > properties:clock-names:
> > > 'uniqueItems' is not one of ['$ref', 'additionalItems', 
> > > 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 
> > > 'dependencies', 'deprecated', 'description', 'else', 'enum', 'items', 
> > > 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'not', 'oneOf', 
> > > 'pattern', 'patternProperties', 'properties', 'required', 'then', 'type', 
> > > 'typeSize', 'unevaluatedProperties']
> >
> > I can add it.
> >
> > The other option is to fix this in the clock schema. I'm not sure if
> > there's a need for duplicate clock-names. Seems unlikely. I'll test
> > that.
> 
> Actually, clock-names is already set to be unique. Did you see otherwise?

Yeah, I duplicated on purpose "clkout1" and it was not reported as an
error. That's why I added "uniqueItems".

Best regards,
Krzysztof

Re: [PATCH] drm/amdgpu/display: make various arrays static, makes object smaller

[Harry Wentland]

On 2019-10-08 10:00 a.m., Joe Perches wrote:
> On Tue, 2019-10-08 at 13:56 +, Harry Wentland wrote:
> []
>>> diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c 
>>> b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
>> []
>>> @@ -2745,7 +2745,7 @@ static enum bp_result bios_get_board_layout_info(
>>> struct bios_parser *bp;
>>> enum bp_result record_result;
>>>  
>>> -   const unsigned int slot_index_to_vbios_id[MAX_BOARD_SLOTS] = {
>>> +   static const unsigned int slot_index_to_vbios_id[MAX_BOARD_SLOTS] = {
>>
>> Won't this break the multi-GPU case where you'll have multiple driver
>> instances with different layout?
> 
> As the array is read-only, how could that happen?
> 

You're right.

Patch is
Reviewed-by: Harry Wentland 

Harry

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

Re: [PATCH 1/3] dt-bindings: arm: samsung: Force clkoutN names to be unique in PMU

[Rob Herring]

On Tue, Oct 8, 2019 at 9:05 AM Rob Herring  wrote:
>
> On Tue, Oct 8, 2019 at 7:50 AM Krzysztof Kozlowski  wrote:
> >
> > On Tue, Oct 08, 2019 at 07:38:14AM -0500, Rob Herring wrote:
> > > On Fri, Oct 4, 2019 at 10:14 AM Krzysztof Kozlowski  
> > > wrote:
> > > >
> > > > The clkoutN names of clocks must be unique because they represent
> > > > unique inputs of clock multiplexer.
> > > >
> > > > Signed-off-by: Krzysztof Kozlowski 
> > > > ---
> > > >  Documentation/devicetree/bindings/arm/samsung/pmu.yaml | 6 --
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml 
> > > > b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > index 73b56fc5bf58..d8e03716f5d2 100644
> > > > --- a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > +++ b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > @@ -53,8 +53,10 @@ properties:
> > > >List of clock names for particular CLKOUT mux inputs
> > > >  minItems: 1
> > > >  maxItems: 32
> > > > -items:
> > > > -  pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> > > > +allOf:
> > > > +  - items:
> > > > +  pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> > > > +  - uniqueItems: true
> > >
> > > You shouldn't need the 'allOf', just add uniqueItems at the same level as 
> > > items.
> >
> > If you mean something like:
> >   56 uniqueItems: true
> >   57 items:
> >   58   pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> >
> > Then the dt_binding_check fails:
> >
> > dev/linux/Documentation/devicetree/bindings/arm/samsung/pmu.yaml: 
> > properties:clock-names:
> > 'uniqueItems' is not one of ['$ref', 'additionalItems', 
> > 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 
> > 'dependencies', 'deprecated', 'description', 'else', 'enum', 'items', 'if', 
> > 'minItems', 'minimum', 'maxItems', 'maximum', 'not', 'oneOf', 'pattern', 
> > 'patternProperties', 'properties', 'required', 'then', 'type', 'typeSize', 
> > 'unevaluatedProperties']
>
> I can add it.
>
> The other option is to fix this in the clock schema. I'm not sure if
> there's a need for duplicate clock-names. Seems unlikely. I'll test
> that.

Actually, clock-names is already set to be unique. Did you see otherwise?

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

Re: [PATCHv2 7/7] drm/omap: hdmi4: fix use of uninitialized var

[Tomi Valkeinen]

On 08/10/2019 17:13, Tony Lindgren wrote:

* Tomi Valkeinen  [190930 10:38]:

If use_mclk is false, mclk_mode is written to a register without
initialization. This doesn't cause any ill effects as the written value
is not used when use_mclk is false.

To fix this, write use_mclk only when use_mclk is true.


Hey nice catch. Based on a quick test looks like this fixes an
issue where power consumption stays higher after using HDMI.

Would be nice to have merged in the v5.4-rc series:

Tested-by: Tony Lindgren 


Really? Ok, well, then it was a good random find =).

I did already push this to drm-misc-next, as I thought it does not have 
any real effect. I'll check if it's ok to push to drm-misc-fixes too, 
with Cc stable.


 Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] dt-bindings: arm: samsung: Force clkoutN names to be unique in PMU

[Rob Herring]

On Tue, Oct 8, 2019 at 7:50 AM Krzysztof Kozlowski  wrote:
>
> On Tue, Oct 08, 2019 at 07:38:14AM -0500, Rob Herring wrote:
> > On Fri, Oct 4, 2019 at 10:14 AM Krzysztof Kozlowski  wrote:
> > >
> > > The clkoutN names of clocks must be unique because they represent
> > > unique inputs of clock multiplexer.
> > >
> > > Signed-off-by: Krzysztof Kozlowski 
> > > ---
> > >  Documentation/devicetree/bindings/arm/samsung/pmu.yaml | 6 --
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml 
> > > b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > index 73b56fc5bf58..d8e03716f5d2 100644
> > > --- a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > @@ -53,8 +53,10 @@ properties:
> > >List of clock names for particular CLKOUT mux inputs
> > >  minItems: 1
> > >  maxItems: 32
> > > -items:
> > > -  pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> > > +allOf:
> > > +  - items:
> > > +  pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
> > > +  - uniqueItems: true
> >
> > You shouldn't need the 'allOf', just add uniqueItems at the same level as 
> > items.
>
> If you mean something like:
>   56 uniqueItems: true
>   57 items:
>   58   pattern: '^clkout([0-9]|[12][0-9]|3[0-1])$'
>
> Then the dt_binding_check fails:
>
> dev/linux/Documentation/devicetree/bindings/arm/samsung/pmu.yaml: 
> properties:clock-names:
> 'uniqueItems' is not one of ['$ref', 'additionalItems', 
> 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 
> 'dependencies', 'deprecated', 'description', 'else', 'enum', 'items', 'if', 
> 'minItems', 'minimum', 'maxItems', 'maximum', 'not', 'oneOf', 'pattern', 
> 'patternProperties', 'properties', 'required', 'then', 'type', 'typeSize', 
> 'unevaluatedProperties']

I can add it.

The other option is to fix this in the clock schema. I'm not sure if
there's a need for duplicate clock-names. Seems unlikely. I'll test
that.

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

Re: [PATCH] drm: damage_helper: Fix race checking plane->state->fb

[Sean Paul]

On Tue, Oct 08, 2019 at 11:50:33AM +0200, Daniel Vetter wrote:
> On Thu, Sep 19, 2019 at 11:04:01AM -0400, Sean Paul wrote:
> > On Thu, Sep 05, 2019 at 12:41:27PM +0200, Daniel Vetter wrote:
> > > On Wed, Sep 4, 2019 at 10:29 PM Sean Paul  wrote:
> > > >
> > > > From: Sean Paul 
> > > >
> > > > Since the dirtyfb ioctl doesn't give us any hints as to which plane is
> > > > scanning out the fb it's marking as damaged, we need to loop through
> > > > planes to find it.
> > > >
> > > > Currently we just reach into plane state and check, but that can race
> > > > with another commit changing the fb out from under us. This patch locks
> > > > the plane before checking the fb and will release the lock if the plane
> > > > is not displaying the dirty fb.
> > > >
> > > > Fixes: b9fc5e01d1ce ("drm: Add helper to implement legacy dirtyfb")
> > > > Cc: Rob Clark 
> > > > Cc: Deepak Rawat 
> > > > Cc: Daniel Vetter 
> > > > Cc: Thomas Hellstrom 
> > > > Cc: Maarten Lankhorst 
> > > > Cc: Maxime Ripard 
> > > > Cc: Sean Paul 
> > > > Cc: David Airlie 
> > > > Cc: Daniel Vetter 
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Cc:  # v5.0+
> > > > Reported-by: Daniel Vetter 
> > > > Signed-off-by: Sean Paul 
> > > > ---
> > > >  drivers/gpu/drm/drm_damage_helper.c | 8 +++-
> > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_damage_helper.c 
> > > > b/drivers/gpu/drm/drm_damage_helper.c
> > > > index 8230dac01a89..3a4126dc2520 100644
> > > > --- a/drivers/gpu/drm/drm_damage_helper.c
> > > > +++ b/drivers/gpu/drm/drm_damage_helper.c
> > > > @@ -212,8 +212,14 @@ int drm_atomic_helper_dirtyfb(struct 
> > > > drm_framebuffer *fb,
> > > > drm_for_each_plane(plane, fb->dev) {
> > > > struct drm_plane_state *plane_state;
> > > >
> > > > -   if (plane->state->fb != fb)
> > > > +   ret = drm_modeset_lock(>mutex, 
> > > > state->acquire_ctx);
> > > > +   if (ret)
> > > 
> > > I think for paranoid safety we should have a WARN_ON(ret == -EALREADY)
> > > here. It should be impossible, but if it's not for some oddball
> > > reason, we'll blow up.
> > 
> > drm_modeset_lock eats EALREADY and returns 0 for that case, so I guess it
> > depends _how_ paranoid you want to be here :-)
> 
> Ah silly me, r-b as-is then.

Thanks, pushed to -misc-next

Sean

> -Daniel
> 
> > 
> > > 
> > > With that: Reviewed-by: Daniel Vetter 
> > > 
> > > But please give this a spin with some workloads and the ww_mutex
> > > slowpath debugging enabled, just to makre sure.
> > 
> > Ok, had a chance to run through some tests this morning with
> > CONFIG_DEBUG_WW_MUTEX_SLOWPATH and things lgtm
> > 
> > Sean
> > 
> > > -Daniel
> > > 
> > > > +   goto out;
> > > > +
> > > > +   if (plane->state->fb != fb) {
> > > > +   drm_modeset_unlock(>mutex);
> > > > continue;
> > > > +   }
> > > >
> > > > plane_state = drm_atomic_get_plane_state(state, plane);
> > > > if (IS_ERR(plane_state)) {
> > > > --
> > > > Sean Paul, Software Engineer, Google / Chromium OS
> > > >
> > > 
> > > 
> > > -- 
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > 
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
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] drm/amdgpu/display: make various arrays static, makes object smaller

[Joe Perches]

On Tue, 2019-10-08 at 13:56 +, Harry Wentland wrote:
[]
> > diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c 
> > b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> []
> > @@ -2745,7 +2745,7 @@ static enum bp_result bios_get_board_layout_info(
> > struct bios_parser *bp;
> > enum bp_result record_result;
> >  
> > -   const unsigned int slot_index_to_vbios_id[MAX_BOARD_SLOTS] = {
> > +   static const unsigned int slot_index_to_vbios_id[MAX_BOARD_SLOTS] = {
> 
> Won't this break the multi-GPU case where you'll have multiple driver
> instances with different layout?

As the array is read-only, how could that happen?


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

[Bug 111913] AMD Navi10 GPU powerplay issues when using two DisplayPort connectors

[bugzilla-daemon]

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

--- Comment #7 from Timur Kristóf  ---
(In reply to Andrew Sheldon from comment #5)
> Are both monitors 60hz? I've seen this occur with 2x60hz setups, but not
> with other combinations of refresh rates. It seems to be similar to issues
> with 75hz in a single monitor configuration.


In my case, both are Dell U2718Q monitors, the resolution is 4K (3840x2160),
and the refresh rate is 60Hz on both monitors.

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