Re: [PATCH v2 17/18] drm/sun4i: tcon-top: Remove mux configuration at probe time

[Chen-Yu Tsai]

On Wed, Jul 11, 2018 at 4:35 AM, Jernej Skrabec  wrote:
> Now that R40 TCON migrated to runtime mux configuration, old code can be
> removed.
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 10/18] drm/sun4i: mixer: Read id from DT

[Chen-Yu Tsai]

On Wed, Jul 11, 2018 at 4:35 AM, Jernej Skrabec  wrote:
> Currently, TCON supports 2 ways to match TCON with engine (mixer in this
> case). Old way is to just traverse of graph backwards and compare node
> pointer. New way is to match TCON and engine by their respective ids.
> All SoCs with DE2 enabled till now used the old way, which means mixer
> id was never used and thus never implemented.
>
> However, for R40, only the new way will be used. To prepare for that,
> implement mixer id fetching from DT.
>
> Signed-off-by: Jernej Skrabec 
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 40 +++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> index aa81b9838ae8..4bd4d8ccb34f 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #include "sun4i_drv.h"
> @@ -322,6 +323,42 @@ static struct regmap_config sun8i_mixer_regmap_config = {
> .max_register   = 0xbfffc, /* guessed */
>  };
>
> +static int sun8i_mixer_of_get_id(struct device_node *node)
> +{
> +   struct device_node *port, *ep;
> +   int ret = -EINVAL;
> +
> +   /* output is port 1 */
> +   port = of_graph_get_port_by_id(node, 1);
> +   if (!port)
> +   return -EINVAL;
> +
> +   /* try to find downstream endpoint */
> +   for_each_available_child_of_node(port, ep) {
> +   struct device_node *remote;
> +   u32 reg;
> +
> +   remote = of_graph_get_remote_endpoint(ep);
> +   if (!remote)
> +   continue;
> +
> +   ret = of_property_read_u32(remote, "reg", );
> +   if (!ret) {
> +   of_node_put(remote);
> +   of_node_put(ep);
> +   of_node_put(port);
> +
> +   return reg;
> +   }
> +
> +   of_node_put(remote);
> +   }
> +
> +   of_node_put(port);
> +
> +   return ret;
> +}
> +

The above looks good.

>  static int sun8i_mixer_bind(struct device *dev, struct device *master,
>   void *data)
>  {
> @@ -353,8 +390,7 @@ static int sun8i_mixer_bind(struct device *dev, struct 
> device *master,
> dev_set_drvdata(dev, mixer);
> mixer->engine.ops = _engine_ops;
> mixer->engine.node = dev->of_node;
> -   /* The ID of the mixer currently doesn't matter */
> -   mixer->engine.id = -1;
> +   mixer->engine.id = sun8i_mixer_of_get_id(dev->of_node);

Should you be handling error codes?

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

Re: [PATCH v2 09/18] drm/sun4i: mixer: Order includes alphabetically

[Chen-Yu Tsai]

On Wed, Jul 11, 2018 at 4:35 AM, Jernej Skrabec  wrote:
> Includes are not alphabetically ordered.
>
> Reorder them.
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 04/18] drm/sun4i: tcon-top: Cleanup clock handling

[Chen-Yu Tsai]

On Wed, Jul 11, 2018 at 7:20 AM, kbuild test robot  wrote:
> Hi Jernej,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on next-20180710]
> [cannot apply to sunxi/sunxi/for-next drm/drm-next robh/for-next v4.18-rc4 
> v4.18-rc3 v4.18-rc2 v4.18-rc4]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Jernej-Skrabec/Allwinner-R40-HDMI-refactoring/20180711-043932
> config: arm64-allyesconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.2.0 make.cross ARCH=arm64
>
> All warnings (new ones prefixed by >>):
>
>In file included from include/linux/kernel.h:10:0,
> from include/linux/list.h:9,
> from include/linux/agp_backend.h:33,
> from include/drm/drmP.h:35,
> from drivers/gpu//drm/sun4i/sun8i_tcon_top.c:4:
>drivers/gpu//drm/sun4i/sun8i_tcon_top.c: In function 
> 'sun8i_tcon_top_register_gate':
>include/linux/err.h:22:49: warning: cast to pointer from integer of 
> different size [-Wint-to-pointer-cast]
> #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned 
> long)-MAX_ERRNO)
> ^
>include/linux/compiler.h:77:42: note: in definition of macro 'unlikely'
> # define unlikely(x) __builtin_expect(!!(x), 0)
>  ^
>>> drivers/gpu//drm/sun4i/sun8i_tcon_top.c:56:6: note: in expansion of macro 
>>> 'IS_ERR_VALUE'
>  if (IS_ERR_VALUE(index))
>  ^~~~
>
> vim +/IS_ERR_VALUE +56 drivers/gpu//drm/sun4i/sun8i_tcon_top.c
>
>  3
>> 4  #include 
>  5
>  6  #include 
>  7
>  8  #include 
>  9  #include 
> 10  #include 
> 11  #include 
> 12  #include 
> 13  #include 
> 14
> 15  #include "sun8i_tcon_top.h"
> 16
> 17  static int sun8i_tcon_top_get_connected_ep_id(struct device_node 
> *node,
> 18int port_id)
> 19  {
> 20  struct device_node *ep, *remote, *port;
> 21  struct of_endpoint endpoint;
> 22
> 23  port = of_graph_get_port_by_id(node, port_id);
> 24  if (!port)
> 25  return -ENOENT;
> 26
> 27  for_each_available_child_of_node(port, ep) {
> 28  remote = of_graph_get_remote_port_parent(ep);
> 29  if (!remote)
> 30  continue;
> 31
> 32  if (of_device_is_available(remote)) {
> 33  of_graph_parse_endpoint(ep, );
> 34
> 35  of_node_put(remote);
> 36
> 37  return endpoint.id;
> 38  }
> 39
> 40  of_node_put(remote);
> 41  }
> 42
> 43  return -ENOENT;
> 44  }
> 45
> 46  static struct clk_hw *sun8i_tcon_top_register_gate(struct device *dev,
> 47 const char *parent,
> 48 void __iomem *regs,
> 49 spinlock_t *lock,
> 50 u8 bit, int 
> name_index)
> 51  {
> 52  const char *clk_name, *parent_name;
> 53  int ret, index;
> 54
> 55  index = of_property_match_string(dev->of_node, "clock-names", 
> parent);
>   > 56  if (IS_ERR_VALUE(index))

Yeah. This is incorrect usage. IS_ERR_VALUE should be used on pointers.
Checking for a negative value here should suffice.

ChenYu

> 57  return ERR_PTR(index);
> 58
> 59  parent_name = of_clk_get_parent_name(dev->of_node, index);
> 60
> 61  ret = of_property_read_string_index(dev->of_node,
> 62  "clock-output-names", 
> name_index,
> 63  _name);
> 64  if (ret)
> 65  return ERR_PTR(ret);
> 66
> 67  return clk_hw_register_gat

Re: [PATCH v2 08/18] ARM: dts: sun8i: r40: Add mixer ids to TCON TOP

[Chen-Yu Tsai]

On Wed, Jul 11, 2018 at 4:35 AM, Jernej Skrabec  wrote:
> sun4i-drm DT binding, second paragraph of the first section says:
>
> For all connections between components up to the TCONs in the display
> pipeline, when there are multiple components of the same type at the
> same depth, the local endpoint ID must be the same as the remote
> component's index.
>
> Add mixer ids in R40 DT as mandated by DT binding.
>
> Fixes: 05a43a262d03 ("ARM: dts: sun8i: r40: Add HDMI pipeline")
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH V3] drm/vkms: Add vblank events simulated by hrtimers

[Rodrigo Siqueira]

This commit adds regular vblank events simulated through hrtimers, which
is a feature required by VKMS to mimic real hardware. Additionally, all
the vblank event send after pageflip is kept in the atomic_flush
function.

Signed-off-by: Rodrigo Siqueira 
---
Changes since v1:
- Removes hardcoded vblank interval to get it from user space
- Compute the vblank timer interval per interruption

Changes since v2:
- Removes unnecessary algorithm to compute the next period
- Uses drm_calc_timestamping_constants to get the vblank interval
  instead of calculating it manually
- Adds disable_vblank helper that turns of crtc
- Simplifies implementation by using drm_crtc_arm_vblank_event
- Replaces the code in atomic_begin to atomic_flush
- Removes unnecessary field in vkms_output

 drivers/gpu/drm/vkms/vkms_crtc.c | 88 +++-
 drivers/gpu/drm/vkms/vkms_drv.c  |  9 
 drivers/gpu/drm/vkms/vkms_drv.h  | 15 ++
 3 files changed, 110 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 84cc05506b09..bc9beccb977e 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -10,6 +10,60 @@
 #include 
 #include 
 
+static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
+{
+   struct vkms_output *output = container_of(timer, struct vkms_output,
+ vblank_hrtimer);
+   struct drm_crtc *crtc = >crtc;
+   int ret_overrun;
+   bool ret;
+
+   ret = drm_crtc_handle_vblank(crtc);
+   if (!ret)
+   DRM_ERROR("vkms failure on handling vblank");
+
+   ret_overrun = hrtimer_forward_now(>vblank_hrtimer,
+ output->period_ns);
+
+   return HRTIMER_RESTART;
+}
+
+static int vkms_enable_vblank(struct drm_crtc *crtc)
+{
+   struct drm_device *dev = crtc->dev;
+   unsigned int pipe = drm_crtc_index(crtc);
+   struct drm_vblank_crtc *vblank = >vblank[pipe];
+   struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+
+   drm_calc_timestamping_constants(crtc, >mode);
+
+   hrtimer_init(>vblank_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+   out->vblank_hrtimer.function = _vblank_simulate;
+   out->period_ns = ktime_set(0, vblank->framedur_ns);
+   hrtimer_start(>vblank_hrtimer, out->period_ns, HRTIMER_MODE_REL);
+
+   return 0;
+}
+
+static void vkms_disable_vblank(struct drm_crtc *crtc)
+{
+   struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
+
+   hrtimer_cancel(>vblank_hrtimer);
+}
+
+bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
+  int *max_error, ktime_t *vblank_time,
+  bool in_vblank_irq)
+{
+   struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
+   struct vkms_output *output = >output;
+
+   *vblank_time = output->vblank_hrtimer.node.expires;
+
+   return true;
+}
+
 static const struct drm_crtc_funcs vkms_crtc_funcs = {
.set_config = drm_atomic_helper_set_config,
.destroy= drm_crtc_cleanup,
@@ -17,6 +71,8 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
.reset  = drm_atomic_helper_crtc_reset,
.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
+   .enable_vblank  = vkms_enable_vblank,
+   .disable_vblank = vkms_disable_vblank,
 };
 
 static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
@@ -28,11 +84,39 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
 static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_crtc_state *old_state)
 {
+   drm_crtc_vblank_on(crtc);
+}
+
+static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
+struct drm_crtc_state *old_state)
+{
+   drm_crtc_vblank_off(crtc);
+}
+
+static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
+  struct drm_crtc_state *old_crtc_state)
+{
+   unsigned long flags;
+
+   if (crtc->state->event) {
+   spin_lock_irqsave(>dev->event_lock, flags);
+
+   if (drm_crtc_vblank_get(crtc) != 0)
+   drm_crtc_send_vblank_event(crtc, crtc->state->event);
+   else
+   drm_crtc_arm_vblank_event(crtc, crtc->state->event);
+
+   spin_unlock_irqrestore(>dev->event_lock, flags);
+
+   crtc->state->event = NULL;
+   }
 }
 
 static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
-   .atomic_check  = vkms_crtc_atomic_check,
-   .atomic_enable = vkms_crtc_atomic_enable,
+   .atomic_check   = vkms_crtc_atomic_check,
+   .atomic_flush   = vkms_crtc_atomic_flush,
+   .atomic_enable  = 

[PULL] drm-intel-next for 4.19

[Rodrigo Vivi]

Hi Dave,

This is probably the last big pull targeting 4.19.
I will probably send another small one by the end of this week and
if possible another smaller one by next one.

Here goes drm-intel-next-2018-07-09:

Higlights here goes to many PSR fixes and improvements; to the Ice lake work 
with
power well support and begin of DSI support addition. Also there were many 
improvements
on execlists and interrupts for minimal latency on command submission; and many 
fixes
on selftests, mostly caught by our CI.

General driver:
- Clean-up on aux irq (Lucas)
- Mark expected switch fall-through for dealing with static analysis tools 
(Gustavo)

Gem:
- Different fixes for GuC (Chris, Anusha, Michal)
- Avoid self-relocation BIAS if no relocation (Chris)
- Improve debugging cases in on EINVAL return and vma allocation (Chris)
- Fixes and improvements on context destroying and freeing (Chris)
- Wait for engines to idle before retiring (Chris)
- Many improvements on execlists and interrupts for minimal latency on command 
submission (Chris)
- Many fixes in selftests, specially on cases highlighted on CI (Chris)
- Other fixes and improvements around GGTT (Chris)
- Prevent background reaping of active objects (Chris)

Display:
- Parallel modeset cleanup to fix driver reset (Chris)
- Get AUX power domain for DP main link (Imre)
- Clean-up on PSR unused func pointers (Rodrigo)
- Many PSR/PSR2 fixes and improvements (DK, Jose, Tarun)
- Add a PSR1 live status (Vathsala)
- Replace old drm_*_{un/reference} with put,get functions (Thomas)
- FBC fixes (Maarten)
- Abstract and document the usage of picking macros (Jani)
- Remove unnecessary check for unsupported modifiers for NV12. (DK)
- Interrupt fixes for display (Ville)
- Clean up on sdvo code (Ville)
- Clean up on current DSI code (Jani)
- Remove support for legacy debugfs crc interface (Maarten)
- Simplify get_encoder_power_domains (Imre)

Icelake:
- MG PLL fixes (Imre)
- Add hw workaround for alpha blending (Vandita)
- Add power well support (Imre)
- Add Interrupt Support (Anusha)
- Start to add support for DSI on Ice Lake (Madhav)

Thanks,
Rodrigo.

The following changes since commit e1cacec9d50d7299893eeab2d895189f3db625da:

  drm/i915: Update DRIVER_DATE to 20180620 (2018-06-20 14:10:48 -0700)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-2018-07-09

for you to fetch changes up to 82edc7e8b8c06151bdc653935bc13b83e2f0fcfa:

  drm/i915: Update DRIVER_DATE to 20180709 (2018-07-09 15:39:27 -0700)


Higlights here goes to many PSR fixes and improvements; to the Ice lake work 
with
power well support and begin of DSI support addition. Also there were many 
improvements
on execlists and interrupts for minimal latency on command submission; and many 
fixes
on selftests, mostly caught by our CI.

General driver:
- Clean-up on aux irq (Lucas)
- Mark expected switch fall-through for dealing with static analysis tools 
(Gustavo)

Gem:
- Different fixes for GuC (Chris, Anusha, Michal)
- Avoid self-relocation BIAS if no relocation (Chris)
- Improve debugging cases in on EINVAL return and vma allocation (Chris)
- Fixes and improvements on context destroying and freeing (Chris)
- Wait for engines to idle before retiring (Chris)
- Many improvements on execlists and interrupts for minimal latency on command 
submission (Chris)
- Many fixes in selftests, specially on cases highlighted on CI (Chris)
- Other fixes and improvements around GGTT (Chris)
- Prevent background reaping of active objects (Chris)

Display:
- Parallel modeset cleanup to fix driver reset (Chris)
- Get AUX power domain for DP main link (Imre)
- Clean-up on PSR unused func pointers (Rodrigo)
- Many PSR/PSR2 fixes and improvements (DK, Jose, Tarun)
- Add a PSR1 live status (Vathsala)
- Replace old drm_*_{un/reference} with put,get functions (Thomas)
- FBC fixes (Maarten)
- Abstract and document the usage of picking macros (Jani)
- Remove unnecessary check for unsupported modifiers for NV12. (DK)
- Interrupt fixes for display (Ville)
- Clean up on sdvo code (Ville)
- Clean up on current DSI code (Jani)
- Remove support for legacy debugfs crc interface (Maarten)
- Simplify get_encoder_power_domains (Imre)

Icelake:
- MG PLL fixes (Imre)
- Add hw workaround for alpha blending (Vandita)
- Add power well support (Imre)
- Add Interrupt Support (Anusha)
- Start to add support for DSI on Ice Lake (Madhav)


Anusha Srivatsa (2):
  drm/i915/guc: Remove USES_GUC_SUBMISSION for ads programming
  drm/i915/icp: Add Interrupt Support

Chris Wilson (65):
  drm/i915: Disable bh around call to tasklet
  drm/i915: Ignore applying the self-relocation BIAS if no relocations
  drm/i915: Redefine EINVAL for debugging
  drm/i915: Defer modeset cleanup to a secondary task
  drm/i915/execlists: Check for ce->state before destroy

Re: [PATCH v2 04/18] drm/sun4i: tcon-top: Cleanup clock handling

[kbuild test robot]

Hi Jernej,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20180710]
[cannot apply to sunxi/sunxi/for-next drm/drm-next robh/for-next v4.18-rc4 
v4.18-rc3 v4.18-rc2 v4.18-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jernej-Skrabec/Allwinner-R40-HDMI-refactoring/20180711-043932
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/agp_backend.h:33,
from include/drm/drmP.h:35,
from drivers/gpu//drm/sun4i/sun8i_tcon_top.c:4:
   drivers/gpu//drm/sun4i/sun8i_tcon_top.c: In function 
'sun8i_tcon_top_register_gate':
   include/linux/err.h:22:49: warning: cast to pointer from integer of 
different size [-Wint-to-pointer-cast]
#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned 
long)-MAX_ERRNO)
^
   include/linux/compiler.h:77:42: note: in definition of macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
 ^
>> drivers/gpu//drm/sun4i/sun8i_tcon_top.c:56:6: note: in expansion of macro 
>> 'IS_ERR_VALUE'
 if (IS_ERR_VALUE(index))
 ^~~~

vim +/IS_ERR_VALUE +56 drivers/gpu//drm/sun4i/sun8i_tcon_top.c

 3  
   > 4  #include 
 5  
 6  #include 
 7  
 8  #include 
 9  #include 
10  #include 
11  #include 
12  #include 
13  #include 
14  
15  #include "sun8i_tcon_top.h"
16  
17  static int sun8i_tcon_top_get_connected_ep_id(struct device_node *node,
18int port_id)
19  {
20  struct device_node *ep, *remote, *port;
21  struct of_endpoint endpoint;
22  
23  port = of_graph_get_port_by_id(node, port_id);
24  if (!port)
25  return -ENOENT;
26  
27  for_each_available_child_of_node(port, ep) {
28  remote = of_graph_get_remote_port_parent(ep);
29  if (!remote)
30  continue;
31  
32  if (of_device_is_available(remote)) {
33  of_graph_parse_endpoint(ep, );
34  
35  of_node_put(remote);
36  
37  return endpoint.id;
38  }
39  
40  of_node_put(remote);
41  }
42  
43  return -ENOENT;
44  }
45  
46  static struct clk_hw *sun8i_tcon_top_register_gate(struct device *dev,
47 const char *parent,
48 void __iomem *regs,
49 spinlock_t *lock,
50 u8 bit, int 
name_index)
51  {
52  const char *clk_name, *parent_name;
53  int ret, index;
54  
55  index = of_property_match_string(dev->of_node, "clock-names", 
parent);
  > 56  if (IS_ERR_VALUE(index))
57  return ERR_PTR(index);
58  
59  parent_name = of_clk_get_parent_name(dev->of_node, index);
60  
61  ret = of_property_read_string_index(dev->of_node,
62  "clock-output-names", 
name_index,
63  _name);
64  if (ret)
65  return ERR_PTR(ret);
66  
67  return clk_hw_register_gate(dev, clk_name, parent_name,
68  CLK_SET_RATE_PARENT,
69  regs + TCON_TOP_GATE_SRC_REG,
70  bit, 0, lock);
71  };
72  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[radeon-alex:amd-staging-drm-next 509/521] htmldocs: drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2956: warning: Function parameter or member 'adev' not described in 'amdgpu_vm_get_task_info'

[kbuild test robot]

tree:   git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next
head:   617b0eb160a87ef4a61d6770f360c5e27e01c5ee
commit: c6d707136b4de409c7e74825e1f39e17e4b0797c [509/521] drm/amdgpu: Add 
support for logging process info in amdgpu_vm.
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick 
(https://www.imagemagick.org)
   mm/mempool.c:228: warning: Function parameter or member 'pool' not described 
in 'mempool_init'
   include/net/cfg80211.h:4279: warning: Function parameter or member 
'wext.ibss' not described in 'wireless_dev'
   include/net/cfg80211.h:4279: warning: Function parameter or member 
'wext.connect' not described in 'wireless_dev'
   include/net/cfg80211.h:4279: warning: Function parameter or member 
'wext.keys' not described in 'wireless_dev'
   include/net/cfg80211.h:4279: warning: Function parameter or member 'wext.ie' 
not described in 'wireless_dev'
   include/net/cfg80211.h:4279: warning: Function parameter or member 
'wext.ie_len' not described in 'wireless_dev'
   include/net/cfg80211.h:4279: warning: Function parameter or member 
'wext.bssid' not described in 'wireless_dev'
   include/net/cfg80211.h:4279: warning: Function parameter or member 
'wext.ssid' not described in 'wireless_dev'
   include/net/cfg80211.h:4279: warning: Function parameter or member 
'wext.default_key' not described in 'wireless_dev'
   include/net/cfg80211.h:4279: warning: Function parameter or member 
'wext.default_mgmt_key' not described in 'wireless_dev'
   include/net/cfg80211.h:4279: warning: Function parameter or member 
'wext.prev_bssid_valid' not described in 'wireless_dev'
   include/net/mac80211.h:2282: warning: Function parameter or member 
'radiotap_timestamp.units_pos' not described in 'ieee80211_hw'
   include/net/mac80211.h:2282: warning: Function parameter or member 
'radiotap_timestamp.accuracy' not described in 'ieee80211_hw'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.use_rts' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.use_cts_prot' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.short_preamble' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.skip_table' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.jiffies' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.vif' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.hw_key' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.flags' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'control.enqueue_time' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 'ack' not 
described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'ack.cookie' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'status.rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'status.ack_signal' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'status.ampdu_ack_len' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'status.ampdu_len' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'status.antenna' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'status.tx_time' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'status.is_valid_ack_signal' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'status.status_driver_data' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'driver_rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 'pad' not 
described in 'ieee80211_tx_info'
   include/net/mac80211.h:955: warning: Function parameter or member 
'rate_driver_data' not described in 'ieee80211_tx_info'
   

[Bug 95329] Metro 2033 Redux benchmark fails to start

[bugzilla-daemon]

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

--- Comment #5 from Alexander Tsoy  ---
Another workaround is possible with newer glibc:

GLIBC_TUNABLES=glibc.malloc.check=3 ./benchmark.sh

-- 
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 107183] Enabling Glamor takes half a second

[bugzilla-daemon]

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

Bug ID: 107183
   Summary: Enabling Glamor takes half a second
   Product: DRI
   Version: XOrg git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/Radeon
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: pmenzel+bugs.freedesk...@molgen.mpg.de

Created attachment 140548
  --> https://bugs.freedesktop.org/attachment.cgi?id=140548=edit
X.Org X Server 1.20.0 log

The X.Org X Server start up takes well over a second on an ASRock E350M1.

Debian Sid/unstable with Linux 4.18-rc4+ is used.

Looking at the log, there is half a second delay right below.

```
[78.639] 
X.Org X Server 1.20.0
X Protocol Version 11, Revision 0
[…]
[78.796] (II) Module glamoregl: vendor="X.Org Foundation"
[78.796]compiled for 1.20.0, module version = 1.0.1
[78.796]ABI class: X.Org ANSI C Emulation, version 0.4
[79.294] (II) RADEON(0): glamor X acceleration enabled on AMD PALM (DRM
2.50.0 / 4.18.0-rc4-00832-g217000c98815, LLVM 6.0.1)
[…]
```

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

[PULL] drm-intel-fixes

[Rodrigo Vivi]

Hi Dave,

Things are calm around drm-intel and we have only 1 fix
for hotplug irq on old platforms for this round.

I saw that you already send the pull request for 4.18-rc5
So I think this could wait for the next week. But up to you.

Here goes drm-intel-fixes-2018-07-10:
- Fix hotplug irq ack on i965/g4x (Ville)

Thanks,
Rodrigo.

The following changes since commit 1e4b044d22517cae7047c99038abb23243ca:

  Linux 4.18-rc4 (2018-07-08 16:34:02 -0700)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2018-07-10

for you to fetch changes up to 96a85cc517a9ee4ae5e8d7f5a36cba05023784eb:

  drm/i915: Fix hotplug irq ack on i965/g4x (2018-07-09 14:36:47 -0700)


- Fix hotplug irq ack on i965/g4x (Ville)


Ville Syrjälä (1):
  drm/i915: Fix hotplug irq ack on i965/g4x

 drivers/gpu/drm/i915/i915_irq.c | 32 ++--
 1 file changed, 30 insertions(+), 2 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/msm: avoid using 'timespec'

[Arnd Bergmann]

On Tue, Jul 10, 2018 at 10:47 PM, Sean Paul  wrote:
> On Mon, Jun 18, 2018 at 05:39:42PM +0200, Arnd Bergmann wrote:
>> The timespec structure and associated interfaces are deprecated and will
>> be removed in the future because of the y2038 overflow.
>>
>> The use of ktime_to_timespec() in timeout_to_jiffies() does not
>> suffer from that overflow, but is easy to avoid by just converting
>> the ktime_t into jiffies directly.
>>
>> Signed-off-by: Arnd Bergmann 
>> ---
>>  drivers/gpu/drm/msm/msm_drv.h | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
>> index b2da1fbf81e0..cc8977476a41 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.h
>> +++ b/drivers/gpu/drm/msm/msm_drv.h
>> @@ -353,8 +353,7 @@ static inline unsigned long timeout_to_jiffies(const 
>> ktime_t *timeout)
>>   remaining_jiffies = 0;
>>   } else {
>>   ktime_t rem = ktime_sub(*timeout, now);
>> - struct timespec ts = ktime_to_timespec(rem);
>> - remaining_jiffies = timespec_to_jiffies();
>> + remaining_jiffies = ktime_divns(rem, NSEC_PER_SEC / HZ);
>
> Do you need to wrap rem in ktime_to_ns() just to be safe?

The ktime_t interfaces are still defined to use an opaque type,
as previously it was a union that could be a seconds/nanoseconds
pair depending on the architecture. These days, ktime_t is just
a 64-bit integer, so div_u64() would work just as well as ktime_divns(),
but this is the documented way to do it.

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

[radeon-alex:amd-staging-drm-next 510/521] drivers/gpu//drm/amd/amdgpu/gmc_v8_0.c:1449:10: warning: missing braces around initializer

[kbuild test robot]

tree:   git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next
head:   617b0eb160a87ef4a61d6770f360c5e27e01c5ee
commit: 22521895da6d53a4d6bbcf80dff76cb7d0a5fd48 [510/521] drm/amdgpu: Present 
amdgpu_task_info in VM_FAULTS.
config: i386-randconfig-a1-201827 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
git checkout 22521895da6d53a4d6bbcf80dff76cb7d0a5fd48
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/gpu//drm/amd/amdgpu/gmc_v8_0.c: In function 
'gmc_v8_0_process_interrupt':
>> drivers/gpu//drm/amd/amdgpu/gmc_v8_0.c:1449:10: warning: missing braces 
>> around initializer [-Wmissing-braces]
  struct amdgpu_task_info task_info = { 0 };
 ^
   drivers/gpu//drm/amd/amdgpu/gmc_v8_0.c:1449:10: warning: (near 
initialization for 'task_info.process_name') [-Wmissing-braces]
--
   drivers/gpu//drm/amd/amdgpu/gmc_v9_0.c: In function 
'gmc_v9_0_process_interrupt':
>> drivers/gpu//drm/amd/amdgpu/gmc_v9_0.c:260:10: warning: missing braces 
>> around initializer [-Wmissing-braces]
  struct amdgpu_task_info task_info = { 0 };
 ^
   drivers/gpu//drm/amd/amdgpu/gmc_v9_0.c:260:10: warning: (near initialization 
for 'task_info.process_name') [-Wmissing-braces]

vim +1449 drivers/gpu//drm/amd/amdgpu/gmc_v8_0.c

  1422  
  1423  static int gmc_v8_0_process_interrupt(struct amdgpu_device *adev,
  1424struct amdgpu_irq_src *source,
  1425struct amdgpu_iv_entry *entry)
  1426  {
  1427  u32 addr, status, mc_client;
  1428  
  1429  if (amdgpu_sriov_vf(adev)) {
  1430  dev_err(adev->dev, "GPU fault detected: %d 0x%08x\n",
  1431  entry->src_id, entry->src_data[0]);
  1432  dev_err(adev->dev, " Can't decode VM fault info here on 
SRIOV VF\n");
  1433  return 0;
  1434  }
  1435  
  1436  addr = RREG32(mmVM_CONTEXT1_PROTECTION_FAULT_ADDR);
  1437  status = RREG32(mmVM_CONTEXT1_PROTECTION_FAULT_STATUS);
  1438  mc_client = RREG32(mmVM_CONTEXT1_PROTECTION_FAULT_MCCLIENT);
  1439  /* reset addr and status */
  1440  WREG32_P(mmVM_CONTEXT1_CNTL2, 1, ~1);
  1441  
  1442  if (!addr && !status)
  1443  return 0;
  1444  
  1445  if (amdgpu_vm_fault_stop == AMDGPU_VM_FAULT_STOP_FIRST)
  1446  gmc_v8_0_set_fault_enable_default(adev, false);
  1447  
  1448  if (printk_ratelimit()) {
> 1449  struct amdgpu_task_info task_info = { 0 };
  1450  
  1451  amdgpu_vm_get_task_info(adev, entry->pasid, _info);
  1452  
  1453  dev_err(adev->dev, "GPU fault detected: %d 0x%08x for 
process %s pid %d thread %s pid %d\n",
  1454  entry->src_id, entry->src_data[0], 
task_info.process_name,
  1455  task_info.tgid, task_info.task_name, 
task_info.pid);
  1456  dev_err(adev->dev, "  VM_CONTEXT1_PROTECTION_FAULT_ADDR 
  0x%08X\n",
  1457  addr);
  1458  dev_err(adev->dev, "  
VM_CONTEXT1_PROTECTION_FAULT_STATUS 0x%08X\n",
  1459  status);
  1460  gmc_v8_0_vm_decode_fault(adev, status, addr, mc_client,
  1461   entry->pasid);
  1462  }
  1463  
  1464  return 0;
  1465  }
  1466  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/msm: avoid using 'timespec'

[Sean Paul]

On Mon, Jun 18, 2018 at 05:39:42PM +0200, Arnd Bergmann wrote:
> The timespec structure and associated interfaces are deprecated and will
> be removed in the future because of the y2038 overflow.
> 
> The use of ktime_to_timespec() in timeout_to_jiffies() does not
> suffer from that overflow, but is easy to avoid by just converting
> the ktime_t into jiffies directly.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/gpu/drm/msm/msm_drv.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index b2da1fbf81e0..cc8977476a41 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -353,8 +353,7 @@ static inline unsigned long timeout_to_jiffies(const 
> ktime_t *timeout)
>   remaining_jiffies = 0;
>   } else {
>   ktime_t rem = ktime_sub(*timeout, now);
> - struct timespec ts = ktime_to_timespec(rem);
> - remaining_jiffies = timespec_to_jiffies();
> + remaining_jiffies = ktime_divns(rem, NSEC_PER_SEC / HZ);

Do you need to wrap rem in ktime_to_ns() just to be safe?

Sean

>   }
>  
>   return remaining_jiffies;
> -- 
> 2.9.0
> 

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

Re: [PATCH] drm/amd/pp: fix semicolon.cocci warnings

[Alex Deucher]

On Tue, Jul 10, 2018 at 1:11 PM, kbuild test robot
 wrote:
> From: kbuild test robot 
>
> drivers/gpu/drm/amd/amdgpu/../powerplay/amd_powerplay.c:1209:17-18: Unneeded 
> semicolon
>
>
>  Remove unneeded semicolon.
>
> Generated by: scripts/coccinelle/misc/semicolon.cocci
>
> Fixes: ea870e44415a ("drm/amd/pp: Export notify_smu_enable_pwe to display")
> CC: Rex Zhu 
> Signed-off-by: kbuild test robot 

Applied.  thanks!

Alex

> ---
>
>  amd_powerplay.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/drivers/gpu/drm/amd/amdgpu/../powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/amdgpu/../powerplay/amd_powerplay.c
> @@ -1206,7 +1206,7 @@ static int pp_notify_smu_enable_pwe(void
> struct pp_hwmgr *hwmgr = handle;
>
> if (!hwmgr || !hwmgr->pm_en)
> -   return -EINVAL;;
> +   return -EINVAL;
>
> if (hwmgr->hwmgr_func->smus_notify_pwe == NULL) {
> pr_info("%s was not implemented.\n", __func__);
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] vulkan: Define new VK_MESA_query_timestamp extension [v2]

[Keith Packard]

Pekka Paalanen  writes:

> On Sat, 23 Jun 2018 12:13:53 -0500
> Jason Ekstrand  wrote:
>
>> I haven't thought through this comment all that hard but would it make 
>> sense to have three timestamps, CPU, GPU, CPU so that you have error bars 
>> on the GPU timestamp?  At the very least, two timestamps would be better 
>> than one so that, when we pull it into the kernel, it can provide something 
>> more accurate than userspace trying to grab a snapshot.
>
> Hi,
>
> three timestamps sounds like a good idea to me, but you might want to
> reach out to media developers (e.g. gstreamer) who have experience in
> synchronizing different clocks and what that will actually take.

Oh, I know that's really hard, and I don't want to solve that problem
here. I explicitly *don't* solve it though -- I simply expose the
ability to get correlated values in the two application-visible time
domains that the Vulkan API already exposes (surface time and GPU
time). How to synchronize the application as those two clocks drift
around is outside the domain of this extension.

> When reading the CPU timestamp, I believe it would be necessary for
> userspace to tell which clock it should be reading. I'm not sure all
> userspace is always using CLOCK_MONOTONIC. But if you have a better
> rationale, that would be interesting to record and document.

The extension doesn't state which clock is returned, you provide a
device and a surface and get timestamps for both of them. This allows
applications to translate between the two Vulkan time domains.

As far as the implementation goes, it asks the device driver to get
correlated GPU and CLOCK_MONOTONIC values and then asks the WSI layer to
translate between CLOCK_MONOTONIC and surface time. Right now, all three
surface types already operate in CLOCK_MONOTONIC natively, so there
isn't any translation needed to surface time.

The anv and radv driver implementations are simplistic and could easily
be improved; they both simply fetch the GPU clock and then
CLOCK_MONOTONIC sequentially without attempting to bound the error
between them.

The entire reason for this extension is to allow me to implement the
GOOGLE_display_timing extension using only public interfaces into the
drivers. That patch is blocked on getting this functionality landed.

-- 
-keith


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

[Bug 95329] Metro 2033 Redux benchmark fails to start

[bugzilla-daemon]

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

--- Comment #4 from Jan Ziak <0xe2.0x9a.0...@gmail.com> ---
Still reproducible in July 2018.

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

Re: [PATCH] drm/tinydrm: Fix doc build warnings

[David Lechner]

On 07/10/2018 12:09 PM, Noralf Trønnes wrote:


Den 10.07.2018 18.40, skrev David Lechner:

On 07/10/2018 11:31 AM, Noralf Trønnes wrote:


Den 10.07.2018 18.18, skrev David Lechner:

On 07/10/2018 10:05 AM, Noralf Trønnes wrote:

include/drm/tinydrm/tinydrm.h:34: warning: Function parameter or member 
'fb_dirty' not described in 'tinydrm_device'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'crtc_state' not described in 'mipi_dbi_enable_flush'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'plane_state' not described in 'mipi_dbi_enable_flush'

Move struct member docs inline so it's not missed next time.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c |  2 ++
  include/drm/tinydrm/tinydrm.h  | 23 +++
  2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 4d1fb31a781f..cb3441e51d5f 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -260,6 +260,8 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs 
= {
  /**
   * mipi_dbi_enable_flush - MIPI DBI enable helper
   * @mipi: MIPI DBI structure
+ * @crtc_state: CRTC state
+ * @plane_state: Plane state
   *
   * This function sets _dbi->enabled, flushes the whole framebuffer and
   * enables the backlight. Drivers can use this in their
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 56e4a916b5e8..fe9827d0ca8a 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -16,16 +16,31 @@
    /**
   * struct tinydrm_device - tinydrm device
- * @drm: DRM device
- * @pipe: Display pipe structure
- * @dirty_lock: Serializes framebuffer flushing
- * @fb_funcs: Framebuffer functions used when creating framebuffers
   */
  struct tinydrm_device {
+    /**
+ * @drm: DRM device
+ */
  struct drm_device *drm;
+
+    /**
+ * @pipe: Display pipe structure
+ */
  struct drm_simple_display_pipe pipe;
+
+    /**
+ * @dirty_lock: Serializes framebuffer flushing
+ */
  struct mutex dirty_lock;
+
+    /**
+ * @fb_funcs: Framebuffer functions used when creating framebuffers
+ */
  const struct drm_framebuffer_funcs *fb_funcs;
+
+    /**
+ * @fb_dirty: Framebuffer dirty callback
+ */
  int (*fb_dirty)(struct drm_framebuffer *framebuffer,
  struct drm_file *file_priv, unsigned flags,
  unsigned color, struct drm_clip_rect *clips,



I assume the kerneldoc parser know how to handle this?



This? What are you referring to?

This is how I build the documentation: make DOCBOOKS="" htmldocs

Noralf.



I haven't seen struct fields with doc comments on each individual
field before. What I meant to say is that I'm assuming that when
you build the htmldocs that it picks up this documentation for each
field the same as if they were documented in the doc comment for
the struct itself. In other words, the resulting documentation
looks the same either way?



Yes it does.
https://www.kernel.org/doc/html/v4.17/doc-guide/kernel-doc.html#in-line-member-documentation-comments



Nifty.

Reviewed-by: David Lechner 

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

[PATCH] drm/amd/pp: fix semicolon.cocci warnings

[kbuild test robot]

From: kbuild test robot 

drivers/gpu/drm/amd/amdgpu/../powerplay/amd_powerplay.c:1209:17-18: Unneeded 
semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Fixes: ea870e44415a ("drm/amd/pp: Export notify_smu_enable_pwe to display")
CC: Rex Zhu 
Signed-off-by: kbuild test robot 
---

 amd_powerplay.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/gpu/drm/amd/amdgpu/../powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/amdgpu/../powerplay/amd_powerplay.c
@@ -1206,7 +1206,7 @@ static int pp_notify_smu_enable_pwe(void
struct pp_hwmgr *hwmgr = handle;
 
if (!hwmgr || !hwmgr->pm_en)
-   return -EINVAL;;
+   return -EINVAL;
 
if (hwmgr->hwmgr_func->smus_notify_pwe == NULL) {
pr_info("%s was not implemented.\n", __func__);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tinydrm: Fix doc build warnings

[Noralf Trønnes]

Den 10.07.2018 18.40, skrev David Lechner:

On 07/10/2018 11:31 AM, Noralf Trønnes wrote:


Den 10.07.2018 18.18, skrev David Lechner:

On 07/10/2018 10:05 AM, Noralf Trønnes wrote:
include/drm/tinydrm/tinydrm.h:34: warning: Function parameter or 
member 'fb_dirty' not described in 'tinydrm_device'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter 
or member 'crtc_state' not described in 'mipi_dbi_enable_flush'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter 
or member 'plane_state' not described in 'mipi_dbi_enable_flush'


Move struct member docs inline so it's not missed next time.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c |  2 ++
  include/drm/tinydrm/tinydrm.h  | 23 +++
  2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c

index 4d1fb31a781f..cb3441e51d5f 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -260,6 +260,8 @@ static const struct drm_framebuffer_funcs 
mipi_dbi_fb_funcs = {

  /**
   * mipi_dbi_enable_flush - MIPI DBI enable helper
   * @mipi: MIPI DBI structure
+ * @crtc_state: CRTC state
+ * @plane_state: Plane state
   *
   * This function sets _dbi->enabled, flushes the whole 
framebuffer and

   * enables the backlight. Drivers can use this in their
diff --git a/include/drm/tinydrm/tinydrm.h 
b/include/drm/tinydrm/tinydrm.h

index 56e4a916b5e8..fe9827d0ca8a 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -16,16 +16,31 @@
    /**
   * struct tinydrm_device - tinydrm device
- * @drm: DRM device
- * @pipe: Display pipe structure
- * @dirty_lock: Serializes framebuffer flushing
- * @fb_funcs: Framebuffer functions used when creating framebuffers
   */
  struct tinydrm_device {
+    /**
+ * @drm: DRM device
+ */
  struct drm_device *drm;
+
+    /**
+ * @pipe: Display pipe structure
+ */
  struct drm_simple_display_pipe pipe;
+
+    /**
+ * @dirty_lock: Serializes framebuffer flushing
+ */
  struct mutex dirty_lock;
+
+    /**
+ * @fb_funcs: Framebuffer functions used when creating 
framebuffers

+ */
  const struct drm_framebuffer_funcs *fb_funcs;
+
+    /**
+ * @fb_dirty: Framebuffer dirty callback
+ */
  int (*fb_dirty)(struct drm_framebuffer *framebuffer,
  struct drm_file *file_priv, unsigned flags,
  unsigned color, struct drm_clip_rect *clips,



I assume the kerneldoc parser know how to handle this?



This? What are you referring to?

This is how I build the documentation: make DOCBOOKS="" htmldocs

Noralf.



I haven't seen struct fields with doc comments on each individual
field before. What I meant to say is that I'm assuming that when
you build the htmldocs that it picks up this documentation for each
field the same as if they were documented in the doc comment for
the struct itself. In other words, the resulting documentation
looks the same either way?



Yes it does.
https://www.kernel.org/doc/html/v4.17/doc-guide/kernel-doc.html#in-line-member-documentation-comments

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

Re: [PATCH] drm/tinydrm: Fix doc build warnings

[David Lechner]

On 07/10/2018 11:31 AM, Noralf Trønnes wrote:


Den 10.07.2018 18.18, skrev David Lechner:

On 07/10/2018 10:05 AM, Noralf Trønnes wrote:

include/drm/tinydrm/tinydrm.h:34: warning: Function parameter or member 
'fb_dirty' not described in 'tinydrm_device'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'crtc_state' not described in 'mipi_dbi_enable_flush'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'plane_state' not described in 'mipi_dbi_enable_flush'

Move struct member docs inline so it's not missed next time.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c |  2 ++
  include/drm/tinydrm/tinydrm.h  | 23 +++
  2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 4d1fb31a781f..cb3441e51d5f 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -260,6 +260,8 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs 
= {
  /**
   * mipi_dbi_enable_flush - MIPI DBI enable helper
   * @mipi: MIPI DBI structure
+ * @crtc_state: CRTC state
+ * @plane_state: Plane state
   *
   * This function sets _dbi->enabled, flushes the whole framebuffer and
   * enables the backlight. Drivers can use this in their
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 56e4a916b5e8..fe9827d0ca8a 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -16,16 +16,31 @@
    /**
   * struct tinydrm_device - tinydrm device
- * @drm: DRM device
- * @pipe: Display pipe structure
- * @dirty_lock: Serializes framebuffer flushing
- * @fb_funcs: Framebuffer functions used when creating framebuffers
   */
  struct tinydrm_device {
+    /**
+ * @drm: DRM device
+ */
  struct drm_device *drm;
+
+    /**
+ * @pipe: Display pipe structure
+ */
  struct drm_simple_display_pipe pipe;
+
+    /**
+ * @dirty_lock: Serializes framebuffer flushing
+ */
  struct mutex dirty_lock;
+
+    /**
+ * @fb_funcs: Framebuffer functions used when creating framebuffers
+ */
  const struct drm_framebuffer_funcs *fb_funcs;
+
+    /**
+ * @fb_dirty: Framebuffer dirty callback
+ */
  int (*fb_dirty)(struct drm_framebuffer *framebuffer,
  struct drm_file *file_priv, unsigned flags,
  unsigned color, struct drm_clip_rect *clips,



I assume the kerneldoc parser know how to handle this?



This? What are you referring to?

This is how I build the documentation: make DOCBOOKS="" htmldocs

Noralf.



I haven't seen struct fields with doc comments on each individual
field before. What I meant to say is that I'm assuming that when
you build the htmldocs that it picks up this documentation for each
field the same as if they were documented in the doc comment for
the struct itself. In other words, the resulting documentation
looks the same either way?

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

Re: [PATCH] drm/tinydrm: Fix doc build warnings

[Noralf Trønnes]

Den 10.07.2018 18.18, skrev David Lechner:

On 07/10/2018 10:05 AM, Noralf Trønnes wrote:
include/drm/tinydrm/tinydrm.h:34: warning: Function parameter or 
member 'fb_dirty' not described in 'tinydrm_device'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter 
or member 'crtc_state' not described in 'mipi_dbi_enable_flush'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter 
or member 'plane_state' not described in 'mipi_dbi_enable_flush'


Move struct member docs inline so it's not missed next time.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c |  2 ++
  include/drm/tinydrm/tinydrm.h  | 23 +++
  2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c

index 4d1fb31a781f..cb3441e51d5f 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -260,6 +260,8 @@ static const struct drm_framebuffer_funcs 
mipi_dbi_fb_funcs = {

  /**
   * mipi_dbi_enable_flush - MIPI DBI enable helper
   * @mipi: MIPI DBI structure
+ * @crtc_state: CRTC state
+ * @plane_state: Plane state
   *
   * This function sets _dbi->enabled, flushes the whole 
framebuffer and

   * enables the backlight. Drivers can use this in their
diff --git a/include/drm/tinydrm/tinydrm.h 
b/include/drm/tinydrm/tinydrm.h

index 56e4a916b5e8..fe9827d0ca8a 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -16,16 +16,31 @@
    /**
   * struct tinydrm_device - tinydrm device
- * @drm: DRM device
- * @pipe: Display pipe structure
- * @dirty_lock: Serializes framebuffer flushing
- * @fb_funcs: Framebuffer functions used when creating framebuffers
   */
  struct tinydrm_device {
+    /**
+ * @drm: DRM device
+ */
  struct drm_device *drm;
+
+    /**
+ * @pipe: Display pipe structure
+ */
  struct drm_simple_display_pipe pipe;
+
+    /**
+ * @dirty_lock: Serializes framebuffer flushing
+ */
  struct mutex dirty_lock;
+
+    /**
+ * @fb_funcs: Framebuffer functions used when creating framebuffers
+ */
  const struct drm_framebuffer_funcs *fb_funcs;
+
+    /**
+ * @fb_dirty: Framebuffer dirty callback
+ */
  int (*fb_dirty)(struct drm_framebuffer *framebuffer,
  struct drm_file *file_priv, unsigned flags,
  unsigned color, struct drm_clip_rect *clips,



I assume the kerneldoc parser know how to handle this?



This? What are you referring to?

This is how I build the documentation: make DOCBOOKS="" htmldocs

Noralf.

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

Re: [RFC PATCH] mm, oom: distinguish blockable mode for mmu notifiers

[Leon Romanovsky]

On Tue, Jul 10, 2018 at 04:14:10PM +0200, Michal Hocko wrote:
> On Tue 10-07-18 16:40:40, Leon Romanovsky wrote:
> > On Mon, Jul 09, 2018 at 02:29:08PM +0200, Michal Hocko wrote:
> > > On Wed 27-06-18 09:44:21, Michal Hocko wrote:
> > > > This is the v2 of RFC based on the feedback I've received so far. The
> > > > code even compiles as a bonus ;) I haven't runtime tested it yet, mostly
> > > > because I have no idea how.
> > > >
> > > > Any further feedback is highly appreciated of course.
> > >
> > > Any other feedback before I post this as non-RFC?
> >
> > From mlx5 perspective, who is primary user of umem_odp.c your change looks 
> > ok.
>
> Can I assume your Acked-by?

I didn't have a chance to test it because it applies on our rdma-next, but
fails to compile.

Thanks

>
> Thanks for your review!
> --
> Michal Hocko
> SUSE Labs
>


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

Re: [PATCH] drm/tinydrm: Fix doc build warnings

[David Lechner]

On 07/10/2018 10:05 AM, Noralf Trønnes wrote:

include/drm/tinydrm/tinydrm.h:34: warning: Function parameter or member 
'fb_dirty' not described in 'tinydrm_device'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'crtc_state' not described in 'mipi_dbi_enable_flush'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'plane_state' not described in 'mipi_dbi_enable_flush'

Move struct member docs inline so it's not missed next time.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---
  drivers/gpu/drm/tinydrm/mipi-dbi.c |  2 ++
  include/drm/tinydrm/tinydrm.h  | 23 +++
  2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 4d1fb31a781f..cb3441e51d5f 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -260,6 +260,8 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs 
= {
  /**
   * mipi_dbi_enable_flush - MIPI DBI enable helper
   * @mipi: MIPI DBI structure
+ * @crtc_state: CRTC state
+ * @plane_state: Plane state
   *
   * This function sets _dbi->enabled, flushes the whole framebuffer and
   * enables the backlight. Drivers can use this in their
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 56e4a916b5e8..fe9827d0ca8a 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -16,16 +16,31 @@
  
  /**

   * struct tinydrm_device - tinydrm device
- * @drm: DRM device
- * @pipe: Display pipe structure
- * @dirty_lock: Serializes framebuffer flushing
- * @fb_funcs: Framebuffer functions used when creating framebuffers
   */
  struct tinydrm_device {
+   /**
+* @drm: DRM device
+*/
struct drm_device *drm;
+
+   /**
+* @pipe: Display pipe structure
+*/
struct drm_simple_display_pipe pipe;
+
+   /**
+* @dirty_lock: Serializes framebuffer flushing
+*/
struct mutex dirty_lock;
+
+   /**
+* @fb_funcs: Framebuffer functions used when creating framebuffers
+*/
const struct drm_framebuffer_funcs *fb_funcs;
+
+   /**
+* @fb_dirty: Framebuffer dirty callback
+*/
int (*fb_dirty)(struct drm_framebuffer *framebuffer,
struct drm_file *file_priv, unsigned flags,
unsigned color, struct drm_clip_rect *clips,



I assume the kerneldoc parser know how to handle this?

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

Re: [PATCH 15/17] ARM: dts: sun8i: r40: Disable TCONs by default.

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> R40 has 4 TCONs, but only 2 of them can receive some kind of output at
> the same time. Let's disable them by default, so only those which are
> really connected on board can be enabled in board dts file.
>
> Signed-off-by: Jernej Skrabec 

Originally I had wanted all TCONs enabled by default. But that would
require some more work for dynamic assignment of TCONs to CRTCs, which
is way out of scope for this series. So,

Reviewed-by: Chen-Yu Tsai 

I'll think about if and how this could be done.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/17] drm/sun4i: tcon: Add support for R40 TCON

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> R40 TV TCON is basically the same as on A83T. However, it needs special
> handling, because it has to set up TCON TOP muxes at runtime.
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 

The runtime muxing of mixer <-> TCON is quite clever.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/17] drm/sun4i: tcon: Add another way for matching mixers with tcon

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> Till now, new way of matching engines with TCONs was reading their
> respective ids and match them by those ids. However, with introduction
> of TCON TOP, that might not be so straightforward anymore.
> - there might be more TCONs that engines (mixers)
> - TCON ids might have non-consecutive ids
>
> Workaround that by matching mixer id with TCON index from TCON list.
>
> For example, R40 has 2 mixers and 4 TCONs. Board designer can choose
> 2 outputs, which are connected to any of those 4 TCONs. As long as there
> are only 2 TCONs enabled in DT, using index in list as alternative id,
> will allow to match them with mixer 0 and 1.
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tinydrm: add backlight dependency for ili9341

[David Lechner]

On 07/10/2018 10:55 AM, Noralf Trønnes wrote:


Anyways, thanks for fixing this:
Acked-by: Noralf Trønnes 

David Lechner do you apply this?

Noralf.



Yes, I will pick it up.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 17/17] dt-bindings: display: sun4i-drm: Fix order of DW HDMI PHY compatibles

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> They are currently sorted alphabetically. However, they should be sorted
> by release date of the family and then alphabetically.
>
> Fixes: 03c35dbf73e0 ("dt-bindings: display: sun4i-drm: Add description of A64 
> HDMI PHY")
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 16/17] drm/sun4i: tcon-top: Remove mux configuration at probe time

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> Now that R40 TCON migrated to runtime mux configuration, old code can be
> removed.
>
> Signed-off-by: Jernej Skrabec 
> ---
>  drivers/gpu/drm/sun4i/sun8i_tcon_top.c | 81 +++---
>  1 file changed, 7 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c 
> b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> index c09b15b64192..78795d6cb174 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> @@ -87,34 +87,6 @@ int sun8i_tcon_top_de_config(struct device *dev, int 
> mixer, int tcon)
>  }
>  EXPORT_SYMBOL(sun8i_tcon_top_de_config);
>
> -static int sun8i_tcon_top_get_connected_ep_id(struct device_node *node,
> - int port_id)
> -{
> -   struct device_node *ep, *remote, *port;
> -   struct of_endpoint endpoint;
> -
> -   port = of_graph_get_port_by_id(node, port_id);
> -   if (!port)
> -   return -ENOENT;
> -
> -   for_each_available_child_of_node(port, ep) {
> -   remote = of_graph_get_remote_port_parent(ep);
> -   if (!remote)
> -   continue;
> -
> -   if (of_device_is_available(remote)) {
> -   of_graph_parse_endpoint(ep, );
> -
> -   of_node_put(remote);
> -
> -   return endpoint.id;
> -   }
> -
> -   of_node_put(remote);
> -   }
> -
> -   return -ENOENT;
> -}
>
>  static struct clk_hw *sun8i_tcon_top_register_gate(struct device *dev,
>const char *parent,
> @@ -149,11 +121,9 @@ static int sun8i_tcon_top_bind(struct device *dev, 
> struct device *master,
> struct platform_device *pdev = to_platform_device(dev);
> struct clk_hw_onecell_data *clk_data;
> struct sun8i_tcon_top *tcon_top;
> -   bool mixer0_unused = false;
> struct resource *res;
> void __iomem *regs;
> -   int ret, i, id;
> -   u32 val;
> +   int ret, i;
>
> tcon_top = devm_kzalloc(dev, sizeof(*tcon_top), GFP_KERNEL);
> if (!tcon_top)
> @@ -198,49 +168,12 @@ static int sun8i_tcon_top_bind(struct device *dev, 
> struct device *master,
> goto err_assert_reset;
> }
>
> -   val = 0;
> -
> -   /* check if HDMI mux output is connected */
> -   if (sun8i_tcon_top_get_connected_ep_id(dev->of_node, 5) >= 0) {
> -   /* find HDMI input endpoint id, if it is connected at all*/
> -   id = sun8i_tcon_top_get_connected_ep_id(dev->of_node, 4);
> -   if (id >= 0)
> -   val = FIELD_PREP(TCON_TOP_HDMI_SRC_MSK, id + 1);
> -   else
> -   DRM_DEBUG_DRIVER("TCON TOP HDMI input is not 
> connected\n");
> -   } else {
> -   DRM_DEBUG_DRIVER("TCON TOP HDMI output is not connected\n");
> -   }
> -
> -   writel(val, regs + TCON_TOP_GATE_SRC_REG);
> -
> -   val = 0;
> -
> -   /* process mixer0 mux output */
> -   id = sun8i_tcon_top_get_connected_ep_id(dev->of_node, 1);
> -   if (id >= 0) {
> -   val = FIELD_PREP(TCON_TOP_PORT_DE0_MSK, id);
> -   } else {
> -   DRM_DEBUG_DRIVER("TCON TOP mixer0 output is not connected\n");
> -   mixer0_unused = true;
> -   }
> -
> -   /* process mixer1 mux output */
> -   id = sun8i_tcon_top_get_connected_ep_id(dev->of_node, 3);
> -   if (id >= 0) {
> -   val |= FIELD_PREP(TCON_TOP_PORT_DE1_MSK, id);
> -
> -   /*
> -* mixer0 mux has priority over mixer1 mux. We have to
> -* make sure mixer0 doesn't overtake TCON from mixer1.
> -*/
> -   if (mixer0_unused && id == 0)
> -   val |= FIELD_PREP(TCON_TOP_PORT_DE0_MSK, 1);
> -   } else {
> -   DRM_DEBUG_DRIVER("TCON TOP mixer1 output is not connected\n");
> -   }
> -
> -   writel(val, regs + TCON_TOP_PORT_SEL_REG);
> +   /*
> +* Default register values might have some reserved bits set, which
> +* prevents TCON TOP from working properly. Set them to 0 here.
> +*/
> +   writel(0, regs + TCON_TOP_GATE_SRC_REG);
> +   writel(0, regs + TCON_TOP_PORT_SEL_REG);

Would it make sense to just force a reset using the reset control?

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

Re: [PATCH 10/17] drm/sun4i: tcon-top: Add helpers for switching mux

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> We want to be able to set TCON TOP muxes at runtime. Add helpers for
> that.
>
> Old, static configuration of muxes at probe time is preserved for now.
> It will be removed when R40 TCON starts using them.
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tinydrm: add backlight dependency for ili9341

[Noralf Trønnes]

Den 09.07.2018 17.20, skrev Arnd Bergmann:

This tinydrm driver fails to link without the backlight support:

drivers/gpu/drm/tinydrm/ili9341.o: In function `ili9341_probe':
ili9341.c:(.text+0x578): undefined reference to `devm_of_find_backlight'

Fixes: 3fa0e8f6f960 ("drm/tinydrm: new driver for ILI9341 display panels")
Signed-off-by: Arnd Bergmann 
---
  drivers/gpu/drm/tinydrm/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index 7a8008b0783f..16f4b5c91f1b 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -23,6 +23,7 @@ config TINYDRM_ILI9225
  config TINYDRM_ILI9341
tristate "DRM support for ILI9341 display panels"
depends on DRM_TINYDRM && SPI
+   depends on BACKLIGHT_CLASS_DEVICE
select TINYDRM_MIPI_DBI
help
  DRM driver for the following Ilitek ILI9341 panels:


There was some work done to make backlight optional, but I see now that
it failed to take into account the possibility of the driver being
builtin and BACKLIGHT_CLASS_DEVICE being a module.

This is from include/linux/backlight.h:

#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
struct backlight_device *of_find_backlight(struct device *dev);
struct backlight_device *devm_of_find_backlight(struct device *dev);
#else
static inline struct backlight_device *of_find_backlight(struct device *dev)
{
    return NULL;
}

static inline struct backlight_device *
devm_of_find_backlight(struct device *dev)
{
    return NULL;
}
#endif


Anyways, thanks for fixing this:
Acked-by: Noralf Trønnes 

David Lechner do you apply this?

Noralf.

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

Re: [PATCH 14/17] ARM: dts: sun8i: r40: Add missing TCON-TOP - TCON connections

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> Current R40 is missing some graph connections between TCON TOP and
> TCONs.
>
> Add them.
>
> Fixes: 05a43a262d03 ("ARM: dts: sun8i: r40: Add HDMI pipeline")
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/17] ARM: dts: sun8i: r40: Remove fallback compatible for TCON TV

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> While registers between A83T and R40 TV TCON are the same, R40 TCON TV
> driver has to set additional muxes in TCON TOP. Because of that, remove
> fallback A83T TCON TV compatible.

Nit: device tree doesn't care about the implementation. You can however
mention that hardware interaction makes it not so compatible. :)

> Signed-off-by: Jernej Skrabec 

Otherwise,

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 20/21] drm/msm: Add SDM845 DPU support

[Sean Paul]

The patch was rejected for being too big. Please refer to
https://gitlab.freedesktop.org/seanpaul/dpu-staging/commit/b4215cf040d1978287bd1403832ffc610659652b

Sean

-- 
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 09/17] drm/sun4i: mixer: Read id from DT

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> Currently, TCON supports 2 ways to match TCON with engine (mixer in this
> case). Old way is to just traverse of graph backwards and compare node
> pointer. New way is to match TCON and engine by their respective ids.
> All SoCs with DE2 enabled till now used the old way, which means mixer
> id was never used and thus never implemented.
>
> However, for R40, only the new way will be used. To prepare for that,
> implemend fetching mixer id from DT.
>
> Signed-off-by: Jernej Skrabec 
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 35 +++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> index ee8febb25903..11221f96746d 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

These should be alphabetically ordered.

>
>  #include "sun4i_drv.h"
>  #include "sun8i_mixer.h"
> @@ -322,6 +323,37 @@ static struct regmap_config sun8i_mixer_regmap_config = {
> .max_register   = 0xbfffc, /* guessed */
>  };
>
> +static int sun8i_mixer_of_get_id(struct device_node *node)
> +{
> +   struct device_node *port, *ep;
> +   int ret = -EINVAL;
> +
> +   /* output is port 1 */
> +   port = of_graph_get_port_by_id(node, 1);
> +   if (!port)
> +   return -EINVAL;
> +
> +   /* try finding an upstream endpoint */

You mean downstream.

> +   for_each_available_child_of_node(port, ep) {
> +   struct device_node *remote;
> +   u32 reg;
> +
> +   remote = of_graph_get_remote_endpoint(ep);
> +   if (!remote)
> +   continue;
> +
> +   ret = of_property_read_u32(remote, "reg", );
> +   if (ret)
> +   continue;

This is somewhat redundant, given the current code structure will loop
over all available child nodes. What you want is probably an early
termination condition instead, i.e. if (!ret).

Also, remember to call of_node_put on the remote node.

ChenYu

> +
> +   ret = reg;
> +   }
> +
> +   of_node_put(port);
> +
> +   return ret;
> +}
> +
>  static int sun8i_mixer_bind(struct device *dev, struct device *master,
>   void *data)
>  {
> @@ -353,8 +385,7 @@ static int sun8i_mixer_bind(struct device *dev, struct 
> device *master,
> dev_set_drvdata(dev, mixer);
> mixer->engine.ops = _engine_ops;
> mixer->engine.node = dev->of_node;
> -   /* The ID of the mixer currently doesn't matter */
> -   mixer->engine.id = -1;
> +   mixer->engine.id = sun8i_mixer_of_get_id(dev->of_node);
>
> mixer->cfg = of_device_get_match_data(dev);
> if (!mixer->cfg)
> --
> 2.18.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/17] ARM: dts: sun8i: r40: Add mixer ids to TCON TOP

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> Mixer has to have a way to read its id, if needed.

You should mention that this is required by the device tree binding sun4i-drm,
in the second paragraph of the first section:

For all connections between components up to the TCONs in the display
pipeline, when there are multiple components of the same type at the
same depth, the local endpoint ID must be the same as the remote
component's index.

You might also want to update the diagram for TCON TOP in the binding doc.

ChenYu

> Add them in R40 DT.
>
> Signed-off-by: Jernej Skrabec 
> ---
>  arch/arm/boot/dts/sun8i-r40.dtsi | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi 
> b/arch/arm/boot/dts/sun8i-r40.dtsi
> index 2afb079a3776..1dd088d82773 100644
> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> @@ -576,9 +576,12 @@
> #size-cells = <0>;
>
> tcon_top_mixer0_in: port@0 {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> reg = <0>;
>
> -   tcon_top_mixer0_in_mixer0: endpoint {
> +   tcon_top_mixer0_in_mixer0: endpoint@0 
> {
> +   reg = <0>;
> remote-endpoint = 
> <_out_tcon_top>;
> };
> };
> @@ -606,9 +609,12 @@
> };
>
> tcon_top_mixer1_in: port@2 {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> reg = <2>;
>
> -   tcon_top_mixer1_in_mixer1: endpoint {
> +   tcon_top_mixer1_in_mixer1: endpoint@1 
> {
> +   reg = <1>;
> remote-endpoint = 
> <_out_tcon_top>;
> };
> };
> --
> 2.18.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/17] drm/sun4i: DW HDMI: Release nodes if error happens during CRTC search

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> If error happens in sun8i_dw_hdmi_find_possible_crtcs(), nodes are not
> released with of_node_put() before returning.
>
> Fix that by calling of_node_put() when necessary. While on it, clean up
> the code by using of_graph_get_remote_node() which also lowers number of
> cases where error handling has to be performed.

Good job!

> Fixes: 57e23de02f48 ("drm/sun4i: DW HDMI: Expand algorithm for possible 
> crtcs")
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/17] dt-bindings: display: sun4i-drm: Add R40 TV TCON description

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> TCON description is expanded with R40 TV TCON compatible. It is a bit
> special, because it is connected to TCON TOP instead directly to mixer
> and it needs special handling.
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 

> ---
>  Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt 
> b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> index 288b4cbc255e..7e2451396a28 100644
> --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> @@ -147,6 +147,7 @@ Required properties:
> * allwinner,sun8i-a33-tcon
> * allwinner,sun8i-a83t-tcon-lcd
> * allwinner,sun8i-a83t-tcon-tv
> +   * allwinner,sun8i-r40-tcon-tv
> * allwinner,sun8i-v3s-tcon
> * allwinner,sun9i-a80-tcon-lcd
> * allwinner,sun9i-a80-tcon-tv
> @@ -181,7 +182,7 @@ For TCONs with channel 0, there is one more clock 
> required:
>  For TCONs with channel 1, there is one more clock required:
> - 'tcon-ch1': The clock driving the TCON channel 1
>
> -When TCON support LVDS (all TCONs except TV TCON on A83T and those found
> +When TCON support LVDS (all TCONs except TV TCONs on A83T, R40 and those 
> found
>  in A13, H3, H5 and V3s SoCs), you need one more reset line:

I think this line is getting more convoluted as we add more exceptions.
We may want to reword it somehow.

> - 'lvds': The reset line driving the LVDS logic
>
> --
> 2.18.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/17] drm/sun4i: tcon: Release node when traversing of graph

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> Function sun4i_tcon_find_engine_traverse() doesn't release node if it
> needs to traverse of graph deeper than 1 level.
>
> Fix this by calling of_node_put().
>
> Fixes: 49836b11fe71 ("drm/sun4i: tcon: Generalize engine search algorithm")
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/17] drm/sun4i: tcon-top: Cleanup clock handling

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:51 AM, Jernej Skrabec  wrote:
> There is no need to acquire reference to clock just to get its name.
>
> This commit just cleans up the code. There is no functional change.
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/17] ARM: dts: sun8i: r40: Remove fallback display engine compatible

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:50 AM, Jernej Skrabec  wrote:
> R40 has pretty unique display pipeline. Because of that, H3 display
> engine compatible fallback should be removed.
>
> Fixes: 05a43a262d03 ("ARM: dts: sun8i: r40: Add HDMI pipeline")
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/17] drm/sun4i: Add R40 display engine compatible

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:50 AM, Jernej Skrabec  wrote:
> R40 has versatile display pipeline. It supports two simultanious outputs
> on various outputs (TVE, VGA, HDMI, MIPI DSI, LCD).
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/17] dt-bindings: display: sun4i-drm: Add R40 display engine compatible

[Chen-Yu Tsai]

On Sat, Jul 7, 2018 at 1:50 AM, Jernej Skrabec  wrote:
> R40 has pretty unique display pipeline. It supports two outputs at the
> same time.
>
> Possible outputs:
> - 1x HDMI,
> - 2x TV output
> - 1x VGA,
> - 1x MIPI DSI and
> - 2x LCD outputs
>
> That is the biggest number of possible outputs from all Allwinner SoC.
> Because of that, add new compatible for it.
>
> Signed-off-by: Jernej Skrabec 

Reviewed-by: Chen-Yu Tsai 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/tinydrm: Fix doc build warnings

[Noralf Trønnes]

include/drm/tinydrm/tinydrm.h:34: warning: Function parameter or member 
'fb_dirty' not described in 'tinydrm_device'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'crtc_state' not described in 'mipi_dbi_enable_flush'
drivers/gpu/drm/tinydrm/mipi-dbi.c:272: warning: Function parameter or member 
'plane_state' not described in 'mipi_dbi_enable_flush'

Move struct member docs inline so it's not missed next time.

Cc: David Lechner 
Signed-off-by: Noralf Trønnes 
---
 drivers/gpu/drm/tinydrm/mipi-dbi.c |  2 ++
 include/drm/tinydrm/tinydrm.h  | 23 +++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c 
b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 4d1fb31a781f..cb3441e51d5f 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -260,6 +260,8 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs 
= {
 /**
  * mipi_dbi_enable_flush - MIPI DBI enable helper
  * @mipi: MIPI DBI structure
+ * @crtc_state: CRTC state
+ * @plane_state: Plane state
  *
  * This function sets _dbi->enabled, flushes the whole framebuffer and
  * enables the backlight. Drivers can use this in their
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 56e4a916b5e8..fe9827d0ca8a 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -16,16 +16,31 @@
 
 /**
  * struct tinydrm_device - tinydrm device
- * @drm: DRM device
- * @pipe: Display pipe structure
- * @dirty_lock: Serializes framebuffer flushing
- * @fb_funcs: Framebuffer functions used when creating framebuffers
  */
 struct tinydrm_device {
+   /**
+* @drm: DRM device
+*/
struct drm_device *drm;
+
+   /**
+* @pipe: Display pipe structure
+*/
struct drm_simple_display_pipe pipe;
+
+   /**
+* @dirty_lock: Serializes framebuffer flushing
+*/
struct mutex dirty_lock;
+
+   /**
+* @fb_funcs: Framebuffer functions used when creating framebuffers
+*/
const struct drm_framebuffer_funcs *fb_funcs;
+
+   /**
+* @fb_dirty: Framebuffer dirty callback
+*/
int (*fb_dirty)(struct drm_framebuffer *framebuffer,
struct drm_file *file_priv, unsigned flags,
unsigned color, struct drm_clip_rect *clips,
-- 
2.15.1

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

Re: [PATCH v2 1/1] drm/panel: Add support for Olimex LCD-OLinuXino panel

[Thierry Reding]

On Tue, Jul 10, 2018 at 04:08:54PM +0300, Stefan Mavrodiev wrote:
> On 07/10/2018 01:32 PM, Thierry Reding wrote:
> > On Mon, Jun 25, 2018 at 09:44:35AM +0300, Stefan Mavrodiev wrote:
[...]
> > > diff --git a/drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c 
> > > b/drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c
[...]
> > > +static int lcd_olinuxino_get_modes(struct drm_panel *panel)
> > > +{
> > > + struct lcd_olinuxino *lcd = to_lcd_olinuxino(panel);
> > > + struct drm_connector *connector = lcd->panel.connector;
> > > + struct lcd_olinuxino_info *lcd_info = >eeprom.info;
> > > + struct drm_device *drm = lcd->panel.drm;
> > > + struct lcd_olinuxino_mode *lcd_mode;
> > > + struct drm_display_mode *mode;
> > > + int i, num = 0;
> > These two can be unsigned.
> > 
> > > +
> > > + /* Read up to 4 modes */
> > > + for (i = 0; i < lcd->eeprom.num_modes && i < 4; i++) {
> > Can it happen that lcd->eeprom.num_modes >= 4? Seems to me like that
> > would be a serious bug. Perhaps move that check to where the EEPROM is
> > read and output a warning, then overwrite lcd->eeprom.num_modes with 4?
> If num_modes is bigger than 4, then lcd_mode pointer will point to invalid
> location. This can happen if someone overwrite the eeprom and apply
> correct checksum at the end. Then i < 4 makes sure this won't happen.

I still think that this should be checked earlier in the code, like at
->probe() time. That way you can output a warning once so that people
have a chance to notice a broken EEPROM and potentially do something
about it. You can then also sanitize the EEPROM contents so that the
rest of the code (i.e. ->get_modes()) doesn't have to check for this
error case.

> > 
> > > + lcd_mode = (struct lcd_olinuxino_mode *)
> > > +>eeprom.reserved[i * sizeof(*lcd_mode)];
> > > +
> > > + mode = drm_mode_create(drm);
> > > + if (!mode) {
> > > + dev_err(drm->dev, "failed to add mode %ux%u@%u\n",
> > > + lcd_mode->hactive,
> > > + lcd_mode->vactive,
> > > + lcd_mode->refresh);
> > > + continue;
> > > + }
> > > +
> > > + mode->clock = lcd_mode->pixelclock;
> > > + mode->hdisplay = lcd_mode->hactive;
> > > + mode->hsync_start = lcd_mode->hactive + lcd_mode->hfp;
> > > + mode->hsync_end = lcd_mode->hactive + lcd_mode->hfp +
> > > +   lcd_mode->hpw;
> > > + mode->htotal = lcd_mode->hactive + lcd_mode->hfp +
> > > +lcd_mode->hpw + lcd_mode->hbp;
> > > + mode->vdisplay = lcd_mode->vactive;
> > > + mode->vsync_start = lcd_mode->vactive + lcd_mode->vfp;
> > > + mode->vsync_end = lcd_mode->vactive + lcd_mode->vfp +
> > > +   lcd_mode->vpw;
> > > + mode->vtotal = lcd_mode->vactive + lcd_mode->vfp +
> > > +lcd_mode->vpw + lcd_mode->vbp;
> > > + mode->vrefresh = lcd_mode->refresh;
> > > +
> > > + if (lcd->eeprom.num_modes == 1)
> > > + mode->type |= DRM_MODE_TYPE_PREFERRED;
> > Is there no way to determine the preferred mode if there are more than
> > one? Perhaps always prefer the first mode?
> My idea here is what if the first mode is not supported by the host? That's
> why
> we will be storing multiple modes, one with the most strict timings
> (e.g rounded to 10kHz or less, according to the lcd datesheet), and others
> with less strict.

Its not the panel driver's responsibility to take into account the host
capabilities. The panel driver should, to the best of its abilities,
report the supported modes and the host driver is responsible for
dealing with the mode list. If any of the modes are not within its
capabilities, then it can filter them out (using the ->mode_valid()
callback).

In this particular case, if there is no clearly defined preferred mode
I'd argue that either you don't mark any mode as preferred, or you
choose the one with the strictest timings. I had hoped that perhaps the
first mode would always be the one with the strictest timings and hence
would be the preferred mode, but it seems like that's not a guarantee.
If so, it's pretty much impossible for the driver to determine a
preferred mode, so can't do much about it.

As for the special case of a single mode being present in the EEPROM,
I'm not sure if that's something worth considering. If there's only one
it doesn't really matter that it's preferred.

Thierry


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

[Bug 93746] Black screen when reconnecting display

[bugzilla-daemon]

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

Michel Dänzer  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 QA Contact|xorg-t...@lists.x.org   |
  Component|Driver/Radeon   |DRM/Radeon
Product|xorg|DRI
   Assignee|xorg-driver-...@lists.x.org |dri-devel@lists.freedesktop
   ||.org
 Status|NEW |RESOLVED

--- Comment #20 from Michel Dänzer  ---
Thanks for the report. Resolving, as Mario's fixes landed long ago.

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

Re: [PATCH] drm/panel: innolux-p079zca: Use of_device_get_match_data()

[Thierry Reding]

On Tue, Jul 10, 2018 at 01:01:27PM +0200, Thierry Reding wrote:
> From: Thierry Reding 
> 
> Use this helper to get rid of some extra boilerplate code.
> 
> Signed-off-by: Thierry Reding 
> ---
>  drivers/gpu/drm/panel/panel-innolux-p079zca.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c 
> b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> index 185a55060195..62fbaac96823 100644
> --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> @@ -314,14 +314,9 @@ static void innolux_panel_del(struct innolux_panel 
> *innolux)
>  static int innolux_panel_probe(struct mipi_dsi_device *dsi)
>  {
>   const struct panel_desc *desc;
> - const struct of_device_id *id;
>   int err;
>  
> - id = of_match_node(innolux_of_match, dsi->dev.of_node);
> - if (!id)
> - return -ENODEV;
> -
> - desc = id->data;
> + desc = of_device_get_match_data(dsi->dev);
>   dsi->mode_flags = desc->flags;
>   dsi->format = desc->format;
>   dsi->lanes = desc->lanes;

Oops, I just noticed I had parts of this in a stash which causes the
above to fail to build. I've squashed the missing bits into the above,
adding an include for linux/of_device.h and passing >dev instead
of dsi->dev to of_device_get_match_data().

Thierry


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

Re: [PATCH] drm/panel: innolux-p079zca: Use of_device_get_match_data()

[Thierry Reding]

On Tue, Jul 10, 2018 at 01:01:27PM +0200, Thierry Reding wrote:
> From: Thierry Reding 
> 
> Use this helper to get rid of some extra boilerplate code.
> 
> Signed-off-by: Thierry Reding 
> ---
>  drivers/gpu/drm/panel/panel-innolux-p079zca.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)

Applied.

Thierry


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

Re: [RFC PATCH] mm, oom: distinguish blockable mode for mmu notifiers

[Michal Hocko]

On Tue 10-07-18 16:40:40, Leon Romanovsky wrote:
> On Mon, Jul 09, 2018 at 02:29:08PM +0200, Michal Hocko wrote:
> > On Wed 27-06-18 09:44:21, Michal Hocko wrote:
> > > This is the v2 of RFC based on the feedback I've received so far. The
> > > code even compiles as a bonus ;) I haven't runtime tested it yet, mostly
> > > because I have no idea how.
> > >
> > > Any further feedback is highly appreciated of course.
> >
> > Any other feedback before I post this as non-RFC?
> 
> From mlx5 perspective, who is primary user of umem_odp.c your change looks ok.

Can I assume your Acked-by?

Thanks for your review!
-- 
Michal Hocko
SUSE Labs
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCHv2 1/9] drm: Add support for extracting sync signal drive edge from videomode

[Thierry Reding]

On Mon, Jun 18, 2018 at 04:22:34PM +0300, Tomi Valkeinen wrote:
> From: Peter Ujfalusi 
> 
> The sync in some panels needs to be driven by different edge of the pixel
> clock compared to data. This is reflected by the
> DISPLAY_FLAGS_SYNC_(POS|NEG)EDGE in videmode flags.
> Add similar similar definitions for bus_flags and convert the sync drive
> edge via drm_bus_flags_from_videomode().
> 
> Signed-off-by: Peter Ujfalusi 
> Signed-off-by: Tomi Valkeinen 
> Signed-off-by: Jyri Sarha 
> ---
>  drivers/gpu/drm/drm_modes.c | 15 +++
>  include/drm/drm_connector.h |  4 
>  2 files changed, 15 insertions(+), 4 deletions(-)

Also applied this as a dependency for patch 7/9. How do you want to deal
with the remaining patches?

Thierry


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

Re: [RFC PATCH] mm, oom: distinguish blockable mode for mmu notifiers

[Leon Romanovsky]

On Mon, Jul 09, 2018 at 02:29:08PM +0200, Michal Hocko wrote:
> On Wed 27-06-18 09:44:21, Michal Hocko wrote:
> > This is the v2 of RFC based on the feedback I've received so far. The
> > code even compiles as a bonus ;) I haven't runtime tested it yet, mostly
> > because I have no idea how.
> >
> > Any further feedback is highly appreciated of course.
>
> Any other feedback before I post this as non-RFC?

From mlx5 perspective, who is primary user of umem_odp.c your change looks ok.

Thanks


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

Re: [PATCH v2 1/1] drm/panel: Add support for Olimex LCD-OLinuXino panel

[Greg Kroah-Hartman]

On Tue, Jul 10, 2018 at 04:08:54PM +0300, Stefan Mavrodiev wrote:
> On 07/10/2018 01:32 PM, Thierry Reding wrote:
> > > +MODULE_AUTHOR("Stefan Mavrodiev ");
> > > +MODULE_DESCRIPTION("LCD-OLinuXino driver");
> > > +MODULE_LICENSE("GPL v2");
> > This seems to contradict the GPL-2.0+ in the SPDX header.
> Can you elaborate on this?

Please read module.h for what this string means.

thanks,

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

Re: [[DPU]PATCH] drm/msm/dsi: move the API setting PLL src to modeset_init()

[ryadav]

A brief update on this topic:

The DSI clock warnings are addressed after adding runtime_pm support to 
DPU driver [1].
MDSS GDSC is used as genpd w/ above series and is requested by parent 
MDSS device on

behalf of all child devices (like DPU, DSI etc).
Before adding the runtime_pm support, DSI driver probing was happening 
before MDSS GDSC
initialization and is why the RCG clock configuration for byte and pixel 
clocks was not

consumed by hardware leading to the warnings.

Regarding display handover from bootloader to Linux display drivers 
(referred as continuous splash feature),
is supported downstream for DSI displays (where splash screen in handed 
over to kernel drivers w/o re-init'ing the hardware)
and needs coordination (as pointed out by Rob) b/t display, clock, gdsc 
and iommu teams.
But continuous splash for hot-pluggable displays is not attempted so 
far.


We will discuss among the teams to see if continuous slash changes 
(present downstream) in conjunction w/ Rob's work

can make their way upstream.

[1] 
https://lists.freedesktop.org/archives/freedreno/2018-May/002502.html


Thanks,
Rajesh

On 2018-06-28 06:05, Rob Clark wrote:
On Wed, Jun 27, 2018 at 2:48 PM, Doug Anderson  
wrote:

Hi,

On Tue, Jun 26, 2018 at 10:27 AM, Rob Clark  
wrote:
On Tue, Jun 26, 2018 at 11:55 AM, Doug Anderson 
 wrote:

Hi,

On Mon, Jun 25, 2018 at 9:45 PM, Sandeep Panda 
 wrote:

From: Abhinav Kumar 

Setting the DSI PLL src in probe doesn't provide the clock
driver sufficient time to reclaim unused clock resources
from coreboot resulting in warnings from clock driver.

Move the DSI PLL src setting to modeset_init() so that the
clock driver can claim unused display clock resources before
the display driver requests for them again.


IMHO this is a bad design.  Sean and Stephen can feel free to 
override

me, but I think the clock driver should be improved to handle this
case and not require the clock to get disabled before Linux enables
it.



I experimented with it a while back[1] (in this case w/ lk lighting 
up

the display).  In that case I needed both the clk and gdsc code to
realize that clks/gdsc's were on at boot, and fixup the refcnt of the
clks (and parent clocks and so on).  And then when probed the display
driver would check if clocks were enabled to decide to readback the
state from the hw.  (Maybe you can short-circuit some of that if you
only care about DSI panels with a single fixed resolution, but as 
soon

as external displays come into the picture you can't assume so much
about the hw  state.)

BR,
-R

[1] https://github.com/freedreno/kernel-msm/commits/display-handover


It seems like something like that is the right real solution here, but
I guess someone needs to step up and work on it.



yeah, sadly I haven't found time to revisit it (kinda been short on
time to work on display/kernel side of things.. it would be helpful if
qcom would stop coming out with new gpu's so often :-P)

But I am pretty sure if we want a general solution that can also deal
w/ splash screen on hot-pluggable display, and not just the simpler
case of fixed resolution dsi panels, it is going to require
cooperation between clk/gdsc and display driver.


...in general I'm wary of any patch that will not work when
"clk_ignore_unused" is passed as a command line parameter.  It's
useful to be able to use this command line parameter for debugging
sometimes and it would be unfortunate if doing so spewed a bunch of
extra warnings.


for "full distro" config where all the display drivers are built as
modules and prior to real display driver loading you have kernel
inheriting the display via efifb or simplefb, I think we need to be
able to flag certain clocks and power domains (and on other platforms,
perhaps regulators) as "if the bootloader left this on, keep it that
way"..  I guess for simple-fb we could perhaps solve it by attaching
power domains and clks to the simple-fb node, but that doesn't really
work for efifb..

BR,
-R





On a side node, it appears that even without ${SUBJECT} patch the
warnings seem to have gone away with the latest stack of patches I've
been testing.  The warnings don't even come back with
"clk_ignore_unused".  I haven't personally dug into why, but I figured
I'd mention it.


-Doug

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

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

[PATCH v2 4/4] drm/arm/malidp: Added support for AFBC modifiers for all layers except DE_SMART

[Ayan Kumar Halder]

On planes which support AFBC, expose an AFBC modifier for use with BGR888.

Signed-off-by: Ayan Kumar halder 
Reviewed-by: Brian Starkey 
Reviewed-by: Liviu Dudau 

Changes from v2:
- Removed the gerrit change-id
- Replaced DRM_ERROR() with DRM_DEBUG_KMS() in malidp_format_mod_supported()
to report unsupported modifiers.
---
 drivers/gpu/drm/arm/malidp_drv.c|  1 +
 drivers/gpu/drm/arm/malidp_planes.c | 46 +++--
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 262a830..4f6e52e 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -392,6 +392,7 @@ static int malidp_init(struct drm_device *drm)
drm->mode_config.max_height = hwdev->max_line_size;
drm->mode_config.funcs = _mode_config_funcs;
drm->mode_config.helper_private = _mode_config_helpers;
+   drm->mode_config.allow_fb_modifiers = true;
 
ret = malidp_crtc_init(drm);
if (ret) {
diff --git a/drivers/gpu/drm/arm/malidp_planes.c 
b/drivers/gpu/drm/arm/malidp_planes.c
index 0122091..605c5ae 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -124,6 +124,35 @@ static void malidp_plane_atomic_print_state(struct 
drm_printer *p,
drm_printf(p, "\tn_planes=%u\n", ms->n_planes);
 }
 
+static bool malidp_format_mod_supported(struct drm_plane *plane,
+   u32 format, u64 modifier)
+{
+   if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
+   return false;
+
+   /* All the pixel formats are supported without any modifier */
+   if (modifier == DRM_FORMAT_MOD_LINEAR)
+   return true;
+
+   if ((modifier >> 56) != DRM_FORMAT_MOD_VENDOR_ARM)
+   return false;
+
+   if (modifier &
+   ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_MOD_VALID_BITS)) {
+   DRM_DEBUG_KMS("Unsupported modifiers\n");
+   return false;
+   }
+
+   switch (modifier) {
+   case DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
+   AFBC_FORMAT_MOD_YTR |
+   AFBC_FORMAT_MOD_SPARSE):
+   if (format == DRM_FORMAT_BGR888)
+   return true;
+   }
+   return false;
+}
+
 static const struct drm_plane_funcs malidp_de_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
@@ -132,6 +161,7 @@ static const struct drm_plane_funcs malidp_de_plane_funcs = 
{
.atomic_duplicate_state = malidp_duplicate_plane_state,
.atomic_destroy_state = malidp_destroy_plane_state,
.atomic_print_state = malidp_plane_atomic_print_state,
+   .format_mod_supported = malidp_format_mod_supported,
 };
 
 static int malidp_se_check_scaling(struct malidp_plane *mp,
@@ -526,6 +556,13 @@ int malidp_de_planes_init(struct drm_device *drm)
u32 *formats;
int ret, i, j, n;
 
+   static const u64 modifiers[] = {
+   DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
+   AFBC_FORMAT_MOD_YTR | AFBC_FORMAT_MOD_SPARSE),
+   DRM_FORMAT_MOD_LINEAR,
+   DRM_FORMAT_MOD_INVALID
+   };
+
formats = kcalloc(map->n_pixel_formats, sizeof(*formats), GFP_KERNEL);
if (!formats) {
ret = -ENOMEM;
@@ -549,9 +586,14 @@ int malidp_de_planes_init(struct drm_device *drm)
 
plane_type = (i == 0) ? DRM_PLANE_TYPE_PRIMARY :
DRM_PLANE_TYPE_OVERLAY;
+
+   /*
+* All the layers except smart layer supports AFBC modifiers.
+*/
ret = drm_universal_plane_init(drm, >base, crtcs,
-  _de_plane_funcs, formats,
-  n, NULL, plane_type, NULL);
+   _de_plane_funcs, formats, n,
+   (id == DE_SMART) ? NULL : modifiers, 
plane_type, NULL);
+
if (ret < 0)
goto cleanup;
 
-- 
2.7.4

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

[PATCH v2 2/4] drm/arm/malidp: Implemented the size validation for AFBC framebuffers

[Ayan Kumar Halder]

AFBC buffers include additional metadata which increases the required
allocation size. Implement the appropriate size validation and sanity
checking for AFBC buffers.
Added malidp specific function for framebuffer creation. This checks
if the framebuffer has AFBC modifiers and if so, it verifies the
necessary constraints on the size, alignment, offsets and pitch.

Changes from v2:
- Replaced DRM_ERROR() with DRM_DEBUG_KMS() in
malidp_verify_afbc_framebuffer_caps() and malidp_verify_afbc_framebuffer_size()

Signed-off-by: Ayan Kumar halder 
Reviewed-by: Brian Starkey 
Reviewed-by: Liviu Dudau 
---
 drivers/gpu/drm/arm/malidp_drv.c | 128 ++-
 drivers/gpu/drm/arm/malidp_hw.h  |   5 ++
 2 files changed, 132 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 8d20faa..262a830 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -35,6 +35,7 @@
 #include "malidp_hw.h"
 
 #define MALIDP_CONF_VALID_TIMEOUT  250
+#define AFBC_HEADER_SIZE   16
 
 static void malidp_write_gamma_table(struct malidp_hw_device *hwdev,
 u32 data[MALIDP_COEFFTAB_NUM_COEFFS])
@@ -245,8 +246,133 @@ static const struct drm_mode_config_helper_funcs 
malidp_mode_config_helpers = {
.atomic_commit_tail = malidp_atomic_commit_tail,
 };
 
+static bool
+malidp_verify_afbc_framebuffer_caps(struct drm_device *dev,
+   const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+   const struct drm_format_info *info;
+
+   if ((mode_cmd->modifier[0] >> 56) != DRM_FORMAT_MOD_VENDOR_ARM) {
+   DRM_DEBUG_KMS("Unknown modifier (not Arm)\n");
+   return false;
+   }
+
+   if (mode_cmd->modifier[0] &
+   ~DRM_FORMAT_MOD_ARM_AFBC(AFBC_MOD_VALID_BITS)) {
+   DRM_DEBUG_KMS("Unsupported modifiers\n");
+   return false;
+   }
+
+   info = drm_get_format_info(dev, mode_cmd);
+   if (!info) {
+   DRM_DEBUG_KMS("Unable to get the format information\n");
+   return false;
+   }
+
+   if (info->num_planes != 1) {
+   DRM_DEBUG_KMS("AFBC buffers expect one plane\n");
+   return false;
+   }
+
+   if (mode_cmd->offsets[0] != 0) {
+   DRM_DEBUG_KMS("AFBC buffers' plane offset should be 0\n");
+   return false;
+   }
+
+   switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
+   case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
+   if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) {
+   DRM_DEBUG_KMS("AFBC buffers must be aligned to 16 
pixels\n");
+   return false;
+   }
+   break;
+   default:
+   DRM_DEBUG_KMS("Unsupported AFBC block size\n");
+   return false;
+   }
+
+   return true;
+}
+
+static bool
+malidp_verify_afbc_framebuffer_size(struct drm_device *dev,
+   struct drm_file *file,
+   const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+   int n_superblocks = 0;
+   const struct drm_format_info *info;
+   struct drm_gem_object *objs = NULL;
+   u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
+   u32 afbc_superblock_width = 0, afbc_size = 0;
+
+   switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
+   case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
+   afbc_superblock_height = 16;
+   afbc_superblock_width = 16;
+   break;
+   default:
+   DRM_DEBUG_KMS("AFBC superblock size is not supported\n");
+   return false;
+   }
+
+   info = drm_get_format_info(dev, mode_cmd);
+
+   n_superblocks = (mode_cmd->width / afbc_superblock_width) *
+   (mode_cmd->height / afbc_superblock_height);
+
+   afbc_superblock_size = info->cpp[0] * afbc_superblock_width *
+   afbc_superblock_height;
+
+   afbc_size = ALIGN(n_superblocks * AFBC_HEADER_SIZE, 128);
+
+   if (mode_cmd->width * info->cpp[0] != mode_cmd->pitches[0]) {
+   DRM_DEBUG_KMS("Invalid value of pitch (=%u) should be same as 
width (=%u) * cpp (=%u)\n",
+ mode_cmd->pitches[0], mode_cmd->width, 
info->cpp[0]);
+   return false;
+   }
+
+   objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
+   if (!objs) {
+   DRM_DEBUG_KMS("Failed to lookup GEM object\n");
+   return false;
+   }
+
+   if (objs->size < afbc_size) {
+   DRM_DEBUG_KMS("buffer size (%zu) too small for AFBC buffer size 
= %u\n",
+ objs->size, afbc_size);
+   drm_gem_object_put_unlocked(objs);
+   return false;
+   }
+
+   drm_gem_object_put_unlocked(objs);
+
+   

[PATCH v2 3/4] drm/arm/malidp: Set the AFBC register bits if the framebuffer has AFBC modifier

[Ayan Kumar Halder]

Added the AFBC decoder registers for DP500 , DP550 and DP650.
These registers control the processing of AFBC buffers. It controls various
features like AFBC decoder enable, lossless transformation and block split
as well as setting of the left, right, top and bottom cropping of AFBC buffers
(in number of pixels).
All the layers (except DE_SMART) support framebuffers with AFBC modifiers.
One needs to set the pixel values of the top, left, bottom and right cropping
for the AFBC framebuffer.
Added the functionality in malidp_de_plane_update() to set the various
registers for AFBC decoder, depending on the modifiers.

Changes from v2:-
- Removed the "if (fb->modifier)" check from malidp_de_plane_update()
and added it in malidp_de_set_plane_afbc(). This will consolidate all the
AFBC specific register configurations in a single function ie
malidp_de_set_plane_afbc().

Signed-off-by: Ayan Kumar halder 
Reviewed-by: Brian Starkey 
---
 drivers/gpu/drm/arm/malidp_hw.c | 27 +++-
 drivers/gpu/drm/arm/malidp_hw.h |  2 +
 drivers/gpu/drm/arm/malidp_planes.c | 83 +
 drivers/gpu/drm/arm/malidp_regs.h   | 20 +
 4 files changed, 113 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 4dbf39f..fd6b510 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -76,33 +76,38 @@ static const struct malidp_format_id malidp550_de_formats[] 
= {
 
 static const struct malidp_layer malidp500_layers[] = {
{ DE_VIDEO1, MALIDP500_DE_LV_BASE, MALIDP500_DE_LV_PTR_BASE,
-   MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, ROTATE_ANY },
+   MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, ROTATE_ANY,
+   MALIDP500_DE_LV_AD_CTRL },
{ DE_GRAPHICS1, MALIDP500_DE_LG1_BASE, MALIDP500_DE_LG1_PTR_BASE,
-   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
+   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, MALIDP500_DE_LG1_AD_CTRL },
{ DE_GRAPHICS2, MALIDP500_DE_LG2_BASE, MALIDP500_DE_LG2_PTR_BASE,
-   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
+   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, MALIDP500_DE_LG2_AD_CTRL },
 };
 
 static const struct malidp_layer malidp550_layers[] = {
{ DE_VIDEO1, MALIDP550_DE_LV1_BASE, MALIDP550_DE_LV1_PTR_BASE,
-   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
+   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
+   MALIDP550_DE_LV1_AD_CTRL },
{ DE_GRAPHICS1, MALIDP550_DE_LG_BASE, MALIDP550_DE_LG_PTR_BASE,
-   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
+   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, MALIDP550_DE_LG_AD_CTRL },
{ DE_VIDEO2, MALIDP550_DE_LV2_BASE, MALIDP550_DE_LV2_PTR_BASE,
-   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
+   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
+   MALIDP550_DE_LV2_AD_CTRL },
{ DE_SMART, MALIDP550_DE_LS_BASE, MALIDP550_DE_LS_PTR_BASE,
-   MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE },
+   MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE, 0 },
 };
 
 static const struct malidp_layer malidp650_layers[] = {
{ DE_VIDEO1, MALIDP550_DE_LV1_BASE, MALIDP550_DE_LV1_PTR_BASE,
-   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
+   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
+   MALIDP550_DE_LV1_AD_CTRL },
{ DE_GRAPHICS1, MALIDP550_DE_LG_BASE, MALIDP550_DE_LG_PTR_BASE,
-   MALIDP_DE_LG_STRIDE, 0, ROTATE_COMPRESSED },
+   MALIDP_DE_LG_STRIDE, 0, ROTATE_COMPRESSED, 
MALIDP550_DE_LG_AD_CTRL },
{ DE_VIDEO2, MALIDP550_DE_LV2_BASE, MALIDP550_DE_LV2_PTR_BASE,
-   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY },
+   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
+   MALIDP550_DE_LV2_AD_CTRL },
{ DE_SMART, MALIDP550_DE_LS_BASE, MALIDP550_DE_LS_PTR_BASE,
-   MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE },
+   MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE, 0 },
 };
 
 #define SE_N_SCALING_COEFFS96
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 4390243..bbe6883 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -67,6 +67,8 @@ struct malidp_layer {
u16 stride_offset;  /* offset to the first stride register. */
s16 yuv2rgb_offset; /* offset to the YUV->RGB matrix entries */
enum rotation_features rot;/* type of rotation supported */
+   /* address offset for the AFBC decoder registers */
+   u16 afbc_decoder_offset;
 };
 
 enum malidp_scaling_coeff_set {
diff --git a/drivers/gpu/drm/arm/malidp_planes.c 
b/drivers/gpu/drm/arm/malidp_planes.c
index 533cdde..0122091 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ 

[PATCH v2 1/4] drm/arm/malidp: Add modifier definitions for describing Arm Framebuffer Compression (AFBC).

[Ayan Kumar Halder]

AFBC is a proprietary lossless image compression protocol and format.
It provides fine-grained random access and minimizes the amount of data
transferred between IP blocks.
AFBC has several features which may be supported and/or used, which are
represented using bits in the modifier. Not all combinations are valid,
and different devices or use-cases may support different combinations.

Changes from v2:-
- Added ack by Maarten Lankhorst

Signed-off-by: Rosen Zhelev 
Signed-off-by: Ayan Kumar halder 
Reviewed-by: Brian Starkey 
Reviewed-by: Liviu Dudau 
Reviewed-by: James (Qian) Wang 
Acked-by: Maarten Lankhorst 
---
 include/uapi/drm/drm_fourcc.h | 83 +++
 1 file changed, 83 insertions(+)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index d5e5235..d43949b 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -183,6 +183,7 @@ extern "C" {
 #define DRM_FORMAT_MOD_VENDOR_QCOM0x05
 #define DRM_FORMAT_MOD_VENDOR_VIVANTE 0x06
 #define DRM_FORMAT_MOD_VENDOR_BROADCOM 0x07
+#define DRM_FORMAT_MOD_VENDOR_ARM 0x08
 /* add more to the end as needed */
 
 #define DRM_FORMAT_RESERVED  ((1ULL << 56) - 1)
@@ -485,6 +486,88 @@ extern "C" {
  */
 #define DRM_FORMAT_MOD_BROADCOM_UIF fourcc_mod_code(BROADCOM, 6)
 
+/*
+ * Arm Framebuffer Compression (AFBC) modifiers
+ *
+ * AFBC is a proprietary lossless image compression protocol and format.
+ * It provides fine-grained random access and minimizes the amount of data
+ * transferred between IP blocks.
+ *
+ * AFBC has several features which may be supported and/or used, which are
+ * represented using bits in the modifier. Not all combinations are valid,
+ * and different devices or use-cases may support different combinations.
+ */
+#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode)   fourcc_mod_code(ARM, 
__afbc_mode)
+
+/*
+ * AFBC superblock size
+ *
+ * Indicates the superblock size(s) used for the AFBC buffer. The buffer
+ * size (in pixels) must be aligned to a multiple of the superblock size.
+ * Four lowest significant bits(LSBs) are reserved for block size.
+ */
+#define AFBC_FORMAT_MOD_BLOCK_SIZE_MASK  0xf
+#define AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 (1ULL)
+#define AFBC_FORMAT_MOD_BLOCK_SIZE_32x8  (2ULL)
+
+/*
+ * AFBC lossless colorspace transform
+ *
+ * Indicates that the buffer makes use of the AFBC lossless colorspace
+ * transform.
+ */
+#define AFBC_FORMAT_MOD_YTR (1ULL <<  4)
+
+/*
+ * AFBC block-split
+ *
+ * Indicates that the payload of each superblock is split. The second
+ * half of the payload is positioned at a predefined offset from the start
+ * of the superblock payload.
+ */
+#define AFBC_FORMAT_MOD_SPLIT   (1ULL <<  5)
+
+/*
+ * AFBC sparse layout
+ *
+ * This flag indicates that the payload of each superblock must be stored at a
+ * predefined position relative to the other superblocks in the same AFBC
+ * buffer. This order is the same order used by the header buffer. In this mode
+ * each superblock is given the same amount of space as an uncompressed
+ * superblock of the particular format would require, rounding up to the next
+ * multiple of 128 bytes in size.
+ */
+#define AFBC_FORMAT_MOD_SPARSE  (1ULL <<  6)
+
+/*
+ * AFBC copy-block restrict
+ *
+ * Buffers with this flag must obey the copy-block restriction. The restriction
+ * is such that there are no copy-blocks referring across the border of 8x8
+ * blocks. For the subsampled data the 8x8 limitation is also subsampled.
+ */
+#define AFBC_FORMAT_MOD_CBR (1ULL <<  7)
+
+/*
+ * AFBC tiled layout
+ *
+ * The tiled layout groups superblocks in 8x8 or 4x4 tiles, where all
+ * superblocks inside a tile are stored together in memory. 8x8 tiles are used
+ * for pixel formats up to and including 32 bpp while 4x4 tiles are used for
+ * larger bpp formats. The order between the tiles is scan line.
+ * When the tiled layout is used, the buffer size (in pixels) must be aligned
+ * to the tile size.
+ */
+#define AFBC_FORMAT_MOD_TILED   (1ULL <<  8)
+
+/*
+ * AFBC solid color blocks
+ *
+ * Indicates that the buffer makes use of solid-color blocks, whereby bandwidth
+ * can be reduced if a whole superblock is a single color.
+ */
+#define AFBC_FORMAT_MOD_SC  (1ULL <<  9)
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.7.4

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

[PATCH v2 0/4] Add support for Arm Framebuffer Compression(AFBC)

[Ayan Kumar Halder]

In the current series of patches, we are trying to add support for AFBC
modifiers in malidp. AFBC modifiers adds some constraints to framebuffer
size, alignment, pitch, formats, etc. Here we are trying to add support
for one combination of AFBC modifier ie AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 |
AFBC_FORMAT_MOD_SPARSE | AFBC_FORMAT_MOD_YTR.
In future, we intend to add support for more combination of AFBC modifiers.
Currently, we are trying to enable a basic support of AFBC in malidp.

Changes from v2:
- Added ack by Maarten Lankhorst 
for patch 1. However, this has been kept in this series in order to help
reviewers review the other patches (which are related to patch 1)
- For patches 2 and 4, replaced DRM_ERROR() with DRM_DEBUG_KMS()
- For patch 3, reworked malidp_de_set_plane_afbc() so as to consolidate
all afbc specific register configuration in this.

Ayan Kumar Halder (4):
  drm/arm/malidp: Add modifier definitions for describing Arm
Framebuffer Compression (AFBC).
  drm/arm/malidp: Implemented the size validation for AFBC framebuffers
  drm/arm/malidp: Set the AFBC register bits if the framebuffer has AFBC
modifier
  drm/arm/malidp: Added support for AFBC modifiers for all layers except
DE_SMART

 drivers/gpu/drm/arm/malidp_drv.c| 129 +++-
 drivers/gpu/drm/arm/malidp_hw.c |  27 +---
 drivers/gpu/drm/arm/malidp_hw.h |   7 ++
 drivers/gpu/drm/arm/malidp_planes.c | 129 +---
 drivers/gpu/drm/arm/malidp_regs.h   |  20 ++
 include/uapi/drm/drm_fourcc.h   |  83 +++
 6 files changed, 373 insertions(+), 22 deletions(-)

-- 
2.7.4

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

Re: [PATCH 08/10] drm/crc: Cleanup crtc_crc_open function

[Kumar, Mahesh]

Hi,

Thanks for the review.

On 7/10/2018 5:19 PM, Laurent Pinchart wrote:

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:27 EEST Mahesh Kumar wrote:

This patch make changes to allocate crc-entries buffer before
enabling CRC generation.
It moves all the failure check early in the function before setting
the source or memory allocation.
Now set_crc_source takes only two variable inputs, values_cnt we
already gets as part of verify_crc_source.

Signed-off-by: Mahesh Kumar 
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Maarten Lankhorst 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  3 +-
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c  |  4 +-
  drivers/gpu/drm/drm_debugfs_crc.c  | 52 ---
  drivers/gpu/drm/i915/intel_drv.h   |  3 +-
  drivers/gpu/drm/i915/intel_pipe_crc.c  |  4 +-
  drivers/gpu/drm/rcar-du/rcar_du_crtc.c |  5 +--
  drivers/gpu/drm/rockchip/rockchip_drm_vop.c|  6 +--
  include/drm/drm_crtc.h |  3 +-
  8 files changed, 30 insertions(+), 50 deletions(-)

[snip]


diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
b/drivers/gpu/drm/drm_debugfs_crc.c index f4d76528d24c..739a813b4b09 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -124,11 +124,9 @@ static ssize_t crc_control_write(struct file *file,
const char __user *ubuf, if (source[len] == '\n')
source[len] = '\0';

-   if (crtc->funcs->verify_crc_source) {
-   ret = crtc->funcs->verify_crc_source(crtc, source, _cnt);
-   if (ret)
-   return ret;
-   }
+   ret = crtc->funcs->verify_crc_source(crtc, source, _cnt);
+   if (ret)
+   return ret;

spin_lock_irq(>lock);

@@ -193,12 +191,15 @@ static int crtc_crc_open(struct inode *inode, struct
file *filep) return ret;
}

-   if (crtc->funcs->verify_crc_source) {
-   ret = crtc->funcs->verify_crc_source(crtc, crc->source,
-_cnt);
-   if (ret)
-   return ret;
-   }
+   ret = crtc->funcs->verify_crc_source(crtc, crc->source, _cnt);
+   if (ret)
+   return ret;
+
+   if (WARN_ON(values_cnt > DRM_MAX_CRC_NR))
+   return -EINVAL;
+
+   if (WARN_ON(values_cnt == 0))
+   return -EINVAL;

spin_lock_irq(>lock);
if (!crc->opened)
@@ -210,30 +211,22 @@ static int crtc_crc_open(struct inode *inode, struct
file *filep) if (ret)
return ret;

-   ret = crtc->funcs->set_crc_source(crtc, crc->source, _cnt);
-   if (ret)
-   goto err;
-
-   if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) {
-   ret = -EINVAL;
-   goto err_disable;
-   }
-
-   if (WARN_ON(values_cnt == 0)) {
-   ret = -EINVAL;
-   goto err_disable;
-   }
-
entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL);
if (!entries) {
ret = -ENOMEM;
-   goto err_disable;
+   goto err;
}

If you moved allocation before the !crc->opened check, you could group the two
code blocks protected by the crc->lock.

agree, will update in next version.
-Mahesh

spin_lock_irq(>lock);
crc->entries = entries;
crc->values_cnt = values_cnt;
+   spin_unlock_irq(>lock);

+   ret = crtc->funcs->set_crc_source(crtc, crc->source);
+   if (ret)
+   goto err;
+
+   spin_lock_irq(>lock);
/*
 * Only return once we got a first frame, so userspace doesn't have to
 * guess when this particular piece of HW will be ready to start
@@ -250,7 +243,7 @@ static int crtc_crc_open(struct inode *inode, struct
file *filep) return 0;

  err_disable:
-   crtc->funcs->set_crc_source(crtc, NULL, _cnt);
+   crtc->funcs->set_crc_source(crtc, NULL);
  err:
spin_lock_irq(>lock);
crtc_crc_cleanup(crc);
@@ -262,9 +255,8 @@ static int crtc_crc_release(struct inode *inode, struct
file *filep) {
struct drm_crtc *crtc = filep->f_inode->i_private;
struct drm_crtc_crc *crc = >crc;
-   size_t values_cnt;

-   crtc->funcs->set_crc_source(crtc, NULL, _cnt);
+   crtc->funcs->set_crc_source(crtc, NULL);

spin_lock_irq(>lock);
crtc_crc_cleanup(crc);
@@ -370,7 +362,7 @@ int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc)
  {
struct dentry *crc_ent, *ent;

-   if (!crtc->funcs->set_crc_source)
+   if (!crtc->funcs->set_crc_source || !crtc->funcs->verify_crc_source)
return 0;

crc_ent = debugfs_create_dir("crc", crtc->debugfs_entry);

[snip]



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

Re: [PATCH v5 0/8] drm: Add generic fbdev emulation

[Noralf Trønnes]

Den 03.07.2018 18.03, skrev Noralf Trønnes:

This patchset adds generic fbdev emulation for drivers that supports GEM
based dumb buffers which support .gem_prime_vmap and gem_prime_mmap. An
API is begun to support in-kernel clients in general.

I've squashed the client patches to ease review.
All patches have ack's and rb's so I'll apply this next week unless
something more comes up. It's taken me 6 months to get this done so I
look forward to getting it applied.

Thanks a lot Daniel for helping me make this happen!

Noralf.

Changes since version 4:
- Squash the two client patches to ease review.
- Remove drm_client_put() doc references.
- Remove drm_client_funcs->release, it's use went away in version 3.
- Add drm_client_dev_hotplug() doc

Changes since version 3:
- drm/cma-helper: Use the generic fbdev emulation: Fix error path
- Remove setting .lastclose in new tinydrm driver ili9341

Changes since version 2:
- Applied first 3 patches to drm-misc-next
- Drop client reference counting and only allow the driver to release
clients.

Noralf Trønnes (8):
   drm: Begin an API for in-kernel clients
   drm/fb-helper: Add generic fbdev emulation .fb_probe function
   drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap
   drm/cma-helper: Use the generic fbdev emulation
   drm/debugfs: Add internal client debugfs file
   drm/fb-helper: Finish the generic fbdev emulation
   drm/tinydrm: Use drm_fbdev_generic_setup()
   drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs()


Series applied to drm-misc, thanks for reviewing.

Noralf.


  Documentation/gpu/drm-client.rst|  12 +
  Documentation/gpu/index.rst |   1 +
  drivers/gpu/drm/Makefile|   2 +-
  drivers/gpu/drm/drm_client.c| 415 
  drivers/gpu/drm/drm_debugfs.c   |   7 +
  drivers/gpu/drm/drm_drv.c   |   8 +
  drivers/gpu/drm/drm_fb_cma_helper.c | 379 +++--
  drivers/gpu/drm/drm_fb_helper.c | 316 -
  drivers/gpu/drm/drm_file.c  |   3 +
  drivers/gpu/drm/drm_probe_helper.c  |   3 +
  drivers/gpu/drm/pl111/pl111_drv.c   |   2 +
  drivers/gpu/drm/tinydrm/core/tinydrm-core.c |   3 +-
  drivers/gpu/drm/tinydrm/ili9225.c   |   1 -
  drivers/gpu/drm/tinydrm/ili9341.c   |   1 -
  drivers/gpu/drm/tinydrm/mi0283qt.c  |   1 -
  drivers/gpu/drm/tinydrm/st7586.c|   1 -
  drivers/gpu/drm/tinydrm/st7735r.c   |   1 -
  include/drm/drm_client.h| 139 ++
  include/drm/drm_device.h|  21 ++
  include/drm/drm_fb_cma_helper.h |   6 -
  include/drm/drm_fb_helper.h |  38 +++
  21 files changed, 1007 insertions(+), 353 deletions(-)
  create mode 100644 Documentation/gpu/drm-client.rst
  create mode 100644 drivers/gpu/drm/drm_client.c
  create mode 100644 include/drm/drm_client.h



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

Re: [PATCH 05/10] drm/rcar-du/crc: Implement verify_crc_source callback

[Kumar, Mahesh]

Hi,

thanks foe the review.

On 7/10/2018 5:07 PM, Laurent Pinchart wrote:

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:24 EEST Mahesh Kumar wrote:

This patch implements "verify_crc_source" callback function for
rcar drm driver.

Signed-off-by: Mahesh Kumar 
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Maarten Lankhorst 
---
  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 40 +++
  1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 15dc9caa128b..24eeaa7e14d7
100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -756,6 +756,45 @@ static void rcar_du_crtc_disable_vblank(struct drm_crtc
*crtc) rcrtc->vblank_enable = false;
  }

+static int rcar_du_crtc_verify_crc_source(struct drm_crtc *crtc,
+ const char *source_name,
+ size_t *values_cnt)
+{
+   struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
+   unsigned int index = 0;
+   unsigned int i;
+   int ret;
+
+   /*
+* Parse the source name. Supported values are "plane%u" to compute the
+* CRC on an input plane (%u is the plane ID), and "auto" to compute the
+* CRC on the composer (VSP) output.
+*/
+   if (!source_name || !strcmp(source_name, "auto")) {
+   goto out;
+   } else if (strstarts(source_name, "plane")) {
+   ret = kstrtouint(source_name + strlen("plane"), 10, );
+   if (ret < 0)
+   return ret;
+
+   for (i = 0; i < rcrtc->vsp->num_planes; ++i) {
+   if (index == rcrtc->vsp->planes[i].plane.base.id) {
+   index = i;
+   break;
+   }
+   }
+
+   if (i >= rcrtc->vsp->num_planes)
+   return -EINVAL;
+   } else {
+   return -EINVAL;
+   }
+
+out:
+   *values_cnt = 1;
+   return 0;

This duplicates lots of code from the rcar_du_crtc_set_crc_source() function.
Could you please extract it to a shared function ?

Agree, it duplicates the code but "index" is needed by set_crc_source call
anyway will create a wrapper to avoid duplication of code.


Could you please also implement support for the .get_crc_sources() operation ?
I think doing so might show limitations in the current API, namely the fact
that the list will need to be dynamically created for this driver.
for that I think rcar_du_crtc_create function can build a dynamic list 
during initializing crtc functions, unless plane can be dynamically 
allocated to any CRTC. this is the place where I need input from maintainer.


-Mahesh



+}
+
  static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc,
   const char *source_name,
   size_t *values_cnt)
@@ -861,6 +900,7 @@ static const struct drm_crtc_funcs crtc_funcs_gen3 = {
.enable_vblank = rcar_du_crtc_enable_vblank,
.disable_vblank = rcar_du_crtc_disable_vblank,
.set_crc_source = rcar_du_crtc_set_crc_source,
+   .verify_crc_source = rcar_du_crtc_verify_crc_source,
  };

  /* 


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

Re: [PATCH] drm/panel: innolux-p079zca: Use of_device_get_match_data()

[Heiko Stuebner]

Am Dienstag, 10. Juli 2018, 13:01:27 CEST schrieb Thierry Reding:
> From: Thierry Reding 
> 
> Use this helper to get rid of some extra boilerplate code.
> 
> Signed-off-by: Thierry Reding 

Reviewed-by: Heiko Stuebner 



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

Re: [RFC PATCH 3/4] drm/arm/malidp: Set the AFBC register bits if the framebuffer has AFBC modifier

[Ayan Halder]

Hi Liviu,
On Thu, Jul 05, 2018 at 06:38:21PM +0100, Liviu Dudau wrote:
> On Thu, Jul 05, 2018 at 02:31:47PM +0100, Ayan Halder wrote:
> > On Tue, Jun 26, 2018 at 02:17:17PM +0100, Liviu Dudau wrote:
> > > Hi Ayan,
> > > 
> > > Thanks for the patch! I have some small comments to make:
> > > 
> > > On Fri, Jun 15, 2018 at 02:51:33PM +0100, Ayan Kumar Halder wrote:
> > > > Added the AFBC decoder registers for DP500 , DP550 and DP650.
> > > > These registers control the processing of AFBC buffers. It controls 
> > > > various
> > > > features like AFBC decoder enable, lossless transformation and block 
> > > > split
> > > > as well as setting of the left, right, top and bottom cropping of AFBC 
> > > > buffers
> > > > (in number of pixels).
> > > > All the layers (except DE_SMART) support framebuffers with AFBC 
> > > > modifiers.
> > > > One needs to set the pixel values of the top, left, bottom and right 
> > > > cropping
> > > > for the AFBC framebuffer.
> > > > Added the functionality in malidp_de_plane_update() to set the various
> > > > registers for AFBC decoder, depending on the modifiers.
> > > > 
> > > > Signed-off-by: Ayan Kumar halder 
> > > > Reviewed-by: Brian Starkey 
> > > > ---
> > > >  drivers/gpu/drm/arm/malidp_hw.c | 27 -
> > > >  drivers/gpu/drm/arm/malidp_hw.h |  2 +
> > > >  drivers/gpu/drm/arm/malidp_planes.c | 81 
> > > > +
> > > >  drivers/gpu/drm/arm/malidp_regs.h   | 20 +
> > > >  4 files changed, 111 insertions(+), 19 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c 
> > > > b/drivers/gpu/drm/arm/malidp_hw.c
> > > > index 4dbf39f..fd6b510 100644
> > > > --- a/drivers/gpu/drm/arm/malidp_hw.c
> > > > +++ b/drivers/gpu/drm/arm/malidp_hw.c
> > > > @@ -76,33 +76,38 @@ static const struct malidp_format_id 
> > > > malidp550_de_formats[] = {
> > > >  
> > > >  static const struct malidp_layer malidp500_layers[] = {
> > > > { DE_VIDEO1, MALIDP500_DE_LV_BASE, MALIDP500_DE_LV_PTR_BASE,
> > > > -   MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, ROTATE_ANY 
> > > > },
> > > > +   MALIDP_DE_LV_STRIDE0, MALIDP500_LV_YUV2RGB, ROTATE_ANY,
> > > > +   MALIDP500_DE_LV_AD_CTRL },
> > > > { DE_GRAPHICS1, MALIDP500_DE_LG1_BASE, 
> > > > MALIDP500_DE_LG1_PTR_BASE,
> > > > -   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
> > > > +   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, 
> > > > MALIDP500_DE_LG1_AD_CTRL },
> > > > { DE_GRAPHICS2, MALIDP500_DE_LG2_BASE, 
> > > > MALIDP500_DE_LG2_PTR_BASE,
> > > > -   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
> > > > +   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, 
> > > > MALIDP500_DE_LG2_AD_CTRL },
> > > >  };
> > > >  
> > > >  static const struct malidp_layer malidp550_layers[] = {
> > > > { DE_VIDEO1, MALIDP550_DE_LV1_BASE, MALIDP550_DE_LV1_PTR_BASE,
> > > > -   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY 
> > > > },
> > > > +   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
> > > > +   MALIDP550_DE_LV1_AD_CTRL },
> > > > { DE_GRAPHICS1, MALIDP550_DE_LG_BASE, MALIDP550_DE_LG_PTR_BASE,
> > > > -   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY },
> > > > +   MALIDP_DE_LG_STRIDE, 0, ROTATE_ANY, 
> > > > MALIDP550_DE_LG_AD_CTRL },
> > > > { DE_VIDEO2, MALIDP550_DE_LV2_BASE, MALIDP550_DE_LV2_PTR_BASE,
> > > > -   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY 
> > > > },
> > > > +   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
> > > > +   MALIDP550_DE_LV2_AD_CTRL },
> > > > { DE_SMART, MALIDP550_DE_LS_BASE, MALIDP550_DE_LS_PTR_BASE,
> > > > -   MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE },
> > > > +   MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE, 0 },
> > > >  };
> > > >  
> > > >  static const struct malidp_layer malidp650_layers[] = {
> > > > { DE_VIDEO1, MALIDP550_DE_LV1_BASE, MALIDP550_DE_LV1_PTR_BASE,
> > > > -   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY 
> > > > },
> > > > +   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
> > > > +   MALIDP550_DE_LV1_AD_CTRL },
> > > > { DE_GRAPHICS1, MALIDP550_DE_LG_BASE, MALIDP550_DE_LG_PTR_BASE,
> > > > -   MALIDP_DE_LG_STRIDE, 0, ROTATE_COMPRESSED },
> > > > +   MALIDP_DE_LG_STRIDE, 0, ROTATE_COMPRESSED, 
> > > > MALIDP550_DE_LG_AD_CTRL },
> > > > { DE_VIDEO2, MALIDP550_DE_LV2_BASE, MALIDP550_DE_LV2_PTR_BASE,
> > > > -   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY 
> > > > },
> > > > +   MALIDP_DE_LV_STRIDE0, MALIDP550_LV_YUV2RGB, ROTATE_ANY,
> > > > +   MALIDP550_DE_LV2_AD_CTRL },
> > > > { DE_SMART, MALIDP550_DE_LS_BASE, MALIDP550_DE_LS_PTR_BASE,
> > > > -   MALIDP550_DE_LS_R1_STRIDE, 0, ROTATE_NONE },
> > > > +  

Re: [PATCH 01/10] drm: crc: Introduce verify_crc_source callback

[Kumar, Mahesh]

Hi,


On 7/10/2018 5:40 PM, Laurent Pinchart wrote:

Hi Mahesh,

On Tuesday, 10 July 2018 14:54:11 EEST Kumar, Mahesh wrote:

On 7/10/2018 4:56 PM, Laurent Pinchart wrote:

On Monday, 2 July 2018 14:07:20 EEST Mahesh Kumar wrote:

This patch adds a new callback function "verify_crc_source" which will
be used during setting the crc source in control node and while opening
data node for crc reading. This will help in avoiding setting of wrong
string for source.

Why do you need to verify the source in the open() operation ? Isn't it
enough to verify it in the write() handler ? The answer to this question
might lie in patch 08/10, in which case I'd add the .verify_crc_source()
call in open() in that patch, not here.

Yes, final goal is achieved by patch 08/10. But if crc_read is called
before setting the source, it may have wrong source set in that case I
wanted to return early at least for the drivers implemented
verify_crc_source. If you still think otherwise I will modify
accordingly in next series.

I don't disagree with you, but I don't think this issue is new. As I got a bit
confused as to why the call is needed in open() (there's no explanation in
this patch), I think fixing the problem in 08/10 would be better.

ok sure :)
-Mahesh



Signed-off-by: Mahesh Kumar 
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Maarten Lankhorst 
---

   drivers/gpu/drm/drm_debugfs_crc.c | 15 +++
   include/drm/drm_crtc.h| 15 +++
   2 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
b/drivers/gpu/drm/drm_debugfs_crc.c index 9f8312137cad..c6a725b79ac6
100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -87,6 +87,8 @@ static ssize_t crc_control_write(struct file *file,
const
char __user *ubuf, struct drm_crtc *crtc = m->private;

struct drm_crtc_crc *crc = >crc;
char *source;

+   size_t values_cnt;
+   int ret = 0;

if (len == 0)

return 0;

@@ -104,6 +106,12 @@ static ssize_t crc_control_write(struct file *file,
const char __user *ubuf, if (source[len] == '\n')

source[len] = '\0';

+   if (crtc->funcs->verify_crc_source) {
+   ret = crtc->funcs->verify_crc_source(crtc, source, _cnt);
+   if (ret)
+   return ret;
+   }
+

spin_lock_irq(>lock);

if (crc->opened) {

@@ -167,6 +175,13 @@ static int crtc_crc_open(struct inode *inode, struct
file *filep) return ret;

}

+   if (crtc->funcs->verify_crc_source) {
+   ret = crtc->funcs->verify_crc_source(crtc, crc->source,
+_cnt);
+   if (ret)
+   return ret;
+   }
+

spin_lock_irq(>lock);
if (!crc->opened)

crc->opened = true;

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 23eddbccab10..1a6dcbf91744 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -661,6 +661,21 @@ struct drm_crtc_funcs {

 */

int (*set_crc_source)(struct drm_crtc *crtc, const char *source,

  size_t *values_cnt);

+   /**
+* @verify_crc_source:
+*
+* verifies the source of CRC checksums of frames before setting the
+* source for CRC and during crc open.
+*
+* This callback is optional if the driver does not support any CRC
+* generation functionality.
+*
+* RETURNS:
+*
+* 0 on success or a negative error code on failure.
+*/
+   int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
+size_t *values_cnt);

/**

 * @atomic_print_state:




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

Re: [PATCH 02/10] drm: crc: Introduce get_crc_sources callback

[Kumar, Mahesh]

Hi,


On 7/10/2018 5:39 PM, Laurent Pinchart wrote:

Hi Mahesh,

On Tuesday, 10 July 2018 15:01:38 EEST Kumar, Mahesh wrote:

On 7/10/2018 4:52 PM, Laurent Pinchart wrote:

Hi Mahesh,
On Monday, 2 July 2018 14:07:21 EEST Mahesh Kumar wrote:

This patch introduce a callback function "get_crc_sources" which
will be called during read of control node. It is an optional
callback function and if driver implements this callback, driver
should print list of available CRC sources in seq_file privided
as an input to the callback.

The commit message seems to be outdated, the callback doesn't take a
seq_file anymore.

ops, will update.


Changes Since V1: (Daniel)

   - return const pointer to an array of crc sources list
   - do validation of sources in CRC-core

Signed-off-by: Mahesh Kumar 
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Maarten Lankhorst 
---

   drivers/gpu/drm/drm_debugfs_crc.c | 20 +++-
   include/drm/drm_crtc.h| 16 
   2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
b/drivers/gpu/drm/drm_debugfs_crc.c index c6a725b79ac6..f4d76528d24c
100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -67,9 +67,27 @@

   static int crc_control_show(struct seq_file *m, void *data)
   {
   
   	struct drm_crtc *crtc = m->private;


+   size_t count;

Count it only used within the if () {} block, you can declare it there.

agree.


+
+   if (crtc->funcs->get_crc_sources) {
+   const char *const *sources = crtc->funcs->get_crc_sources(crtc,
+   );
+   size_t values_cnt;
+   int i;

I only takes positive values, it can be an unsigned int.

ok


+
+   if (count <= 0 || !sources)

count is a size_t, it can't be negative.

The .get_crc_sources() documentation doesn't clearly specify whether
sources should always be NULL when count is zero. I advise updating the
documentation, and possibly updating this check accordingly.

ok will update.


+   goto out;
+
+   seq_puts(m, "[");
+   for (i = 0; i < count; i++)
+   if (!crtc->funcs->verify_crc_source(crtc, sources[i],
+   _cnt))

I assume that you verify sources one by one here to avoid having to create
a list of sources dynamically in the .get_crc_sources() callback ? If so,
I think the .get_crc_sources() callback should document that.

You should also document that .verify_crc_source() is required when
get_crc_sources() is provided.

ok sure.


+   seq_printf(m, "%s ", sources[i]);
+   seq_puts(m, "] ");

This assumes that source names can't include a space. Isn't that too
restrictive ? Shouldn't a different separator be used ? How about one
source name per line ?

what about comma separated as I'm putting names inside square-brackets?


Additionally, shouldn't the active source be marked ?

active source is again printed by the code in next few lines. output
will be of following format.
[space separated list of valid sources] active_source

I had missed that, my bad.

The proposed format seems a bit hackish to me, in the sense that it forbids
both spaces and brackets in source names. One source per line would fix both
and be easy to parse. We would then need to mark the active source, which
could be done by adding a marker to the corresponding line (maybe a * at the
end of the line ?).

sounds good, will do that.
-Mahesh



+   }

+out:
seq_printf(m, "%s\n", crtc->crc.source);
-
return 0;
   }

[snip]



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

Re: [PATCH 01/10] drm: crc: Introduce verify_crc_source callback

[Laurent Pinchart]

Hi Mahesh,

On Tuesday, 10 July 2018 14:54:11 EEST Kumar, Mahesh wrote:
> On 7/10/2018 4:56 PM, Laurent Pinchart wrote:
> > On Monday, 2 July 2018 14:07:20 EEST Mahesh Kumar wrote:
> >> This patch adds a new callback function "verify_crc_source" which will
> >> be used during setting the crc source in control node and while opening
> >> data node for crc reading. This will help in avoiding setting of wrong
> >> string for source.
> > 
> > Why do you need to verify the source in the open() operation ? Isn't it
> > enough to verify it in the write() handler ? The answer to this question
> > might lie in patch 08/10, in which case I'd add the .verify_crc_source()
> > call in open() in that patch, not here.
> 
> Yes, final goal is achieved by patch 08/10. But if crc_read is called
> before setting the source, it may have wrong source set in that case I
> wanted to return early at least for the drivers implemented
> verify_crc_source. If you still think otherwise I will modify
> accordingly in next series.

I don't disagree with you, but I don't think this issue is new. As I got a bit 
confused as to why the call is needed in open() (there's no explanation in 
this patch), I think fixing the problem in 08/10 would be better.

> >> Signed-off-by: Mahesh Kumar 
> >> Cc: dri-devel@lists.freedesktop.org
> >> Reviewed-by: Maarten Lankhorst 
> >> ---
> >> 
> >>   drivers/gpu/drm/drm_debugfs_crc.c | 15 +++
> >>   include/drm/drm_crtc.h| 15 +++
> >>   2 files changed, 30 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> >> b/drivers/gpu/drm/drm_debugfs_crc.c index 9f8312137cad..c6a725b79ac6
> >> 100644
> >> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> >> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> >> @@ -87,6 +87,8 @@ static ssize_t crc_control_write(struct file *file,
> >> const
> >> char __user *ubuf, struct drm_crtc *crtc = m->private;
> >> 
> >>struct drm_crtc_crc *crc = >crc;
> >>char *source;
> >> 
> >> +  size_t values_cnt;
> >> +  int ret = 0;
> >> 
> >>if (len == 0)
> >>
> >>return 0;
> >> 
> >> @@ -104,6 +106,12 @@ static ssize_t crc_control_write(struct file *file,
> >> const char __user *ubuf, if (source[len] == '\n')
> >> 
> >>source[len] = '\0';
> >> 
> >> +  if (crtc->funcs->verify_crc_source) {
> >> +  ret = crtc->funcs->verify_crc_source(crtc, source, _cnt);
> >> +  if (ret)
> >> +  return ret;
> >> +  }
> >> +
> >> 
> >>spin_lock_irq(>lock);
> >>
> >>if (crc->opened) {
> >> 
> >> @@ -167,6 +175,13 @@ static int crtc_crc_open(struct inode *inode, struct
> >> file *filep) return ret;
> >> 
> >>}
> >> 
> >> +  if (crtc->funcs->verify_crc_source) {
> >> +  ret = crtc->funcs->verify_crc_source(crtc, crc->source,
> >> +   _cnt);
> >> +  if (ret)
> >> +  return ret;
> >> +  }
> >> +
> >> 
> >>spin_lock_irq(>lock);
> >>if (!crc->opened)
> >>
> >>crc->opened = true;
> >> 
> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> >> index 23eddbccab10..1a6dcbf91744 100644
> >> --- a/include/drm/drm_crtc.h
> >> +++ b/include/drm/drm_crtc.h
> >> @@ -661,6 +661,21 @@ struct drm_crtc_funcs {
> >> 
> >> */
> >>
> >>int (*set_crc_source)(struct drm_crtc *crtc, const char *source,
> >>
> >>  size_t *values_cnt);
> >> 
> >> +  /**
> >> +   * @verify_crc_source:
> >> +   *
> >> +   * verifies the source of CRC checksums of frames before setting the
> >> +   * source for CRC and during crc open.
> >> +   *
> >> +   * This callback is optional if the driver does not support any CRC
> >> +   * generation functionality.
> >> +   *
> >> +   * RETURNS:
> >> +   *
> >> +   * 0 on success or a negative error code on failure.
> >> +   */
> >> +  int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
> >> +   size_t *values_cnt);
> >> 
> >>/**
> >>
> >> * @atomic_print_state:


-- 
Regards,

Laurent Pinchart



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

Re: [PATCH 02/10] drm: crc: Introduce get_crc_sources callback

[Laurent Pinchart]

Hi Mahesh,

On Tuesday, 10 July 2018 15:01:38 EEST Kumar, Mahesh wrote:
> On 7/10/2018 4:52 PM, Laurent Pinchart wrote:
> > Hi Mahesh,
> > On Monday, 2 July 2018 14:07:21 EEST Mahesh Kumar wrote:
> >> This patch introduce a callback function "get_crc_sources" which
> >> will be called during read of control node. It is an optional
> >> callback function and if driver implements this callback, driver
> >> should print list of available CRC sources in seq_file privided
> >> as an input to the callback.
> > 
> > The commit message seems to be outdated, the callback doesn't take a
> > seq_file anymore.
> 
> ops, will update.
> 
> >> Changes Since V1: (Daniel)
> >> 
> >>   - return const pointer to an array of crc sources list
> >>   - do validation of sources in CRC-core
> >> 
> >> Signed-off-by: Mahesh Kumar 
> >> Cc: dri-devel@lists.freedesktop.org
> >> Reviewed-by: Maarten Lankhorst 
> >> ---
> >> 
> >>   drivers/gpu/drm/drm_debugfs_crc.c | 20 +++-
> >>   include/drm/drm_crtc.h| 16 
> >>   2 files changed, 35 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> >> b/drivers/gpu/drm/drm_debugfs_crc.c index c6a725b79ac6..f4d76528d24c
> >> 100644
> >> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> >> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> >> @@ -67,9 +67,27 @@
> >> 
> >>   static int crc_control_show(struct seq_file *m, void *data)
> >>   {
> >>   
> >>struct drm_crtc *crtc = m->private;
> >> 
> >> +  size_t count;
> > 
> > Count it only used within the if () {} block, you can declare it there.
> 
> agree.
> 
> >> +
> >> +  if (crtc->funcs->get_crc_sources) {
> >> +  const char *const *sources = crtc->funcs->get_crc_sources(crtc,
> >> +  );
> >> +  size_t values_cnt;
> >> +  int i;
> > 
> > I only takes positive values, it can be an unsigned int.
> 
> ok
> 
> >> +
> >> +  if (count <= 0 || !sources)
> > 
> > count is a size_t, it can't be negative.
> > 
> > The .get_crc_sources() documentation doesn't clearly specify whether
> > sources should always be NULL when count is zero. I advise updating the
> > documentation, and possibly updating this check accordingly.
> 
> ok will update.
> 
> >> +  goto out;
> >> +
> >> +  seq_puts(m, "[");
> >> +  for (i = 0; i < count; i++)
> >> +  if (!crtc->funcs->verify_crc_source(crtc, sources[i],
> >> +  _cnt))
> > 
> > I assume that you verify sources one by one here to avoid having to create
> > a list of sources dynamically in the .get_crc_sources() callback ? If so,
> > I think the .get_crc_sources() callback should document that.
> > 
> > You should also document that .verify_crc_source() is required when
> > get_crc_sources() is provided.
> 
> ok sure.
> 
> >> +  seq_printf(m, "%s ", sources[i]);
> >> +  seq_puts(m, "] ");
> > 
> > This assumes that source names can't include a space. Isn't that too
> > restrictive ? Shouldn't a different separator be used ? How about one
> > source name per line ?
> 
> what about comma separated as I'm putting names inside square-brackets?
> 
> > Additionally, shouldn't the active source be marked ?
> 
> active source is again printed by the code in next few lines. output
> will be of following format.
> [space separated list of valid sources] active_source

I had missed that, my bad.

The proposed format seems a bit hackish to me, in the sense that it forbids 
both spaces and brackets in source names. One source per line would fix both 
and be easy to parse. We would then need to mark the active source, which 
could be done by adding a marker to the corresponding line (maybe a * at the 
end of the line ?).

> >> +  }
> >> 
> >> +out:
> >>seq_printf(m, "%s\n", crtc->crc.source);
> >> -
> >>return 0;
> >>   }

[snip]

-- 
Regards,

Laurent Pinchart



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

Re: [PATCH 10/10] drm: crc: Introduce pre_crc_read function

[Laurent Pinchart]

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:29 EEST Mahesh Kumar wrote:
> This patch implements a callback function "pre_crc_read" which will
> be called before crc read. In this function driver can implement and
> preparation work required for successfully reading CRC data.

Reviewing this is difficult with a user. Could you submit a patch that makes 
use of this callback in a driver ?

> Signed-off-by: Mahesh Kumar 
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_debugfs_crc.c |  8 
>  include/drm/drm_crtc.h| 14 ++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> b/drivers/gpu/drm/drm_debugfs_crc.c index add35b77165b..7aeed89f934a 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -272,6 +272,14 @@ static ssize_t crtc_crc_read(struct file *filep, char
> __user *user_buf, return 0;
>   }
> 
> + if (crtc->funcs->pre_crc_read) {
> + ret = crtc->funcs->pre_crc_read(crtc);
> + if (ret) {
> + spin_unlock_irq(>lock);
> + return ret;
> + }
> + }
> +
>   /* Nothing to read? */
>   while (crtc_crc_data_count(crc) == 0) {
>   if (filep->f_flags & O_NONBLOCK) {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 81780325f36f..7e2eab9c2f52 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -691,6 +691,20 @@ struct drm_crtc_funcs {
>*/
>   const char *const *(*get_crc_sources)(struct drm_crtc *crtc,
> size_t *count);
> + /**
> +  * @pre_crc_read:
> +  *
> +  * Driver callback for performing any preparation work required by
> +  * driver before reading CRC
> +  *
> +  * This callback is optional if the driver does not support CRC
> +  * generation or no prework is required before reading the crc
> +  *
> +  * RETURNS:
> +  *
> +  * 0 on success or a negative error code on failure.
> +  */
> + int (*pre_crc_read)(struct drm_crtc *crtc);
> 
>   /**
>* @atomic_print_state:

-- 
Regards,

Laurent Pinchart



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

Re: [PATCH 09/10] Revert "drm: crc: Wait for a frame before returning from open()"

[Laurent Pinchart]

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:28 EEST Mahesh Kumar wrote:
> This reverts commit e8fa5671183c80342d520ad81d14fa79a9d4a680.
> 
> Don't wait for first CRC during crtc_crc_open. It avoids one frame wait
> during open. If application want to wait after read call, it can use
> poll/read blocking read() call.
> 
> Suggested-by: Ville Syrjälä 
> Signed-off-by: Mahesh Kumar 
> Cc: dri-devel@lists.freedesktop.org
> Cc: Tomeu Vizoso 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/drm_debugfs_crc.c | 16 
>  1 file changed, 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> b/drivers/gpu/drm/drm_debugfs_crc.c index 739a813b4b09..add35b77165b 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -226,24 +226,8 @@ static int crtc_crc_open(struct inode *inode, struct
> file *filep) if (ret)
>   goto err;
> 
> - spin_lock_irq(>lock);
> - /*
> -  * Only return once we got a first frame, so userspace doesn't have to
> -  * guess when this particular piece of HW will be ready to start
> -  * generating CRCs.
> -  */
> - ret = wait_event_interruptible_lock_irq(crc->wq,
> - crtc_crc_data_count(crc),
> - crc->lock);
> - spin_unlock_irq(>lock);
> -
> - if (ret)
> - goto err_disable;
> -
>   return 0;
> 
> -err_disable:
> - crtc->funcs->set_crc_source(crtc, NULL);
>  err:
>   spin_lock_irq(>lock);
>   crtc_crc_cleanup(crc);


-- 
Regards,

Laurent Pinchart



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

Re: [PATCH 02/10] drm: crc: Introduce get_crc_sources callback

[Kumar, Mahesh]

Hi,

thanks for the review.
On 7/10/2018 4:52 PM, Laurent Pinchart wrote:

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:21 EEST Mahesh Kumar wrote:

This patch introduce a callback function "get_crc_sources" which
will be called during read of control node. It is an optional
callback function and if driver implements this callback, driver
should print list of available CRC sources in seq_file privided
as an input to the callback.

The commit message seems to be outdated, the callback doesn't take a seq_file
anymore.

ops, will update.



Changes Since V1: (Daniel)
  - return const pointer to an array of crc sources list
  - do validation of sources in CRC-core

Signed-off-by: Mahesh Kumar 
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Maarten Lankhorst 
---
  drivers/gpu/drm/drm_debugfs_crc.c | 20 +++-
  include/drm/drm_crtc.h| 16 
  2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
b/drivers/gpu/drm/drm_debugfs_crc.c index c6a725b79ac6..f4d76528d24c 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -67,9 +67,27 @@
  static int crc_control_show(struct seq_file *m, void *data)
  {
struct drm_crtc *crtc = m->private;
+   size_t count;

Count it only used within the if () {} block, you can declare it there.

agree.



+
+   if (crtc->funcs->get_crc_sources) {
+   const char *const *sources = crtc->funcs->get_crc_sources(crtc,
+   );
+   size_t values_cnt;
+   int i;

I only takes positive values, it can be an unsigned int.

ok



+
+   if (count <= 0 || !sources)

count is a size_t, it can't be negative.

The .get_crc_sources() documentation doesn't clearly specify whether sources
should always be NULL when count is zero. I advise updating the documentation,
and possibly updating this check accordingly.

ok will update.



+   goto out;
+
+   seq_puts(m, "[");
+   for (i = 0; i < count; i++)
+   if (!crtc->funcs->verify_crc_source(crtc, sources[i],
+   _cnt))

I assume that you verify sources one by one here to avoid having to create a
list of sources dynamically in the .get_crc_sources() callback ? If so, I
think the .get_crc_sources() callback should document that.

You should also document that .verify_crc_source() is required when
get_crc_sources() is provided.

ok sure.



+   seq_printf(m, "%s ", sources[i]);
+   seq_puts(m, "] ");

This assumes that source names can't include a space. Isn't that too
restrictive ? Shouldn't a different separator be used ? How about one source
name per line ?

what about comma separated as I'm putting names inside square-brackets?


Additionally, shouldn't the active source be marked ?
active source is again printed by the code in next few lines. output 
will be of following format.

[space separated list of valid sources] active_source

-Mahesh



+   }

+out:
seq_printf(m, "%s\n", crtc->crc.source);
-
return 0;
  }

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 1a6dcbf91744..ffaec138aeee 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -676,6 +676,22 @@ struct drm_crtc_funcs {
 */
int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
 size_t *values_cnt);
+   /**
+* @get_crc_sources:
+*
+* Driver callback for getting a list of all the available sources for
+* CRC generation.
+*
+* This callback is optional if the driver does not support exporting of
+* possible CRC sources list. CRC-core does the verification of sources.
+*
+* RETURNS:
+*
+* a constant character pointer to the list of all the available CRC
+* sources
+*/
+   const char *const *(*get_crc_sources)(struct drm_crtc *crtc,
+ size_t *count);

/**
 * @atomic_print_state:


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

Re: [PATCH 01/10] drm: crc: Introduce verify_crc_source callback

[Kumar, Mahesh]

Hi,

Thanks for the review,
On 7/10/2018 4:56 PM, Laurent Pinchart wrote:

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:20 EEST Mahesh Kumar wrote:

This patch adds a new callback function "verify_crc_source" which will
be used during setting the crc source in control node and while opening
data node for crc reading. This will help in avoiding setting of wrong
string for source.

Why do you need to verify the source in the open() operation ? Isn't it enough
to verify it in the write() handler ? The answer to this question might lie in
patch 08/10, in which case I'd add the .verify_crc_source() call in open() in
that patch, not here.
Yes, final goal is achieved by patch 08/10. But if crc_read is called 
before setting the source, it may have wrong source set in that case I 
wanted to return early at least for the drivers implemented 
verify_crc_source. If you still think otherwise I will modify 
accordingly in next series.


-Mahesh




Signed-off-by: Mahesh Kumar 
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Maarten Lankhorst 
---
  drivers/gpu/drm/drm_debugfs_crc.c | 15 +++
  include/drm/drm_crtc.h| 15 +++
  2 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
b/drivers/gpu/drm/drm_debugfs_crc.c index 9f8312137cad..c6a725b79ac6 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -87,6 +87,8 @@ static ssize_t crc_control_write(struct file *file, const
char __user *ubuf, struct drm_crtc *crtc = m->private;
struct drm_crtc_crc *crc = >crc;
char *source;
+   size_t values_cnt;
+   int ret = 0;

if (len == 0)
return 0;
@@ -104,6 +106,12 @@ static ssize_t crc_control_write(struct file *file,
const char __user *ubuf, if (source[len] == '\n')
source[len] = '\0';

+   if (crtc->funcs->verify_crc_source) {
+   ret = crtc->funcs->verify_crc_source(crtc, source, _cnt);
+   if (ret)
+   return ret;
+   }
+
spin_lock_irq(>lock);

if (crc->opened) {
@@ -167,6 +175,13 @@ static int crtc_crc_open(struct inode *inode, struct
file *filep) return ret;
}

+   if (crtc->funcs->verify_crc_source) {
+   ret = crtc->funcs->verify_crc_source(crtc, crc->source,
+_cnt);
+   if (ret)
+   return ret;
+   }
+
spin_lock_irq(>lock);
if (!crc->opened)
crc->opened = true;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 23eddbccab10..1a6dcbf91744 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -661,6 +661,21 @@ struct drm_crtc_funcs {
 */
int (*set_crc_source)(struct drm_crtc *crtc, const char *source,
  size_t *values_cnt);
+   /**
+* @verify_crc_source:
+*
+* verifies the source of CRC checksums of frames before setting the
+* source for CRC and during crc open.
+*
+* This callback is optional if the driver does not support any CRC
+* generation functionality.
+*
+* RETURNS:
+*
+* 0 on success or a negative error code on failure.
+*/
+   int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
+size_t *values_cnt);

/**
 * @atomic_print_state:




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

Re: [PATCH 08/10] drm/crc: Cleanup crtc_crc_open function

[Laurent Pinchart]

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:27 EEST Mahesh Kumar wrote:
> This patch make changes to allocate crc-entries buffer before
> enabling CRC generation.
> It moves all the failure check early in the function before setting
> the source or memory allocation.
> Now set_crc_source takes only two variable inputs, values_cnt we
> already gets as part of verify_crc_source.
> 
> Signed-off-by: Mahesh Kumar 
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  3 +-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c  |  4 +-
>  drivers/gpu/drm/drm_debugfs_crc.c  | 52 ---
>  drivers/gpu/drm/i915/intel_drv.h   |  3 +-
>  drivers/gpu/drm/i915/intel_pipe_crc.c  |  4 +-
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c |  5 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c|  6 +--
>  include/drm/drm_crtc.h |  3 +-
>  8 files changed, 30 insertions(+), 50 deletions(-)

[snip]

> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> b/drivers/gpu/drm/drm_debugfs_crc.c index f4d76528d24c..739a813b4b09 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -124,11 +124,9 @@ static ssize_t crc_control_write(struct file *file,
> const char __user *ubuf, if (source[len] == '\n')
>   source[len] = '\0';
> 
> - if (crtc->funcs->verify_crc_source) {
> - ret = crtc->funcs->verify_crc_source(crtc, source, _cnt);
> - if (ret)
> - return ret;
> - }
> + ret = crtc->funcs->verify_crc_source(crtc, source, _cnt);
> + if (ret)
> + return ret;
> 
>   spin_lock_irq(>lock);
> 
> @@ -193,12 +191,15 @@ static int crtc_crc_open(struct inode *inode, struct
> file *filep) return ret;
>   }
> 
> - if (crtc->funcs->verify_crc_source) {
> - ret = crtc->funcs->verify_crc_source(crtc, crc->source,
> -  _cnt);
> - if (ret)
> - return ret;
> - }
> + ret = crtc->funcs->verify_crc_source(crtc, crc->source, _cnt);
> + if (ret)
> + return ret;
> +
> + if (WARN_ON(values_cnt > DRM_MAX_CRC_NR))
> + return -EINVAL;
> +
> + if (WARN_ON(values_cnt == 0))
> + return -EINVAL;
> 
>   spin_lock_irq(>lock);
>   if (!crc->opened)
> @@ -210,30 +211,22 @@ static int crtc_crc_open(struct inode *inode, struct
> file *filep) if (ret)
>   return ret;
> 
> - ret = crtc->funcs->set_crc_source(crtc, crc->source, _cnt);
> - if (ret)
> - goto err;
> -
> - if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) {
> - ret = -EINVAL;
> - goto err_disable;
> - }
> -
> - if (WARN_ON(values_cnt == 0)) {
> - ret = -EINVAL;
> - goto err_disable;
> - }
> -
>   entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL);
>   if (!entries) {
>   ret = -ENOMEM;
> - goto err_disable;
> + goto err;
>   }

If you moved allocation before the !crc->opened check, you could group the two 
code blocks protected by the crc->lock.

>   spin_lock_irq(>lock);
>   crc->entries = entries;
>   crc->values_cnt = values_cnt;
> + spin_unlock_irq(>lock);
> 
> + ret = crtc->funcs->set_crc_source(crtc, crc->source);
> + if (ret)
> + goto err;
> +
> + spin_lock_irq(>lock);
>   /*
>* Only return once we got a first frame, so userspace doesn't have to
>* guess when this particular piece of HW will be ready to start
> @@ -250,7 +243,7 @@ static int crtc_crc_open(struct inode *inode, struct
> file *filep) return 0;
> 
>  err_disable:
> - crtc->funcs->set_crc_source(crtc, NULL, _cnt);
> + crtc->funcs->set_crc_source(crtc, NULL);
>  err:
>   spin_lock_irq(>lock);
>   crtc_crc_cleanup(crc);
> @@ -262,9 +255,8 @@ static int crtc_crc_release(struct inode *inode, struct
> file *filep) {
>   struct drm_crtc *crtc = filep->f_inode->i_private;
>   struct drm_crtc_crc *crc = >crc;
> - size_t values_cnt;
> 
> - crtc->funcs->set_crc_source(crtc, NULL, _cnt);
> + crtc->funcs->set_crc_source(crtc, NULL);
> 
>   spin_lock_irq(>lock);
>   crtc_crc_cleanup(crc);
> @@ -370,7 +362,7 @@ int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc)
>  {
>   struct dentry *crc_ent, *ent;
> 
> - if (!crtc->funcs->set_crc_source)
> + if (!crtc->funcs->set_crc_source || !crtc->funcs->verify_crc_source)
>   return 0;
> 
>   crc_ent = debugfs_create_dir("crc", crtc->debugfs_entry);

[snip]

-- 
Regards,

Laurent Pinchart



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

Re: [PATCH 05/10] drm/rcar-du/crc: Implement verify_crc_source callback

[Laurent Pinchart]

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:24 EEST Mahesh Kumar wrote:
> This patch implements "verify_crc_source" callback function for
> rcar drm driver.
> 
> Signed-off-by: Mahesh Kumar 
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 40 +++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 15dc9caa128b..24eeaa7e14d7
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -756,6 +756,45 @@ static void rcar_du_crtc_disable_vblank(struct drm_crtc
> *crtc) rcrtc->vblank_enable = false;
>  }
> 
> +static int rcar_du_crtc_verify_crc_source(struct drm_crtc *crtc,
> +   const char *source_name,
> +   size_t *values_cnt)
> +{
> + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> + unsigned int index = 0;
> + unsigned int i;
> + int ret;
> +
> + /*
> +  * Parse the source name. Supported values are "plane%u" to compute the
> +  * CRC on an input plane (%u is the plane ID), and "auto" to compute the
> +  * CRC on the composer (VSP) output.
> +  */
> + if (!source_name || !strcmp(source_name, "auto")) {
> + goto out;
> + } else if (strstarts(source_name, "plane")) {
> + ret = kstrtouint(source_name + strlen("plane"), 10, );
> + if (ret < 0)
> + return ret;
> +
> + for (i = 0; i < rcrtc->vsp->num_planes; ++i) {
> + if (index == rcrtc->vsp->planes[i].plane.base.id) {
> + index = i;
> + break;
> + }
> + }
> +
> + if (i >= rcrtc->vsp->num_planes)
> + return -EINVAL;
> + } else {
> + return -EINVAL;
> + }
> +
> +out:
> + *values_cnt = 1;
> + return 0;

This duplicates lots of code from the rcar_du_crtc_set_crc_source() function. 
Could you please extract it to a shared function ?

Could you please also implement support for the .get_crc_sources() operation ? 
I think doing so might show limitations in the current API, namely the fact 
that the list will need to be dynamically created for this driver.

> +}
> +
>  static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc,
>  const char *source_name,
>  size_t *values_cnt)
> @@ -861,6 +900,7 @@ static const struct drm_crtc_funcs crtc_funcs_gen3 = {
>   .enable_vblank = rcar_du_crtc_enable_vblank,
>   .disable_vblank = rcar_du_crtc_disable_vblank,
>   .set_crc_source = rcar_du_crtc_set_crc_source,
> + .verify_crc_source = rcar_du_crtc_verify_crc_source,
>  };
> 
>  /* 

-- 
Regards,

Laurent Pinchart



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

Re: [PATCH 01/10] drm: crc: Introduce verify_crc_source callback

[Laurent Pinchart]

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:20 EEST Mahesh Kumar wrote:
> This patch adds a new callback function "verify_crc_source" which will
> be used during setting the crc source in control node and while opening
> data node for crc reading. This will help in avoiding setting of wrong
> string for source.

Why do you need to verify the source in the open() operation ? Isn't it enough 
to verify it in the write() handler ? The answer to this question might lie in 
patch 08/10, in which case I'd add the .verify_crc_source() call in open() in 
that patch, not here.

> Signed-off-by: Mahesh Kumar 
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/drm_debugfs_crc.c | 15 +++
>  include/drm/drm_crtc.h| 15 +++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> b/drivers/gpu/drm/drm_debugfs_crc.c index 9f8312137cad..c6a725b79ac6 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -87,6 +87,8 @@ static ssize_t crc_control_write(struct file *file, const
> char __user *ubuf, struct drm_crtc *crtc = m->private;
>   struct drm_crtc_crc *crc = >crc;
>   char *source;
> + size_t values_cnt;
> + int ret = 0;
> 
>   if (len == 0)
>   return 0;
> @@ -104,6 +106,12 @@ static ssize_t crc_control_write(struct file *file,
> const char __user *ubuf, if (source[len] == '\n')
>   source[len] = '\0';
> 
> + if (crtc->funcs->verify_crc_source) {
> + ret = crtc->funcs->verify_crc_source(crtc, source, _cnt);
> + if (ret)
> + return ret;
> + }
> +
>   spin_lock_irq(>lock);
> 
>   if (crc->opened) {
> @@ -167,6 +175,13 @@ static int crtc_crc_open(struct inode *inode, struct
> file *filep) return ret;
>   }
> 
> + if (crtc->funcs->verify_crc_source) {
> + ret = crtc->funcs->verify_crc_source(crtc, crc->source,
> +  _cnt);
> + if (ret)
> + return ret;
> + }
> +
>   spin_lock_irq(>lock);
>   if (!crc->opened)
>   crc->opened = true;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 23eddbccab10..1a6dcbf91744 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -661,6 +661,21 @@ struct drm_crtc_funcs {
>*/
>   int (*set_crc_source)(struct drm_crtc *crtc, const char *source,
> size_t *values_cnt);
> + /**
> +  * @verify_crc_source:
> +  *
> +  * verifies the source of CRC checksums of frames before setting the
> +  * source for CRC and during crc open.
> +  *
> +  * This callback is optional if the driver does not support any CRC
> +  * generation functionality.
> +  *
> +  * RETURNS:
> +  *
> +  * 0 on success or a negative error code on failure.
> +  */
> + int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
> +  size_t *values_cnt);
> 
>   /**
>* @atomic_print_state:


-- 
Regards,

Laurent Pinchart



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

Re: [PATCH 02/10] drm: crc: Introduce get_crc_sources callback

[Laurent Pinchart]

Hi Mahesh,

Thank you for the patch.

On Monday, 2 July 2018 14:07:21 EEST Mahesh Kumar wrote:
> This patch introduce a callback function "get_crc_sources" which
> will be called during read of control node. It is an optional
> callback function and if driver implements this callback, driver
> should print list of available CRC sources in seq_file privided
> as an input to the callback.

The commit message seems to be outdated, the callback doesn't take a seq_file 
anymore.

> Changes Since V1: (Daniel)
>  - return const pointer to an array of crc sources list
>  - do validation of sources in CRC-core
> 
> Signed-off-by: Mahesh Kumar 
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Maarten Lankhorst 
> ---
>  drivers/gpu/drm/drm_debugfs_crc.c | 20 +++-
>  include/drm/drm_crtc.h| 16 
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> b/drivers/gpu/drm/drm_debugfs_crc.c index c6a725b79ac6..f4d76528d24c 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -67,9 +67,27 @@
>  static int crc_control_show(struct seq_file *m, void *data)
>  {
>   struct drm_crtc *crtc = m->private;
> + size_t count;

Count it only used within the if () {} block, you can declare it there.

> +
> + if (crtc->funcs->get_crc_sources) {
> + const char *const *sources = crtc->funcs->get_crc_sources(crtc,
> + );
> + size_t values_cnt;
> + int i;

I only takes positive values, it can be an unsigned int.

> +
> + if (count <= 0 || !sources)

count is a size_t, it can't be negative.

The .get_crc_sources() documentation doesn't clearly specify whether sources 
should always be NULL when count is zero. I advise updating the documentation, 
and possibly updating this check accordingly.

> + goto out;
> +
> + seq_puts(m, "[");
> + for (i = 0; i < count; i++)
> + if (!crtc->funcs->verify_crc_source(crtc, sources[i],
> + _cnt))

I assume that you verify sources one by one here to avoid having to create a 
list of sources dynamically in the .get_crc_sources() callback ? If so, I 
think the .get_crc_sources() callback should document that.

You should also document that .verify_crc_source() is required when 
get_crc_sources() is provided.

> + seq_printf(m, "%s ", sources[i]);
> + seq_puts(m, "] ");

This assumes that source names can't include a space. Isn't that too 
restrictive ? Shouldn't a different separator be used ? How about one source 
name per line ?

Additionally, shouldn't the active source be marked ?

> + }
> 
> +out:
>   seq_printf(m, "%s\n", crtc->crc.source);
> -
>   return 0;
>  }
> 
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 1a6dcbf91744..ffaec138aeee 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -676,6 +676,22 @@ struct drm_crtc_funcs {
>*/
>   int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
>size_t *values_cnt);
> + /**
> +  * @get_crc_sources:
> +  *
> +  * Driver callback for getting a list of all the available sources for
> +  * CRC generation.
> +  *
> +  * This callback is optional if the driver does not support exporting of
> +  * possible CRC sources list. CRC-core does the verification of sources.
> +  *
> +  * RETURNS:
> +  *
> +  * a constant character pointer to the list of all the available CRC
> +  * sources
> +  */
> + const char *const *(*get_crc_sources)(struct drm_crtc *crtc,
> +   size_t *count);
> 
>   /**
>* @atomic_print_state:

-- 
Regards,

Laurent Pinchart



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

Re: 答复: 答复: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Takashi Iwai]

On Tue, 10 Jul 2018 13:11:20 +0200,
jimqu wrote:
> 
> 
> 
> On 2018年07月10日 17:50, Takashi Iwai wrote:
> > On Tue, 10 Jul 2018 11:13:27 +0200,
> > jimqu wrote:
> >>
> >>
> >> On 2018年07月10日 16:01, Takashi Iwai wrote:
> >>> On Tue, 10 Jul 2018 09:44:42 +0200,
> >>> Qu, Jim wrote:
>  Hi Takashi,
> 
>  I am not expert in audio driver, so I just update some test result, 
>  please help triage the issue.
> 
>  With audio runtime pm:
> >>> What does this mean?  Is it the stock 4.17.x (or 4.18-rc)?
> >>> Please clarify your test environment at first: which kernel, which
> >>> hardware setup.
> >>>
> >> This is v4.15 which backport Lukas' patches and also one bug fix
> >> https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=57cb54e53bdd.
> >> The platform is AMDAPU(with audio) + AMD dGPU , it is a hybird GFX
> >> platform. In generic, dGPU will always be suspended.
> > Ah, so it's AMD+AMD, not Intel+AMD combo.  Now I understand the
> > situation better, thanks.
> >
>  1. ubuntu audio setting use pactl to query audio card/output sink. 
>  Attachment is pactl output with audio runtime pm.
>  2. cat /proc/asound/card0/eld* can get nothing.
> 
>  Without audio runtime pm:
>  1. pactl can get available audio output/sink
>  2. can get eld info from eld#0.1
> 
>  IMO, the issue should be:
> 
>  Current operations like HDMI hotplug in/out, pactl command, they do
>  not access audio HW, so the audio could not resume back at this
>  period.
> >>> Sorry, not understood.  Why they don't access the audio hardware?
> >> This is my guess, since I do not get azx_runtime_resume() print info
> >> which I added. it maybe access HW during this period, but do not
> >> trigger audio resume progress.
> >> 
>  Therefore, upper software could not get HDMI ELD info, can select a 
>  available card/output sink.
>  I am also confuse that if there is no ELD info, how driver to steam 
>  audio device to wake up itself? It's sort of a chicken-or-egg question.
> >>> As long as it's i915 and HD-audio, the hotplug and ELD info transfer
> >>> doesn't trigger the runtime PM since it's done via the direct
> >>> callback.  And ELD value is cached in HD-audio side.
> >> Yeah, I have reviewed these code, it constructs ELD after reading
> >> EDID, and write them into HW buffer when set display mode.
> > If the controller chip supports "wake-enable" feature (HD-audio WAKEEN
> > register), it could be woken up at unsolicited event even during the
> > link power down.  But, your report implies that AMD controller doesn't
> > do this, or something missing there.  That's the likely cause of the
> > missing hotplug event.
> 
> Is there any method to confirm it?

The driver always sets WAKEEN bit, see azx_runtime_suspend() in
hda_intel.c.  If it still doesn't behave as expected, it means that
the feature isn't supported on that chip :)

> > So, if my understanding is right, the situation won't be different
> > even if you have a single AMD GPU.  And possibly a side-effect of the
> > latest fix to force link-down for AMD GPU.  Need to check it...
> 
> Before Lukas' patches, it is relative with dGPU, because audio power
> is controlled by vgaswitchreroo and GFX driver, but now it won't.

Right, and this is the correct behavior.

> revert the fix of amdgpu suspend issue, audio issue also can be observed.

Did you check the behavior with the single AMD GPU hardware?
If confirmed, we can forget about vga_switcheroo.


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

Re: [PATCH] drm/panel: type promotion bug in s6e8aa0_read_mtp_id()

[Thierry Reding]

On Wed, Jul 04, 2018 at 12:38:09PM +0300, Dan Carpenter wrote:
> The ARRAY_SIZE() macro is type size_t.  If s6e8aa0_dcs_read() returns a
> negative error code, then "ret < ARRAY_SIZE(id)" is false because the
> negative error code is type promoted to a high positive value.
> 
> Fixes: 02051ca06371 ("drm/panel: add S6E8AA0 driver")
> Signed-off-by: Dan Carpenter 

Applied, thanks.

Thierry


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

Re: 答复: 答复: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[jimqu]

On 2018年07月10日 17:50, Takashi Iwai wrote:

On Tue, 10 Jul 2018 11:13:27 +0200,
jimqu wrote:



On 2018年07月10日 16:01, Takashi Iwai wrote:

On Tue, 10 Jul 2018 09:44:42 +0200,
Qu, Jim wrote:

Hi Takashi,

I am not expert in audio driver, so I just update some test result, please help 
triage the issue.

With audio runtime pm:

What does this mean?  Is it the stock 4.17.x (or 4.18-rc)?
Please clarify your test environment at first: which kernel, which
hardware setup.


This is v4.15 which backport Lukas' patches and also one bug fix
https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=57cb54e53bdd.
The platform is AMDAPU(with audio) + AMD dGPU , it is a hybird GFX
platform. In generic, dGPU will always be suspended.

Ah, so it's AMD+AMD, not Intel+AMD combo.  Now I understand the
situation better, thanks.


1. ubuntu audio setting use pactl to query audio card/output sink. Attachment 
is pactl output with audio runtime pm.
2. cat /proc/asound/card0/eld* can get nothing.

Without audio runtime pm:
1. pactl can get available audio output/sink
2. can get eld info from eld#0.1

IMO, the issue should be:

Current operations like HDMI hotplug in/out, pactl command, they do
not access audio HW, so the audio could not resume back at this
period.

Sorry, not understood.  Why they don't access the audio hardware?

This is my guess, since I do not get azx_runtime_resume() print info
which I added. it maybe access HW during this period, but do not
trigger audio resume progress.


Therefore, upper software could not get HDMI ELD info, can select a available 
card/output sink.
I am also confuse that if there is no ELD info, how driver to steam audio 
device to wake up itself? It's sort of a chicken-or-egg question.

As long as it's i915 and HD-audio, the hotplug and ELD info transfer
doesn't trigger the runtime PM since it's done via the direct
callback.  And ELD value is cached in HD-audio side.

Yeah, I have reviewed these code, it constructs ELD after reading
EDID, and write them into HW buffer when set display mode.

If the controller chip supports "wake-enable" feature (HD-audio WAKEEN
register), it could be woken up at unsolicited event even during the
link power down.  But, your report implies that AMD controller doesn't
do this, or something missing there.  That's the likely cause of the
missing hotplug event.


Is there any method to confirm it?

So, if my understanding is right, the situation won't be different
even if you have a single AMD GPU.  And possibly a side-effect of the
latest fix to force link-down for AMD GPU.  Need to check it...


Before Lukas' patches, it is relative with dGPU, because audio power is 
controlled by vgaswitchreroo and GFX driver, but now it won't.

revert the fix of amdgpu suspend issue, audio issue also can be observed.


Takashi


The exception is that if the notification is done during the PM
operation.  But, the connection and ELD query is performed at the end
of codec resume, hence it'll be covered there.

So in theory, ELD information should be passed from the GPU to
HD-audio no matter whether runtime PM is enabled or not.


BTW, please stop top-posting.  It makes the thread readability awfully
worse.


Sorry, I justed used outlook client.

thanks,

Takashi


Thanks
JimQu


发件人: Takashi Iwai 
发送时间: 2018年7月10日 1:09
收件人: Qu, Jim
抄送: Daniel Vetter; Alex Deucher; alsa-de...@alsa-project.org; 
amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Deucher, 
Alexander
主题: Re: 答复: [alsa-devel]答复: [PATCH] vgaswitchroo: set audio client id 
according to bound gpu client id

On Mon, 09 Jul 2018 18:05:09 +0200,
Qu, Jim wrote:

Hi Takashi,

Not intel, but it is AMD APU+ AMD GFX, the APU has a local HDMI port for 
extension. And dGPU is only for offloading render via PRIME.

Originally, the HDA driver before v4.17, there is a bug, that all the audio is 
set to CLIENT_DIS, so the when the dGPU suspend, the audio which is bound to 
iGPU will also be suspend.

Now, after Lukas' patches. The audio will auto suspend. It make ubuntu audio 
setting could not detect HDMI audio, even if HDMI has light up.

OK, and it has all necessary patches including the one Lukas
suggested, right?

If so, figure out what you're actually seeing as "PA could not detect
HDMI audio".  For example, is it the HDMI (jack) detection (giving
false even if it's connected)?  Or is it an error at opening the given
device? Does the driver give the proper ELD bytes as well as the jack
state?


Takashi


Thanks
JimQu

-邮件原件-
发件人: Takashi Iwai 
发送时间: 2018年7月9日 23:58
收件人: Daniel Vetter 
抄送: Alex Deucher ; alsa-de...@alsa-project.org; 
amd-...@lists.freedesktop.org; Qu, Jim ; dri-devel@lists.freedesktop.org; 
Deucher, Alexander 
主题: Re: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to 
bound gpu client id

On Mon, 09 Jul 2018 17:56:43 +0200,
Daniel 

Re: [PATCH v2 3/3] drm/panel: add Kingdisplay kd097d04 panel driver

[Thierry Reding]

On Mon, Jul 02, 2018 at 12:32:29PM +0200, Heiko Stuebner wrote:
> From: Nickey Yang 
> 
> Support Kingdisplay kd097d04 9.7" 1536x2048 TFT LCD panel,
> it is a MIPI dual-DSI panel.
> 
> v2:
> - update timing + cmds from chromeos kernel
> - new backlight API including switch to devm_of_find_backlight
> - fix most of Sean Paul's comments
>   enable/prepare tracking seems something all panels do
> - document origins of the init sequence
> - lanes per dsi interface to 4 (two interfaces). Matches how tegra
>   and pending rockchip dual-dsi handle (dual-)dsi lanes
> - spdx header instead of license boilerplate
> 
> Signed-off-by: Nickey Yang 
> Signed-off-by: Heiko Stuebner 
> ---
>  drivers/gpu/drm/panel/Kconfig |  11 +
>  drivers/gpu/drm/panel/Makefile|   1 +
>  .../drm/panel/panel-kingdisplay-kd097d04.c| 469 ++
>  3 files changed, 481 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 25682ff3449a..9a0061f7fed7 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -68,6 +68,17 @@ config DRM_PANEL_JDI_LT070ME05000
> The panel has a 1200(RGB)×1920 (WUXGA) resolution and uses
> 24 bit per pixel.
>  
> +config DRM_PANEL_KINGDISPLAY_KD097D04
> + tristate "Kingdisplay kd097d04 panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> +   Say Y here if you want to enable support for Kingdisplay kd097d04
> +   TFT-LCD modules. The panel has a 1536x2048 resolution and uses
> +   24 bit RGB per pixel. It provides a MIPI DSI interface to
> +   the host and has a built-in LED backlight.
> +
>  config DRM_PANEL_SAMSUNG_LD9040
>   tristate "Samsung LD9040 RGB/SPI panel"
>   depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index f26efc11d746..314b272f1e14 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
>  obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
>  obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
>  obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
> +obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
>  obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o
>  obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
> panel-panasonic-vvx10f034n00.o
> diff --git a/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c 
> b/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c
> new file mode 100644
> index ..9fc6b06fffee
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-kingdisplay-kd097d04.c
> @@ -0,0 +1,469 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2017, Fuzhou Rockchip Electronics Co., Ltd
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +struct kingdisplay_panel {
> + struct drm_panel base;
> + struct mipi_dsi_device *link;
> +
> + struct backlight_device *backlight;
> + struct regulator *supply;
> + struct gpio_desc *enable_gpio;
> +
> + bool prepared;
> + bool enabled;
> +};
> +
> +struct kingdisplay_panel_cmd {
> + char cmd;
> + char data;
> +};
> +
> +/*
> + * According to the discussion on
> + * https://review.coreboot.org/#/c/coreboot/+/22472/
> + * the panel init array is not part of the panels datasheet but instead
> + * just came in this form from the panel vendor.
> + */
> +static const struct kingdisplay_panel_cmd init_code[] = {
> + /* voltage setting */
> + { 0xB0, 0x00 },
> + { 0xB2, 0x02 },
> + { 0xB3, 0x11 },
> + { 0xB4, 0x00 },
> + { 0xB6, 0x80 },
> + /* VCOM disable */
> + { 0xB7, 0x02 },
> + { 0xB8, 0x80 },
> + { 0xBA, 0x43 },
> + /* VCOM setting */
> + { 0xBB, 0x53 },
> + /* VSP setting */
> + { 0xBC, 0x0A },
> + /* VSN setting */
> + { 0xBD, 0x4A },
> + /* VGH setting */
> + { 0xBE, 0x2F },
> + /* VGL setting */
> + { 0xBF, 0x1A },
> + { 0xF0, 0x39 },
> + { 0xF1, 0x22 },
> + /* Gamma setting */
> + { 0xB0, 0x02 },
> + { 0xC0, 0x00 },
> + { 0xC1, 0x01 },
> + { 0xC2, 0x0B },
> + { 0xC3, 0x15 },
> + { 0xC4, 0x22 },
> + { 0xC5, 0x11 },
> + { 0xC6, 0x15 },
> + { 0xC7, 0x19 },
> + { 0xC8, 0x1A },
> + { 0xC9, 0x16 },
> + { 0xCA, 0x18 },
> + { 0xCB, 0x13 },
> + { 0xCC, 0x18 },
> + { 0xCD, 0x13 },
> + { 0xCE, 0x1C },
> + { 0xCF, 0x19 },
> + { 0xD0, 0x21 },
> + { 0xD1, 0x2C },
> + { 0xD2, 0x2F },
> + { 0xD3, 0x30 },
> + { 0xD4, 0x19 },
> + { 

Re: [PATCH v2 2/3] dt-bindings: Add KINGDISPLAY KD097D04 panel bindings

[Thierry Reding]

On Mon, Jul 02, 2018 at 12:32:28PM +0200, Heiko Stuebner wrote:
> From: Nickey Yang 
> 
> The KINGDISPLAY KD097D04 is a 9.7" panel with a 1536x2048
> resolution and connected to DSI using 8 lanes.
> 
> Signed-off-by: Nickey Yang 
> Acked-by: Rob Herring 
> Signed-off-by: Heiko Stuebner 
> ---
>  .../display/panel/kingdisplay,kd097d04.txt| 22 +++
>  1 file changed, 22 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/kingdisplay,kd097d04.txt

Applied, thanks.

Thierry


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

Re: [PATCH v2 1/3] dt-bindings: Add vendor prefix for kingdisplay

[Thierry Reding]

On Mon, Jul 02, 2018 at 12:32:27PM +0200, Heiko Stuebner wrote:
> From: Nickey Yang 
> 
> Kingdisplay Technology Co., Ltd, established in
> China Shenzhen in 2006, is a national high-tech
> enterprise specializing in the R, manufacturing
> and marketing of TFT-LCM and touch panel.
> 
> Signed-off-by: Nickey Yang 
> Acked-by: Rob Herring 
> Signed-off-by: Heiko Stuebner 
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thierry


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

[PATCH] drm/panel: innolux-p079zca: Use of_device_get_match_data()

[Thierry Reding]

From: Thierry Reding 

Use this helper to get rid of some extra boilerplate code.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/panel/panel-innolux-p079zca.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c 
b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index 185a55060195..62fbaac96823 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
@@ -314,14 +314,9 @@ static void innolux_panel_del(struct innolux_panel 
*innolux)
 static int innolux_panel_probe(struct mipi_dsi_device *dsi)
 {
const struct panel_desc *desc;
-   const struct of_device_id *id;
int err;
 
-   id = of_match_node(innolux_of_match, dsi->dev.of_node);
-   if (!id)
-   return -ENODEV;
-
-   desc = id->data;
+   desc = of_device_get_match_data(dsi->dev);
dsi->mode_flags = desc->flags;
dsi->format = desc->format;
dsi->lanes = desc->lanes;
-- 
2.17.0

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

Re: [PATCH v5 4/4] drm/panel: p079zca: support Innolux P097PFG panel

[Thierry Reding]

On Mon, Jul 02, 2018 at 12:27:21PM +0200, Heiko Stuebner wrote:
> From: Lin Huang 
> 
> Support Innolux P097PFG 9.7" 1536x2048 TFT LCD panel, it reuse
> the Innolux P079ZCA panel driver.
> 
> Changes in v2:
> - None
> Changes in v3:
> - None
> Changes in v4:
> - None
> Changes in v5:
> - Document source of init-commands
> - 4 lanes per DSI interface
> 
> Signed-off-by: Lin Huang 
> Signed-off-by: Heiko Stuebner 
> ---
>  drivers/gpu/drm/panel/panel-innolux-p079zca.c | 200 +-
>  1 file changed, 196 insertions(+), 4 deletions(-)

Applied with two small changes, see below.

> diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c 
> b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> index 630b0c2549ef..8d25b87bfbd6 100644
> --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> @@ -20,6 +20,15 @@
>  
>  #include 
>  
> +struct panel_init_cmd {
> + int len;

I changed the type here to size_t for consistency.

> + const char *data;
> +};
> +
> +#define _INIT_CMD(...) { \
> + .len = sizeof((char[]){__VA_ARGS__}), \
> + .data = (char[]){__VA_ARGS__} }
> +
>  struct panel_desc {
>   const struct drm_display_mode *modes;
>   unsigned int bpc;
> @@ -30,6 +39,7 @@ struct panel_desc {
>  
>   unsigned long flags;
>   enum mipi_dsi_pixel_format format;
> + const struct panel_init_cmd *init_cmds;
>   unsigned int lanes;
>   const char * const *supply_names;
>   unsigned int num_supplies;
> @@ -121,13 +131,43 @@ static int innolux_panel_prepare(struct drm_panel 
> *panel)
>   if (err < 0)
>   return err;
>  
> - /* T2: 15ms - 1000ms */
> - usleep_range(15000, 16000);
> + /* p079zca: t2 (20ms), p097pfg: t4 (15ms) */
> + usleep_range(2, 21000);
>  
>   gpiod_set_value_cansleep(innolux->enable_gpio, 1);
>  
> - /* T4: 15ms - 1000ms */
> - usleep_range(15000, 16000);
> + /* p079zca: t4, p097pfg: t5 */
> + usleep_range(2, 21000);
> +
> + if (innolux->desc->init_cmds) {
> + const struct panel_init_cmd *cmds =
> + innolux->desc->init_cmds;
> + int i;

I made this unsigned.

Thierry


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

Re: [PATCH v5 3/4] dt-bindings: Add Innolux P097PFG panel bindings

[Thierry Reding]

On Mon, Jul 02, 2018 at 12:27:20PM +0200, Heiko Stuebner wrote:
> From: huang lin 
> 
> The Innolux P097PFG panel is 9.7" panel with 1536X2048
> resolution, it reuse P079ZCA panel driver, so improve
> p079ZCA dt-binding to support P097PFG.
> 
> Changes in v2:
> - None
> Changes in v3:
> - None
> Changes in v4:
> - None
> Changes in v5:
> - use separate file for binding
> - keep power supplies as required
> 
> Signed-off-by: Lin Huang 
> Signed-off-by: Heiko Stuebner 
> ---
>  .../display/panel/innolux,p097pfg.txt | 24 +++
>  1 file changed, 24 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/innolux,p097pfg.txt

Applied, thanks.

Thierry


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

Re: [PATCH v5 2/4] drm/panel: p079zca: add variable unprepare_delay properties

[Thierry Reding]

On Mon, Jul 02, 2018 at 12:27:19PM +0200, Heiko Stuebner wrote:
> From: Lin Huang 
> 
> When panel power down, p079zca need delay between reset and disable
> power supply, but p097pfg does not need it. Similarly p097zca needs
> a delay after entering panel sleep mode. So add two delay properties,
> so we can meet these two panel power down sequence.
> 
> Signed-off-by: Lin Huang 
> [add sleep-mode delay]
> Signed-off-by: Heiko Stuebner 
> ---
>  drivers/gpu/drm/panel/panel-innolux-p079zca.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)

Applied, thanks.

Thierry


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

Re: [PATCH v5 1/4] drm/panel: p079zca: refactor panel driver to support multiple panels

[Thierry Reding]

On Mon, Jul 02, 2018 at 12:27:18PM +0200, Heiko Stuebner wrote:
> From: huang lin 
> 
> Refactor Innolux P079ZCA panel driver, let it support
> multi panels from Innolux that share similar power sequences.
> 
> Panels may require different power supplies so use regulator bulk
> interfaces and define per panel supply-names.
> 
> Changes in v2:
> - Change regulator property name to meet the panel datasheet
> Changes in v3:
> - this patch only refactor P079ZCA panel to support multi panel,
>   support P097PFG panel in another patch
> Changes in v4:
> - Modify the patch which suggest by Thierry
> Changes in v5:
> - use regulator_bulk to handle different supply number
> 
> Signed-off-by: Lin Huang 
> Signed-off-by: Heiko Stuebner 
> ---
>  drivers/gpu/drm/panel/panel-innolux-p079zca.c | 143 --
>  1 file changed, 100 insertions(+), 43 deletions(-)

Applied with minor changes, see below.

> diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c 
> b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> index bb53e0850764..840ad4a6a6a6 100644
> --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> @@ -20,12 +20,29 @@
>  
>  #include 
>  
> +struct panel_desc {
> + const struct drm_display_mode *modes;

I renamed this to "mode" because there is always only one.

> + unsigned int bpc;
> + struct {
> + unsigned int width;
> + unsigned int height;
> + } size;
> +
> + unsigned long flags;
> + enum mipi_dsi_pixel_format format;
> + unsigned int lanes;
> + const char * const *supply_names;
> + unsigned int num_supplies;
> +};
> +
>  struct innolux_panel {
>   struct drm_panel base;
>   struct mipi_dsi_device *link;
> + const struct panel_desc *desc;
>  
>   struct backlight_device *backlight;
> - struct regulator *supply;
> + struct regulator_bulk_data *supplies;
> + unsigned int num_supplies;
>   struct gpio_desc *enable_gpio;
>  
>   bool prepared;
> @@ -77,9 +94,7 @@ static int innolux_panel_unprepare(struct drm_panel *panel)
>   /* T8: 80ms - 1000ms */
>   msleep(80);
>  
> - err = regulator_disable(innolux->supply);
> - if (err < 0)
> - return err;
> + regulator_bulk_disable(innolux->desc->num_supplies, innolux->supplies);

I kept the error check and return here.

Thierry


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

Re: [PATCH][next] drm/panel: ILI9881c: fix missing assignment to error return ret

[Thierry Reding]

On Tue, Jun 26, 2018 at 05:03:54PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Currently, ret is being checked for an error condition however it
> is not being assigned in the previous statement on the call of
> function mipi_dsi_dcs_exit_sleep_mode.  Add in the missing assignment
> of ret.
> 
> Detected by CoverityScan, CID#1470174, 1470178 ("Unchecked return value")
> 
> Fixes: 26aec25593c2 ("drm/panel: Add Ilitek ILI9881c panel driver")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Thierry


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

Re: [PATCH 2/2] drm/panel: simple: Add support for DataImage SCF0700C48GGU18

[Thierry Reding]

On Mon, Jun 25, 2018 at 02:41:30PM +0200, Michal Vokáč wrote:
> This adds support for the DataImage SCF0700C48GGU18 7.0" WVGA (800x480)
> TFT LCD panel. The panel has 24-bit parallel interface and can be
> supported by the simple panel driver.
> 
> Signed-off-by: Michal Vokáč 
> ---
>  .../display/panel/dataimage,scf0700c48ggu18.txt|  8 ++
>  drivers/gpu/drm/panel/panel-simple.c   | 29 
> ++
>  2 files changed, 37 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/dataimage,scf0700c48ggu18.txt

Applied, thanks.

Thierry


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

Re: [PATCH 1/2] dt-bindings: Add DataImage, Inc. vendor prefix

[Thierry Reding]

On Mon, Jun 25, 2018 at 02:41:29PM +0200, Michal Vokáč wrote:
> DataImage is a Taiwan-based manufacturer of LCD panels.
> 
> Signed-off-by: Michal Vokáč 
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

Thierry


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

Re: [PATCH v2 1/1] drm/panel: Add support for Olimex LCD-OLinuXino panel

[Thierry Reding]

On Mon, Jun 25, 2018 at 09:44:35AM +0300, Stefan Mavrodiev wrote:
> This patch adds Olimex Ltd. LCD-OLinuXino bridge panel driver. The
> panel is used with different LCDs (currently from 480x272 to 1280x800).
> Small EEPROM chip is used for identification, which holds some
> factory data and timing requirements.
> 
> Signed-off-by: Stefan Mavrodiev 
> ---
> Changes for v2:
> - Changed lcd_olinuxino_funcs to static const
> 
>  .../display/panel/olimex,lcd-olinuxino.txt |  42 +++
>  MAINTAINERS|   6 +
>  drivers/gpu/drm/panel/Kconfig  |  10 +
>  drivers/gpu/drm/panel/Makefile |   1 +
>  drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c | 360 
> +
>  5 files changed, 419 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/olimex,lcd-olinuxino.txt
>  create mode 100644 drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/olimex,lcd-olinuxino.txt 
> b/Documentation/devicetree/bindings/display/panel/olimex,lcd-olinuxino.txt
> new file mode 100644
> index 000..a89f9c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/olimex,lcd-olinuxino.txt
> @@ -0,0 +1,42 @@
> +Binding for Olimex Ltd. LCD-OLinuXino bridge panel.
> +
> +This device can be used as bridge between a host controller and LCD panels.
> +Currently supported LCDs are:
> +  - LCD-OLinuXino-4.3TS
> +  - LCD-OLinuXino-5
> +  - LCD-OLinuXino-7
> +  - LCD-OLinuXino-10
> +
> +The panel itself contains:
> +  - AT24C16C EEPROM holding panel identification and timing requirements
> +  - AR1021 resistive touch screen controller (optional)
> +  - FT5x6 capacitive touch screnn controller (optional)
> +  - GT911/GT928 capacitive touch screen controller (optional)
> +
> +The above chips share same I2C bus. The EEPROM is factory preprogrammed with
> +device information (id, serial, etc.) and timing requirements.
> +
> +Touchscreen bingings can be found in these files:
> +  - input/touchscreen/goodix.txt
> +  - input/touchscreen/edt-ft5x06.txt
> +  - input/touchscreen/ar1021.txt
> +
> +Required properties:
> +  - compatible: should be "olimex,lcd-olinuxino"
> +  - reg: address of the configuration EEPROM, should be <0x50>
> +  - power-supply: phandle of the regulator that provides the supply voltage
> +
> +Optional properties:
> +  - enable-gpios: GPIO pin to enable or disable the panel
> +  - backlight: phandle of the backlight device attacked to the panel
> +
> +Example:
> + {
> + panel@50 {
> + compatible = "olimex,lcd-olinuxino";
> + reg = <0x50>;
> + power-supply = <_vcc5v0>;
> + enable-gpios = < 7 8 GPIO_ACTIVE_HIGH>;
> + backlight = <>;
> + };
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 624c3fd..30343f1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4557,6 +4557,12 @@ S: Supported
>  F:   drivers/gpu/drm/nouveau/
>  F:   include/uapi/drm/nouveau_drm.h
>  
> +DRM DRIVER FOR OLIMEX LCD-OLINUXINO PANELS
> +M:   Stefan Mavrodiev 
> +S:   Maintained
> +F:   drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c
> +F:   Documentation/devicetree/bindings/display/panel/olimex,lcd-olinuxino.txt
> +
>  DRM DRIVER FOR PERVASIVE DISPLAYS REPAPER PANELS
>  M:   Noralf Trønnes 
>  S:   Maintained
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 25682ff..0292994 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -81,6 +81,16 @@ config DRM_PANEL_LG_LG4573
> Say Y here if you want to enable support for LG4573 RGB panel.
> To compile this driver as a module, choose M here.
>  
> +config DRM_PANEL_OLIMEX_LCD_OLINUXINO
> + tristate "Olimex LCD-OLinuXino panel"
> + depends on OF
> + depends on I2C
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> +   Say Y here if you want to enable support for Olimex Ltd.
> +   LCD-OLinuXino panel. The panel is used with different sizes LCDs,
> +   from 480x272 to 1280x800, and 24 bit per pixel.
> +
>  config DRM_PANEL_ORISETECH_OTM8009A
>   tristate "Orise Technology otm8009a 480x800 dsi 2dl panel"
>   depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index f26efc1..185027f 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += 
> panel-ilitek-ili9322.o
>  obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o
>  obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> +obj-$(CONFIG_DRM_PANEL_OLIMEX_LCD_OLINUXINO) += panel-olimex-lcd-olinuxino.o
>  obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o
>  obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
> 

[PATCH 3/3] ARM: dts: cubox: add LCD controller and TDA998x configuration

[Russell King]

Add DT configuration for the HDMI display output on the Dove Cubox.
This adds support for the LCD0 controller which is connected to a
TDA19988 HDMI encoder.

Signed-off-by: Russell King 
---
 arch/arm/boot/dts/dove-cubox.dts | 43 
 1 file changed, 43 insertions(+)

diff --git a/arch/arm/boot/dts/dove-cubox.dts b/arch/arm/boot/dts/dove-cubox.dts
index 580e3cbcfbf7..f6dd56f63d09 100644
--- a/arch/arm/boot/dts/dove-cubox.dts
+++ b/arch/arm/boot/dts/dove-cubox.dts
@@ -67,6 +67,25 @@
gpu-subsystem {
status = "okay";
};
+
+   reserved-memory {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   display_reserved: framebuffer {
+   compatible = "marvell,dove-framebuffer";
+   size = <0x0200>;
+   alignment = <0x0200>;
+   no-map;
+   };
+   };
+
+   display-subsystem {
+   compatible = "marvell,dove-display-subsystem";
+   memory-region = <_reserved>;
+   ports = <_port>;
+   };
 };
 
  { status = "okay"; };
@@ -117,6 +136,30 @@
silabs,pll-master;
};
};
+
+   tda998x: hdmi-encoder {
+   compatible = "nxp,tda998x";
+   reg = <0x70>;
+   video-ports = <0x234501>;
+   interrupts-extended = < 27 IRQ_TYPE_LEVEL_LOW>;
+
+   port {
+   tda998x_video: endpoint {
+   remote-endpoint = <_rgb>;
+   };
+   };
+   };
+};
+
+ {
+   status = "okay";
+   clocks = < 0>;
+   clock-names = "ext_ref_clk1";
+   lcd0_port: port {
+   lcd0_rgb: endpoint {
+   remote-endpoint = <_video>;
+   };
+   };
 };
 
  {
-- 
2.7.4

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

Re: [PATCH] drm/panel: simple: Add support for Innolux G070Y2-L01

[Thierry Reding]

On Mon, Jun 04, 2018 at 01:16:48PM +0200, Christoph Fritz wrote:
> This patch adds support for Innolux G070Y2-L01 7" WVGA (800x480) TFT LCD
> panel.
> 
> Signed-off-by: Christoph Fritz 
> ---
>  .../bindings/display/panel/innolux,g070y2-l01.txt  | 12 +++
>  drivers/gpu/drm/panel/panel-simple.c   | 37 
> --
>  2 files changed, 47 insertions(+), 2 deletions(-)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/innolux,g070y2-l01.txt

Applied, thanks.

Thierry


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

Re: [RFC PATCHv2 7/9] drm/panel: simple: add newhaven,nhd-4.3-480272ef-atxl LCD

[Thierry Reding]

On Mon, Jun 18, 2018 at 04:22:40PM +0300, Tomi Valkeinen wrote:
> Add support for newhaven,nhd-4.3-480272ef-atxl to panel-simple.
> 
> Signed-off-by: Tomi Valkeinen 
> Cc: Thierry Reding 
> ---
>  .../panel/newhaven,nhd-4.3-480272ef-atxl.txt  |  7 +
>  drivers/gpu/drm/panel/panel-simple.c  | 29 +++
>  2 files changed, 36 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/newhaven,nhd-4.3-480272ef-atxl.txt

Applied after reordering the new entry to be in the proper alphabetical
ordering.

Thierry


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

Re: [PATCH v3 5/9] drm/panel: add support for BOE HV070WSA-100 panel to simple-panel

[Thierry Reding]

On Tue, Jun 19, 2018 at 10:19:26AM +0200, Maciej Purski wrote:
> From: Andrzej Hajda 
> 
> The patch adds support for BOE HV070WSA-100 WSVGA 7.01 inch panel
> in panel-simple driver. The panel is used in Exynos5250-arndale boards.
> 
> Signed-off-by: Andrzej Hajda 
> Signed-off-by: Maciej Purski 
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 25 +
>  1 file changed, 25 insertions(+)

Applied, thanks.

Thierry


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

Re: [PATCH v3 4/9] dt-bindings: display: add DT bindings for BOE HV070WSA-100 panel

[Thierry Reding]

On Tue, Jun 19, 2018 at 10:19:25AM +0200, Maciej Purski wrote:
> From: Andrzej Hajda 
> 
> The patch adds bindings to BOE HV070-WSA WSVGA panel.
> Bindings are compatible with simple panel bindings.
> 
> Signed-off-by: Andrzej Hajda 
> Signed-off-by: Maciej Purski 
> ---
>  .../bindings/display/panel/boe,hv070wsa-100.txt| 28 
> ++
>  1 file changed, 28 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/boe,hv070wsa-100.txt

Applied with the fix as requested by Rob.

Thierry


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

Re: 答复: 答复: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Takashi Iwai]

On Tue, 10 Jul 2018 11:13:27 +0200,
jimqu wrote:
> 
> 
> 
> On 2018年07月10日 16:01, Takashi Iwai wrote:
> > On Tue, 10 Jul 2018 09:44:42 +0200,
> > Qu, Jim wrote:
> >> Hi Takashi,
> >>
> >> I am not expert in audio driver, so I just update some test result, please 
> >> help triage the issue.
> >>
> >> With audio runtime pm:
> > What does this mean?  Is it the stock 4.17.x (or 4.18-rc)?
> > Please clarify your test environment at first: which kernel, which
> > hardware setup.
> >
> This is v4.15 which backport Lukas' patches and also one bug fix
> https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=57cb54e53bdd.
> The platform is AMDAPU(with audio) + AMD dGPU , it is a hybird GFX
> platform. In generic, dGPU will always be suspended.

Ah, so it's AMD+AMD, not Intel+AMD combo.  Now I understand the
situation better, thanks.

> >> 1. ubuntu audio setting use pactl to query audio card/output sink. 
> >> Attachment is pactl output with audio runtime pm.
> >> 2. cat /proc/asound/card0/eld* can get nothing.
> >>
> >> Without audio runtime pm:
> >> 1. pactl can get available audio output/sink
> >> 2. can get eld info from eld#0.1
> >>
> >> IMO, the issue should be:
> >>
> >> Current operations like HDMI hotplug in/out, pactl command, they do
> >> not access audio HW, so the audio could not resume back at this
> >> period.
> > Sorry, not understood.  Why they don't access the audio hardware?
> This is my guess, since I do not get azx_runtime_resume() print info
> which I added. it maybe access HW during this period, but do not
> trigger audio resume progress.
> 
> >> Therefore, upper software could not get HDMI ELD info, can select a 
> >> available card/output sink.
> >> I am also confuse that if there is no ELD info, how driver to steam audio 
> >> device to wake up itself? It's sort of a chicken-or-egg question.
> > As long as it's i915 and HD-audio, the hotplug and ELD info transfer
> > doesn't trigger the runtime PM since it's done via the direct
> > callback.  And ELD value is cached in HD-audio side.
> Yeah, I have reviewed these code, it constructs ELD after reading
> EDID, and write them into HW buffer when set display mode.

If the controller chip supports "wake-enable" feature (HD-audio WAKEEN
register), it could be woken up at unsolicited event even during the
link power down.  But, your report implies that AMD controller doesn't
do this, or something missing there.  That's the likely cause of the
missing hotplug event.

So, if my understanding is right, the situation won't be different
even if you have a single AMD GPU.  And possibly a side-effect of the
latest fix to force link-down for AMD GPU.  Need to check it...


Takashi

> > The exception is that if the notification is done during the PM
> > operation.  But, the connection and ELD query is performed at the end
> > of codec resume, hence it'll be covered there.
> >
> > So in theory, ELD information should be passed from the GPU to
> > HD-audio no matter whether runtime PM is enabled or not.
> >
> >
> > BTW, please stop top-posting.  It makes the thread readability awfully
> > worse.
> >
> Sorry, I justed used outlook client.
> > thanks,
> >
> > Takashi
> >
> >> Thanks
> >> JimQu
> >>
> >> 
> >> 发件人: Takashi Iwai 
> >> 发送时间: 2018年7月10日 1:09
> >> 收件人: Qu, Jim
> >> 抄送: Daniel Vetter; Alex Deucher; alsa-de...@alsa-project.org; 
> >> amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Deucher, 
> >> Alexander
> >> 主题: Re: 答复: [alsa-devel]答复: [PATCH] vgaswitchroo: set audio client 
> >> id according to bound gpu client id
> >>
> >> On Mon, 09 Jul 2018 18:05:09 +0200,
> >> Qu, Jim wrote:
> >>> Hi Takashi,
> >>>
> >>> Not intel, but it is AMD APU+ AMD GFX, the APU has a local HDMI port for 
> >>> extension. And dGPU is only for offloading render via PRIME.
> >>>
> >>> Originally, the HDA driver before v4.17, there is a bug, that all the 
> >>> audio is set to CLIENT_DIS, so the when the dGPU suspend, the audio which 
> >>> is bound to iGPU will also be suspend.
> >>>
> >>> Now, after Lukas' patches. The audio will auto suspend. It make ubuntu 
> >>> audio setting could not detect HDMI audio, even if HDMI has light up.
> >> OK, and it has all necessary patches including the one Lukas
> >> suggested, right?
> >>
> >> If so, figure out what you're actually seeing as "PA could not detect
> >> HDMI audio".  For example, is it the HDMI (jack) detection (giving
> >> false even if it's connected)?  Or is it an error at opening the given
> >> device? Does the driver give the proper ELD bytes as well as the jack
> >> state?
> >>
> >>
> >> Takashi
> >>
> >>> Thanks
> >>> JimQu
> >>>
> >>> -邮件原件-
> >>> 发件人: Takashi Iwai 
> >>> 发送时间: 2018年7月9日 23:58
> >>> 收件人: Daniel Vetter 
> >>> 抄送: Alex Deucher ; alsa-de...@alsa-project.org; 
> >>> amd-...@lists.freedesktop.org; Qu, Jim ; 
> >>> 

Re: [PATCH v4 1/2] drm/panel: Add support for Truly NT35597 panel

[Thierry Reding]

On Wed, May 30, 2018 at 03:37:26PM -0700, abhin...@codeaurora.org wrote:
> Hi Sean
> 
> Thanks for your review.
> 
> Some responses below. Please help to check.
> 
> Thanks
> 
> Abhinav

Looks like this got stuck in review. Some comments below, which I hope
will match Sean's opinion.

> On 2018-05-30 12:12, Sean Paul wrote:
> > On Fri, May 25, 2018 at 05:27:51PM -0700, Abhinav Kumar wrote:
> > > Add support for Truly NT35597 panel used
> > > in MSM reference platforms.
> > > 
> > > This panel supports both single DSI and dual DSI
> > > modes.
> > > 
> > > However, this patch series adds support only for
> > > dual DSI mode.
> > > 
> > > Changes in v4:
> > > - Fix the license identifier
> > > - Fix formatting issues for the regulator loads
> > > - Fix error messages and return code
> > > 
> > > Signed-off-by: Archit Taneja 
> > > Signed-off-by: Abhinav Kumar 
> > > ---
> > >  drivers/gpu/drm/panel/Kconfig   |   8 +
> > >  drivers/gpu/drm/panel/Makefile  |   1 +
> > >  drivers/gpu/drm/panel/panel-truly-nt35597.c | 576
> > > 
> > >  3 files changed, 585 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35597.c
> > > 
> > > diff --git a/drivers/gpu/drm/panel/Kconfig
> > > b/drivers/gpu/drm/panel/Kconfig
> > > index 25682ff..2fcd9b1 100644
> > > --- a/drivers/gpu/drm/panel/Kconfig
> > > +++ b/drivers/gpu/drm/panel/Kconfig
> > > @@ -177,4 +177,12 @@ config DRM_PANEL_SITRONIX_ST7789V
> > > Say Y here if you want to enable support for the Sitronix
> > > ST7789V controller for 240x320 LCD panels
> > > 
> > > +config DRM_PANEL_TRULY_NT35597_WQXGA
> > > + tristate "Truly WQXGA"
> > > + depends on OF
> > > + depends on DRM_MIPI_DSI
> > > + select VIDEOMODE_HELPERS
> > > + help
> > > +   Say Y here if you want to enable support for Truly NT35597 WQXGA
> > > Dual DSI
> > > +   Video Mode panel
> > >  endmenu
> > > diff --git a/drivers/gpu/drm/panel/Makefile
> > > b/drivers/gpu/drm/panel/Makefile
> > > index f26efc1..056ea93 100644
> > > --- a/drivers/gpu/drm/panel/Makefile
> > > +++ b/drivers/gpu/drm/panel/Makefile
> > > @@ -18,3 +18,4 @@ obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) +=
> > > panel-seiko-43wvf1g.o
> > >  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) +=
> > > panel-sharp-lq101r1sx01.o
> > >  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) +=
> > > panel-sharp-ls043t1le01.o
> > >  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> > > +obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
> > > diff --git a/drivers/gpu/drm/panel/panel-truly-nt35597.c
> > > b/drivers/gpu/drm/panel/panel-truly-nt35597.c
> > > new file mode 100644
> > > index 000..a57cbf0
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/panel/panel-truly-nt35597.c
> > > @@ -0,0 +1,576 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +static const char * const regulator_names[] = {
> > > + "vdda",
> > > + "vdispp",
> > > + "vdispn"
> > > +};
> [Abhinav] Will add a ',' here as per jordan's comment
> > > +
> > > +static unsigned long const regulator_enable_loads[] = {
> > > + 62000,
> > > + 10,
> > > + 10,
> > > +};
> > > +
> > > +static unsigned long const regulator_disable_loads[] = {
> > > + 80,
> > > + 100,
> > > + 100,
> > > +};
> > > +
> > > +struct truly_wqxga {
> > > + struct device *dev;
> > > + struct drm_panel panel;
> > > +
> > > + struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
> > > +
> > > + struct gpio_desc *reset_gpio;
> > > + struct gpio_desc *mode_gpio;
> > > +
> > > + struct backlight_device *backlight;
> > > + struct videomode vm;
> > > +
> > > + struct mipi_dsi_device *dsi[2];
> > > +
> > > + bool prepared;
> > > + bool enabled;
> > > +};
> > > +
> > > +static inline struct truly_wqxga *panel_to_truly_wqxga(struct
> > > drm_panel *panel)
> > > +{
> > > + return container_of(panel, struct truly_wqxga, panel);
> > > +}
> > > +
> > > +static int truly_wqxga_power_on(struct truly_wqxga *ctx)
> > > +{
> > > + int ret, i;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> > > + ret = regulator_set_load(ctx->supplies[i].consumer,
> > > + regulator_enable_loads[i]);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
> > > ctx->supplies);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + msleep(20);
> > > + gpiod_set_value(ctx->reset_gpio, 1);
> > > + msleep(20);
> > > + gpiod_set_value(ctx->reset_gpio, 0);
> > > + msleep(20);
> > > + gpiod_set_value(ctx->reset_gpio, 1);
> > > + msleep(50);
> > 
> > Why is this needed? Could you please comment?
> > 

[Bug 107149] Resuming from suspend can cause not working hw acceleration

[bugzilla-daemon]

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

--- Comment #1 from Michel Dänzer  ---
Please attach the dmesg output here directly.

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