Re: [PATCH 2/4] drm: bridge: adv7511: Split connector creation to a separate function

2020-04-12 Thread Sam Ravnborg
Hi Laurent.

On Thu, Apr 09, 2020 at 03:46:08AM +0300, Laurent Pinchart wrote:
> To prepare for making the connector creation optional, move the related
> code out of adv7511_bridge_attach() to a separate function.
> 
> Signed-off-by: Laurent Pinchart 

On nit below, but otherwise:
Acked-by: Sam Ravnborg 

Sam

> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 62 +---
>  1 file changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 58d02e92b6b9..e3b62ad95389 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -783,7 +783,10 @@ static void adv7511_mode_set(struct adv7511 *adv7511,
>   adv7511->f_tmds = mode->clock;
>  }
>  
> -/* Connector funcs */
> +/* 
> -
> + * DRM Connector Operations
> + */
> +
>  static struct adv7511 *connector_to_adv7511(struct drm_connector *connector)
>  {
>   return container_of(connector, struct adv7511, connector);
> @@ -827,7 +830,40 @@ static const struct drm_connector_funcs 
> adv7511_connector_funcs = {
>   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
>  
> -/* Bridge funcs */
> +static int adv7511_connector_init(struct adv7511 *adv)
> +{
> + struct drm_bridge *bridge = >bridge;
> + int ret;
> +
> + if (!bridge->encoder) {
> + DRM_ERROR("Parent encoder object not found");
> + return -ENODEV;
> + }
> +
> + if (adv->i2c_main->irq)
> + adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
> + else
> + adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
> + DRM_CONNECTOR_POLL_DISCONNECT;
> +
> + ret = drm_connector_init(bridge->dev, >connector,
> +  _connector_funcs,
> +  DRM_MODE_CONNECTOR_HDMIA);
> + if (ret) {

Here we test for ret != 0

> + DRM_ERROR("Failed to initialize connector with drm\n");
> + return ret;
> + }
> + drm_connector_helper_add(>connector,
> +  _connector_helper_funcs);
> + drm_connector_attach_encoder(>connector, bridge->encoder);
> +
> + return 0;
> +}
> +
> +/* 
> -
> + * DRM Bridge Operations
> + */
> +
>  static struct adv7511 *bridge_to_adv7511(struct drm_bridge *bridge)
>  {
>   return container_of(bridge, struct adv7511, bridge);
> @@ -867,27 +903,9 @@ static int adv7511_bridge_attach(struct drm_bridge 
> *bridge,
>   return -EINVAL;
>   }
>  
> - if (!bridge->encoder) {
> - DRM_ERROR("Parent encoder object not found");
> - return -ENODEV;
> - }
> -
> - if (adv->i2c_main->irq)
> - adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
> - else
> - adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
> - DRM_CONNECTOR_POLL_DISCONNECT;
> -
> - ret = drm_connector_init(bridge->dev, >connector,
> -  _connector_funcs,
> -  DRM_MODE_CONNECTOR_HDMIA);
> - if (ret) {
> - DRM_ERROR("Failed to initialize connector with drm\n");
> + ret = adv7511_connector_init(adv);
> + if (ret < 0)
Here we test for ret < 0

The code works - but it is inconsistent.
drm_connector_init() is documented to:
"Zero on success, error code on failure."

>   return ret;
> - }
> - drm_connector_helper_add(>connector,
> -  _connector_helper_funcs);
> - drm_connector_attach_encoder(>connector, bridge->encoder);
>  
>   if (adv->type == ADV7533 || adv->type == ADV7535)
>   ret = adv7533_attach_dsi(adv);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> ___
> 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/4] drm: bridge: adv7511: Split EDID read to a separate function

2020-04-12 Thread Sam Ravnborg
Hi Laurent.

On Thu, Apr 09, 2020 at 03:46:07AM +0300, Laurent Pinchart wrote:
> To prepare for the implementation of the DRM bridge connector
> operations, move EDID read out of adv7511_get_modes() to a separate
> function.
> 
> Signed-off-by: Laurent Pinchart 

The code do not check if drm_do_get_edid() return NULL.
But all functions called with the edid seems to handle a NULL edid well.
So thats OK.

Acked-by: Sam Ravnborg 

> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 23 ++--
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index 87b58c1acff4..58d02e92b6b9 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -589,11 +589,10 @@ static int adv7511_get_edid_block(void *data, u8 *buf, 
> unsigned int block,
>   * ADV75xx helpers
>   */
>  
> -static int adv7511_get_modes(struct adv7511 *adv7511,
> -  struct drm_connector *connector)
> +static struct edid *adv7511_get_edid(struct adv7511 *adv7511,
> +  struct drm_connector *connector)
>  {
>   struct edid *edid;
> - unsigned int count;
>  
>   /* Reading the EDID only works if the device is powered */
>   if (!adv7511->powered) {
> @@ -612,15 +611,25 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>   if (!adv7511->powered)
>   __adv7511_power_off(adv7511);
>  
> -
> - drm_connector_update_edid_property(connector, edid);
> - count = drm_add_edid_modes(connector, edid);
> -
>   adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
>  drm_detect_hdmi_monitor(edid));
>  
>   cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
>  
> + return edid;
> +}
> +
> +static int adv7511_get_modes(struct adv7511 *adv7511,
> +  struct drm_connector *connector)
> +{
> + struct edid *edid;
> + unsigned int count;
> +
> + edid = adv7511_get_edid(adv7511, connector);
> +
> + drm_connector_update_edid_property(connector, edid);
> + count = drm_add_edid_modes(connector, edid);
> +
>   kfree(edid);
>  
>   return count;
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> ___
> 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 2/2] drm: bridge: simple-bridge: Make connector creation optional

2020-04-12 Thread Sam Ravnborg
On Thu, Apr 09, 2020 at 03:36:36AM +0300, Laurent Pinchart wrote:
> Make the connector creation optional to enable usage of the
> simple-bridge with the DRM bridge connector helper.
> 
> Signed-off-by: Laurent Pinchart 

Looks straightforward.
Acked-by: Sam Ravnborg 

Some rambling below you can ignore.

Sam

> ---
>  drivers/gpu/drm/bridge/simple-bridge.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/simple-bridge.c 
> b/drivers/gpu/drm/bridge/simple-bridge.c
> index bac223d0430d..bad638088029 100644
> --- a/drivers/gpu/drm/bridge/simple-bridge.c
> +++ b/drivers/gpu/drm/bridge/simple-bridge.c
> @@ -101,16 +101,14 @@ static int simple_bridge_attach(struct drm_bridge 
> *bridge,
>   struct simple_bridge *sbridge = drm_bridge_to_simple_bridge(bridge);

The code below uses both sbridge-> and bridge->
It confused me that we access via bridge-> when possilbe and only
reverts to the "upper" sbridge when needed.
This is unrelated to this patch - just an observation.
It makes code shorter so I can see why it is done.

>   int ret;
>  
> - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> - DRM_ERROR("Fix bridge driver to make connector optional!");
> - return -EINVAL;
> - }
> -
>   ret = drm_bridge_attach(bridge->encoder, sbridge->next_bridge, bridge,
>   DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>   if (ret < 0)
>   return ret;
>  
> + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> + return 0;
> +
>   if (!bridge->encoder) {
>   DRM_ERROR("Missing encoder\n");
>   return -ENODEV;
> @@ -127,8 +125,7 @@ static int simple_bridge_attach(struct drm_bridge *bridge,
>   return ret;
>   }
>  
> - drm_connector_attach_encoder(>connector,
> -   bridge->encoder);
> + drm_connector_attach_encoder(>connector, bridge->encoder);
Unrelated change, but patch is trivial...

>  
>   return 0;
>  }
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> ___
> 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/2] drm: bridge: simple-bridge: Delegate operations to next bridge

2020-04-12 Thread Sam Ravnborg
Hi Laurent.

On Thu, Apr 09, 2020 at 03:36:35AM +0300, Laurent Pinchart wrote:
> Instead of poking into the DT node of the next bridge for its DDC bus
> and implementing the .get_modes() and .detect() connector operations
> manually, retrieve the next bridge in the chain and delegate these
> operations to it.

I had the impression that we could have any number of bridges,
and the approach was to request some info further down in the chain for
info, without knowing if the next or the next->next was the bridge that
could provide the information.
But this seems not to be the case - here we assume ->next can get the
edid - or if not we have a fallback.

The relation that the next bridge was the one with i2c was present
before this patch - so it is not directly related to this patch but
a more general observation.

A few nits below. With these nits considered the patch is:
Acked-by: Sam Ravnborg 

Sam

> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/gpu/drm/bridge/simple-bridge.c | 98 +-
>  1 file changed, 33 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/simple-bridge.c 
> b/drivers/gpu/drm/bridge/simple-bridge.c
> index a2dca7a3ef03..bac223d0430d 100644
> --- a/drivers/gpu/drm/bridge/simple-bridge.c
> +++ b/drivers/gpu/drm/bridge/simple-bridge.c
> @@ -29,7 +29,7 @@ struct simple_bridge {
>  
>   const struct simple_bridge_info *info;
>  
> - struct i2c_adapter  *ddc;
> + struct drm_bridge   *next_bridge;
>   struct regulator*vdd;
>   struct gpio_desc*enable;
>  };
> @@ -52,29 +52,24 @@ static int simple_bridge_get_modes(struct drm_connector 
> *connector)
>   struct edid *edid;
>   int ret;
>  
> - if (!sbridge->ddc)
> - goto fallback;
> -
> - edid = drm_get_edid(connector, sbridge->ddc);
> - if (!edid) {
> - DRM_INFO("EDID readout failed, falling back to standard 
> modes\n");
> - goto fallback;
> + edid = drm_bridge_get_edid(sbridge->next_bridge, connector);

drm_bridge_get_edid() is not documented to return NULL:
"The retrieved EDID on success, or an error pointer otherwise."
So IS_ERR() would do the trick here.

> + if (IS_ERR_OR_NULL(edid)) {
> + if (!edid)
> + DRM_INFO("EDID readout failed, falling back to standard 
> modes\n");
> +
> + /*
> +  * In case we cannot retrieve the EDIDs (missing or broken DDC
> +  * bus from the next bridge), fallback on the XGA standards and
> +  * prefer a mode pretty much anyone can handle.
> +  */
> + ret = drm_add_modes_noedid(connector, 1920, 1200);
> + drm_set_preferred_mode(connector, 1024, 768);
> + return ret;
>   }
>  
>   drm_connector_update_edid_property(connector, edid);
>   ret = drm_add_edid_modes(connector, edid);
>   kfree(edid);
> - return ret;
> -
> -fallback:
> - /*
> -  * In case we cannot retrieve the EDIDs (broken or missing i2c
> -  * bus), fallback on the XGA standards
> -  */
> - ret = drm_add_modes_noedid(connector, 1920, 1200);
> -
> - /* And prefer a mode pretty much anyone can handle */
> - drm_set_preferred_mode(connector, 1024, 768);
>  
>   return ret;
>  }
> @@ -88,16 +83,7 @@ simple_bridge_connector_detect(struct drm_connector 
> *connector, bool force)
>  {
>   struct simple_bridge *sbridge = 
> drm_connector_to_simple_bridge(connector);
>  
> - /*
> -  * Even if we have an I2C bus, we can't assume that the cable
> -  * is disconnected if drm_probe_ddc fails. Some cables don't
> -  * wire the DDC pins, or the I2C bus might not be working at
> -  * all.
> -  */
> - if (sbridge->ddc && drm_probe_ddc(sbridge->ddc))
> - return connector_status_connected;
> -
> - return connector_status_unknown;
> + return drm_bridge_detect(sbridge->next_bridge);
>  }
>  
>  static const struct drm_connector_funcs simple_bridge_con_funcs = {
> @@ -120,6 +106,11 @@ static int simple_bridge_attach(struct drm_bridge 
> *bridge,
>   return -EINVAL;
>   }
>  
> + ret = drm_bridge_attach(bridge->encoder, sbridge->next_bridge, bridge,
> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> + if (ret < 0)
> + return ret;
> +
>   if (!bridge->encoder) {
>   DRM_ERROR("Missing encoder\n");
>   return -ENODEV;
> @@ -130,7 +121,7 @@ static int simple_bridge_attach(struct drm_bridge *bridge,
>   ret = drm_connector_init_with_ddc(bridge->dev, >connector,
> _bridge_con_funcs,
> sbridge->info->connector_type,
> -   sbridge->ddc);
> +   sbridge->next_bridge->ddc);
>   if (ret) {
>   DRM_ERROR("Failed to initialize connector\n");
>  

RE: [PATCH] drm/amdgpu: Add missing '\n' in log messages

2020-04-12 Thread Quan, Evan
Reviewed-by: Evan Quan 

-Original Message-
From: Christophe JAILLET  
Sent: Saturday, April 11, 2020 10:04 PM
To: Deucher, Alexander ; Koenig, Christian 
; Zhou, David(ChunMing) ; 
airl...@linux.ie; dan...@ffwll.ch; Zhang, Hawking ; 
Quan, Evan ; Grodzovsky, Andrey ; 
Liu, Monk ; Russell, Kent ; Ma, Le 

Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org; kernel-janit...@vger.kernel.org; Christophe 
JAILLET 
Subject: [PATCH] drm/amdgpu: Add missing '\n' in log messages

Message logged by 'dev_xxx()' or 'pr_xxx()' should end with a '\n'.

While at it, split some long lines that where not that far.

Signed-off-by: Christophe JAILLET 
---
Most of them have been added in commit bd607166af7f ("drm/amdgpu: Enable 
reading FRU chip via I2C v3")
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 87f7c129c8ce..3d0a50e8c36b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3249,25 +3249,25 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 
r = device_create_file(adev->dev, _attr_pcie_replay_count);
if (r) {
-   dev_err(adev->dev, "Could not create pcie_replay_count");
+   dev_err(adev->dev, "Could not create pcie_replay_count\n");
return r;
}
 
r = device_create_file(adev->dev, _attr_product_name);
if (r) {
-   dev_err(adev->dev, "Could not create product_name");
+   dev_err(adev->dev, "Could not create product_name\n");
return r;
}
 
r = device_create_file(adev->dev, _attr_product_number);
if (r) {
-   dev_err(adev->dev, "Could not create product_number");
+   dev_err(adev->dev, "Could not create product_number\n");
return r;
}
 
r = device_create_file(adev->dev, _attr_serial_number);
if (r) {
-   dev_err(adev->dev, "Could not create serial_number");
+   dev_err(adev->dev, "Could not create serial_number\n");
return r;
}
 
@@ -4270,7 +4270,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
job_signaled = true;
 
if (job_signaled) {
-   dev_info(adev->dev, "Guilty job already signaled, skipping HW 
reset");
+   dev_info(adev->dev, "Guilty job already signaled, skipping HW 
reset\n");
goto skip_hw_reset;
}
 
@@ -4339,10 +4339,12 @@ int amdgpu_device_gpu_recover(struct amdgpu_device 
*adev,
 
if (r) {
/* bad news, how to tell it to userspace ? */
-   dev_info(tmp_adev->dev, "GPU reset(%d) failed\n", 
atomic_read(_adev->gpu_reset_counter));
+   dev_info(tmp_adev->dev, "GPU reset(%d) failed\n",
+atomic_read(_adev->gpu_reset_counter));
amdgpu_vf_error_put(tmp_adev, 
AMDGIM_ERROR_VF_GPU_RESET_FAIL, 0, r);
} else {
-   dev_info(tmp_adev->dev, "GPU reset(%d) succeeded!\n", 
atomic_read(_adev->gpu_reset_counter));
+   dev_info(tmp_adev->dev, "GPU reset(%d) succeeded!\n",
+atomic_read(_adev->gpu_reset_counter));
}
}
 
-- 
2.20.1

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


Re: [PATCH] fbdev: mx3fb: const pointer to ipu_di_signal_cfg

2020-04-12 Thread Sam Ravnborg
On Wed, Apr 08, 2020 at 11:01:41PM +0300, Laurent Pinchart wrote:
> Hi Sam,
> 
> Thank you for the patch.
> 
> On Wed, Apr 08, 2020 at 08:29:26PM +0200, Sam Ravnborg wrote:
> > Laurent Pinchart  and
> > Jani Nikula  both
> > suggested to make the pointer to struct ipu_di_signal_cfg const.
> > 
> > Fix this.
> > 
> > Signed-off-by: Sam Ravnborg 
> > Fixes: 3f6c93ec9254 ("fbdev: mx3fb: avoid warning about psABI change")
> > Cc: Jani Nikula 
> > Cc: Laurent Pinchart 
> > Cc: Arnd Bergmann 
> > Cc: Sam Ravnborg 
> > Cc: Enrico Weigelt 
> > Cc: Bartlomiej Zolnierkiewicz 
> > Cc: linux-fb...@vger.kernel.org
> 
> Assuming this is compile-tested,
> 
> Reviewed-by: Laurent Pinchart 

Thanks. Applied and pushed out to drm-misc-next.

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


[PATCH] video: vt8500lcdfb: fix fallthrough warning

2020-04-12 Thread Sam Ravnborg
Fix following warning:
vt8500lcdfb.c: In function 'vt8500lcd_blank':
vt8500lcdfb.c:229:6: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
  if (info->fix.visual == FB_VISUAL_PSEUDOCOLOR ||
 ^
vt8500lcdfb.c:233:2: note: here
 case FB_BLANK_UNBLANK:
 ^~~~

Adding a simple "fallthrough;" fixed the warning.
The fix was build tested.

Signed-off-by: Sam Ravnborg 
Reported-by: kbuild test robot 
Fixes: e41f1a989408 ("fbdev: Implement simple blanking in pseudocolor modes for 
vt8500lcdfb")
Cc: Alexey Charkov 
Cc: Paul Mundt 
Cc: Bartlomiej Zolnierkiewicz 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fb...@vger.kernel.org
Cc:  # v2.6.38+
---
 drivers/video/fbdev/vt8500lcdfb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/fbdev/vt8500lcdfb.c 
b/drivers/video/fbdev/vt8500lcdfb.c
index f744479dc7df..c61476247ba8 100644
--- a/drivers/video/fbdev/vt8500lcdfb.c
+++ b/drivers/video/fbdev/vt8500lcdfb.c
@@ -230,6 +230,7 @@ static int vt8500lcd_blank(int blank, struct fb_info *info)
info->fix.visual == FB_VISUAL_STATIC_PSEUDOCOLOR)
for (i = 0; i < 256; i++)
vt8500lcd_setcolreg(i, 0, 0, 0, 0, info);
+   fallthrough;
case FB_BLANK_UNBLANK:
if (info->fix.visual == FB_VISUAL_PSEUDOCOLOR ||
info->fix.visual == FB_VISUAL_STATIC_PSEUDOCOLOR)
-- 
2.20.1

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


Re: [PATCH v2 01/36] dt-bindings: display: allow port and ports in panel-lvds

2020-04-12 Thread Sam Ravnborg
Hi Rob.

On Thu, Apr 09, 2020 at 10:11:10AM -0600, Rob Herring wrote:
> On Wed, Apr 8, 2020 at 1:51 PM Sam Ravnborg  wrote:
> >
> > Both port and ports names may be used.
> > port - for a single port
> > ports - if there is more than one port in sub-nodes
> >
> > Fixes the following warning:
> > advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required 
> > property
> >
> > advantech,idk-2121wr.yaml needs several ports, so uses a ports node.
> >
> > Signed-off-by: Sam Ravnborg 
> > Cc: Fabrizio Castro 
> > Cc: Lad Prabhakar 
> > Cc: Thierry Reding 
> > Cc: Sam Ravnborg 
> > ---
> >  Documentation/devicetree/bindings/display/panel/lvds.yaml | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> 
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml 
> > b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > index d0083301acbe..f9132d50821c 100644
> > --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > @@ -102,6 +102,12 @@ required:
> >- width-mm
> >- height-mm
> >- panel-timing
> > -  - port
> > +
> > +if:
> > +  required:
> > +- port
> > +else:
> > +  required:
> > +- ports
> 
> Humm, I guess 'then' is not required. That's a bit weird IMO.
> 
> I usually do a 'oneOf' for these cases.

For the record, I re-did this patch using oneOf - much nicer.
And since the patch was re-written I dropped you r-b.

> 
> Either way, please apply this to drm-misc-fixes (or
> drm-misc-next-fixes depending on the state of the tree). Or I can take
> it. I'd like to get all the warnings cleared up by rc2.

Will take care after -rc1 is out. Needs drm-fixes to be updated with -rc1 first.
Will make sure to cover all bindings warnigns in panel/

Sam

> 
> Reviewed-by: Rob Herring 
> 
> Rob
> ___
> 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 6/7] drm/mediatek: mtk_dsi: Use the drm_panel_bridge API

2020-04-12 Thread Sam Ravnborg
Hi Enric.

Just a "drive-by" comment.
I browsed all the patches - and nothing jumped at me.
But then I did not follow all the changes.

> @@ -1202,10 +1055,19 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>   }
>  
>   ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
> -   >panel, >next_bridge);
> +   , >next_bridge);
>   if (ret)
>   goto err_unregister_host;
>  
> + if (panel) {
> + panel->connector_type = DRM_MODE_CONNECTOR_DSI;
This assignment of panel->connector_type is wrong.
We should let the panel tell the type of connector.

And if the panel fails to do so - then fix it in the panel.

One day, when I get sufficiently bored/motivated I plan to go through
all panels to make sure they all update connector_type.

Sam



> + dsi->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
> + if (IS_ERR(dsi->panel_bridge)) {
> + ret = PTR_ERR(dsi->panel_bridge);
> + goto err_unregister_host;
> + }
> + }
> +
>   dsi->driver_data = of_device_get_match_data(dev);
>  
>   dsi->engine_clk = devm_clk_get(dev, "engine");
> -- 
> 2.25.1
> 
> ___
> 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


[Bug 205497] Attempt to read amd gpu id causes a freeze

2020-04-12 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=205497

--- Comment #23 from albertogomezma...@gmail.com ---
I did not test it. I have not here the laptop to do it. I have now another
laptop with ryyzen 7 3700U

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


Re: [PATCH] drm/msm: Fix typo

2020-04-12 Thread abhinavk

On 2020-04-12 07:35, Christophe JAILLET wrote:

Duplicated 'we'

Signed-off-by: Christophe JAILLET 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 998bef1190a3..b5fed67c4651 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -959,7 +959,7 @@ static int mdp5_crtc_cursor_set(struct drm_crtc 
*crtc,

if (!ctl)
return -EINVAL;

-   /* don't support LM cursors when we we have source split enabled */
+   /* don't support LM cursors when we have source split enabled */
if (mdp5_cstate->pipeline.r_mixer)
return -EINVAL;

@@ -1030,7 +1030,7 @@ static int mdp5_crtc_cursor_move(struct drm_crtc
*crtc, int x, int y)
return -EINVAL;
}

-   /* don't support LM cursors when we we have source split enabled */
+   /* don't support LM cursors when we have source split enabled */
if (mdp5_cstate->pipeline.r_mixer)
return -EINVAL;

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


[PATCH v1 3/4] dt-bindings: media: add wiring property to video-interfaces

2020-04-12 Thread Sam Ravnborg
The wiring property is used to describe the wiring between
the connector and the panel. The property can be used when the
wiring is used to change the mode from for example
BGR to RGB. The first users are the at91sam9 family where
such a wiring trick is sometimes used.
The possilbe values are so far limited to what is required
by the at91sam9 family, but using "text" allows us to extend
this in the future.

There exists similar properties today:
 - display/tilcdc/tilcdc.txt: blue-and-red-wiring
 - display/atmel,lcdc.txt: atmel,lcd-wiring-mode

Neither of the above are defined as endpoint properties.

The new property "wiring" has a more general name and
is defined as an endpoint property.
It will replace atmel,lcd-wiring-mode and may replace
blue-and-red-wiring.

Signed-off-by: Sam Ravnborg 
Cc: Mauro Carvalho Chehab 
Cc: Rob Herring 
Cc: Hans Verkuil 
Cc: linux-me...@vger.kernel.org
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
b/Documentation/devicetree/bindings/media/video-interfaces.txt
index f884ada0bffc..c3bb87c5c9a9 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -141,6 +141,9 @@ Optional endpoint properties
 - link-frequencies: Allowed data bus frequencies. For MIPI CSI-2, for
   instance, this is the actual frequency of the bus, not bits per clock per
   lane value. An array of 64-bit unsigned integers.
+- wiring: Wiring of data lines to display.
+  "straight" - normal wiring.
+  "red-blue-reversed" - red and blue lines reversed.
 - lane-polarities: an array of polarities of the lanes starting from the clock
   lane and followed by the data lanes in the same order as in data-lanes.
   Valid values are 0 (normal) and 1 (inverted). The length of the array
-- 
2.20.1

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


[PATCH v1 2/4] dt-bindings: display: convert atmel lcdc to DT Schema

2020-04-12 Thread Sam Ravnborg
Add a new binding file to describe the bindings
for the Atmel LCDC IP.
This replaces the old txt based binding.

The binding file describes the current binding,
including properties to specify register values etc.
The binding will be updated in a follow-up patch,
the current binding describes the actual situation.

This new binding file replaces the old .txt based
binding which is deleted.

Signed-off-by: Sam Ravnborg 
---
 .../bindings/display/atmel,lcdc.txt   |  88 ---
 .../bindings/display/atmel/lcdc.yaml  | 137 ++
 2 files changed, 137 insertions(+), 88 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/atmel,lcdc.txt
 create mode 100644 Documentation/devicetree/bindings/display/atmel/lcdc.yaml

diff --git a/Documentation/devicetree/bindings/display/atmel,lcdc.txt 
b/Documentation/devicetree/bindings/display/atmel,lcdc.txt
deleted file mode 100644
index acb5a0132127..
--- a/Documentation/devicetree/bindings/display/atmel,lcdc.txt
+++ /dev/null
@@ -1,88 +0,0 @@
-Atmel LCDC Framebuffer
--
-
-Required properties:
-- compatible :
-   "atmel,at91sam9261-lcdc" , 
-   "atmel,at91sam9263-lcdc" ,
-   "atmel,at91sam9g10-lcdc" ,
-   "atmel,at91sam9g45-lcdc" ,
-   "atmel,at91sam9g45es-lcdc" ,
-   "atmel,at91sam9rl-lcdc" ,
-   "atmel,at32ap-lcdc"
-- reg : Should contain 1 register ranges(address and length).
-   Can contain an additional register range(address and length)
-   for fixed framebuffer memory. Useful for dedicated memories.
-- interrupts : framebuffer controller interrupt
-- display: a phandle pointing to the display node
-
-Required nodes:
-- display: a display node is required to initialize the lcd panel
-   This should be in the board dts.
-- default-mode: a videomode within the display with timing parameters
-   as specified below.
-
-Optional properties:
-- lcd-supply: Regulator for LCD supply voltage.
-
-Example:
-
-   fb0: fb@0050 {
-   compatible = "atmel,at91sam9g45-lcdc";
-   reg = <0x0050 0x1000>;
-   interrupts = <23 3 0>;
-   pinctrl-names = "default";
-   pinctrl-0 = <_fb>;
-   display = <>;
-   #address-cells = <1>;
-   #size-cells = <1>;
-
-   };
-
-Example for fixed framebuffer memory:
-
-   fb0: fb@0050 {
-   compatible = "atmel,at91sam9263-lcdc";
-   reg = <0x0070 0x1000 0x7000 0x20>;
-   [...]
-   };
-
-Atmel LCDC Display
--
-Required properties (as per of_videomode_helper):
-
- - atmel,dmacon: dma controller configuration
- - atmel,lcdcon2: lcd controller configuration
- - atmel,guard-time: lcd guard time (Delay in frame periods)
- - bits-per-pixel: lcd panel bit-depth.
-
-Optional properties (as per of_videomode_helper):
- - atmel,lcdcon-backlight: enable backlight
- - atmel,lcdcon-backlight-inverted: invert backlight PWM polarity
- - atmel,lcd-wiring-mode: lcd wiring mode "RGB" or "BRG"
- - atmel,power-control-gpio: gpio to power on or off the LCD (as many as 
needed)
-
-Example:
-   display0: display {
-   bits-per-pixel = <32>;
-   atmel,lcdcon-backlight;
-   atmel,dmacon = <0x1>;
-   atmel,lcdcon2 = <0x80008002>;
-   atmel,guard-time = <9>;
-   atmel,lcd-wiring-mode = <1>;
-
-   display-timings {
-   native-mode = <>;
-   timing0: timing0 {
-   clock-frequency = <900>;
-   hactive = <480>;
-   vactive = <272>;
-   hback-porch = <1>;
-   hfront-porch = <1>;
-   vback-porch = <40>;
-   vfront-porch = <1>;
-   hsync-len = <45>;
-   vsync-len = <1>;
-   };
-   };
-   };
diff --git a/Documentation/devicetree/bindings/display/atmel/lcdc.yaml 
b/Documentation/devicetree/bindings/display/atmel/lcdc.yaml
new file mode 100644
index ..7dcb9a4d5902
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/atmel/lcdc.yaml
@@ -0,0 +1,137 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/atmel/lcdc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel LCDC (LCD Controller) display controller with PWM
+
+maintainers:
+  - Sam Ravnborg 
+
+description: |
+  The Atmel LCDC Display Controller is display controller that
+  includes a PWM for backlight/contrast.
+
+properties:
+  compatible:
+enum:
+  - atmel,at91sam9261-lcdc
+  - atmel,at91sam9263-lcdc
+  - 

[PATCH v1 0/4] dt-bindings: DT Schema variants of atmel lcdc, hlcdc

2020-04-12 Thread Sam Ravnborg
The following four patches update the bindings for Atmel
LCDC, HLCDC display controllers to DT Schema.

The bindings were re-written in DT Schema format,
and examples adjusted/updated to fit.

This patch-set introduces a new endpoint property in video-interfaces
used to tell if red and blue wiring is reversed.
Red and blue wires are reversed as the LCDC IP natively supports BGR
whereas most SW expects RGB. With the wires reversed the SW thinks
the IP supports RGB.
See more details in the changelog of the patch.

The Atmel lcdc binding deprecate a lot of properties in other
to update it to an up-to-date binding. There are no users of the
updated binding, thats something I work on.

Note a big difference between LCDC and HLCDC.
LCDC has all in one binding (interrupts, pwm, display controller).
HLCDC has individual sub-nodes for the PWM and the display contoller.

When posting an update on the old atmel,lcdc.txt file the feedback
was back then that we did not want the split like done for hlcdc.
And it makes for a simpler binding in this way - so feedback looks right.

Despite that Microchip has purchased Atmel, the atmel name is kept.
Also the files are placed in a directory named atmel/.

Feedback welcome!

Sam

Sam Ravnborg (4):
  dt-bindings: display: convert atmel-hlcdc to DT Schema
  dt-bindings: display: convert atmel lcdc to DT Schema
  dt-bindings: media: add wiring property to video-interfaces
  dt-bindings: display: add port support to atmel lcdc

 .../devicetree/bindings/display/atmel,lcdc.txt |  88 
 .../devicetree/bindings/display/atmel/hlcdc-dc.txt |  75 ---
 .../bindings/display/atmel/hlcdc-dc.yaml   | 102 +
 .../devicetree/bindings/display/atmel/lcdc.yaml| 229 +
 .../devicetree/bindings/media/video-interfaces.txt |   3 +
 .../devicetree/bindings/mfd/atmel-hlcdc.txt|  55 -
 .../devicetree/bindings/mfd/atmel-hlcdc.yaml   |  78 +++
 .../devicetree/bindings/pwm/atmel-hlcdc-pwm.txt|  29 ---
 .../devicetree/bindings/pwm/atmel-hlcdc-pwm.yaml   |  39 
 9 files changed, 451 insertions(+), 247 deletions(-)


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


[PATCH v1 4/4] dt-bindings: display: add port support to atmel lcdc

2020-04-12 Thread Sam Ravnborg
Update the Atmel LCDC binding to include:
- pwm. Used for backlight
- endpoints using port node
  Used for handle to panel
- Added wiring property that is used to describe
  the wiring between the LCDC and the panel

Existing properties that should not be used in new
bindings are deprecated.

Updated example to include the updated way to specify panel etc.

Signed-off-by: Sam Ravnborg 
---
 .../bindings/display/atmel/lcdc.yaml  | 94 ++-
 1 file changed, 93 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/atmel/lcdc.yaml 
b/Documentation/devicetree/bindings/display/atmel/lcdc.yaml
index 7dcb9a4d5902..b5c2628f7805 100644
--- a/Documentation/devicetree/bindings/display/atmel/lcdc.yaml
+++ b/Documentation/devicetree/bindings/display/atmel/lcdc.yaml
@@ -28,6 +28,7 @@ properties:
 
   "#address-cells":
 const: 1
+
   "#size-cells":
 const: 0
 
@@ -43,13 +44,84 @@ properties:
   lcd-supply:
 description: Regulator for LCD supply voltage.
 
+  "#pwm-cells":
+description:
+  This PWM chip use the default 3 cells bindings
+  defined in ../../pwm/pwm.yaml.
+const: 3
+
+  clocks:
+maxItems: 2
+
+  clock-names:
+maxItems: 2
+items:
+  - const: lcdc_clk
+  - const: hclk
+
+  port@0:
+type: object
+description: Endpoints of the display controller
+
+properties:
+
+  reg:
+const: 0
+
+  "#address-cells":
+const: 1
+
+  "#size-cells":
+const: 0
+
+  endpoint@0:
+type: object
+description: endpoint node that include phandle to panel
+
+properties:
+
+  reg:
+const: 0
+
+  wiring:
+enum:
+  - straight
+  - red-blue-reversed
+description: |
+  The LCDC is based on a blue-green-red configuration but to adapt
+  to SW only supporting red-green-blue the data lines for red and 
blue
+  may be reversed.
+  See details in: 
http://ww1.microchip.com/downloads/en/AppNotes/doc6300.pdf
+  "straight" - default value. Data lines are not reversed, uses BGR
+  "red-blue-reversed" - red and green are reversed, uses RGB
+
+  remote-endpoint:
+$ref: /schemas/types.yaml#/definitions/phandle
+description:
+  phandle to the panel node
+
+required:
+  - reg
+  - remote-endpoint
+
+additionalProperties: false
+
+required:
+  - "#address-cells"
+  - "#size-cells"
+  - reg
+
+additionalProperties: false
+
   display:
 $ref: /schemas/types.yaml#/definitions/phandle
+deprecated: true
 description: phandle to display node
 
 patternProperties:
   "^display[0-9]$":
 type: object
+deprecated: true
 description: |
   Display node is required to initialize the lcd panel.
   This should be in the board dts
@@ -107,12 +179,32 @@ required:
 
 examples:
   - |
+#include 
+#include 
+
 fb {
 compatible = "atmel,at91sam9263-lcdc";
 reg = <0x0070 0x1000>;
-interrupts = <23 3 0>;
+interrupts = <26 IRQ_TYPE_LEVEL_HIGH 3>;
+clocks = < PMC_TYPE_PERIPHERAL 26>, < PMC_TYPE_PERIPHERAL 26>;
+clock-names = "lcdc_clk", "hclk";
+
+/* pwm for backlight */
+#pwm-cells = <3>;
+
 #address-cells = <1>;
 #size-cells = <0>;
+
+port@0 {
+reg = <0>;
+#address-cells = <1>;
+#size-cells = <0>;
+endpoint@0 {
+reg = <0>;
+wiring = "red-blue-reversed";
+remote-endpoint = <_input>;
+};
+};
 };
 
   - |
-- 
2.20.1

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


[PATCH v1 1/4] dt-bindings: display: convert atmel-hlcdc to DT Schema

2020-04-12 Thread Sam Ravnborg
atmel hlcdc is a MFD devide with two sub-devices:
- PWM
- Display controller

Add binding files for each device:

 - mfd/atmel-hlcdc - this is overall device
 - pwm/atmel-hlcdc-pwm - the pwm part, used for backlight
 - display/atmel/hlcdc-dc - the display controller

The hlcdc IP is present in several different chips from microchip
(former Atmel). The individual chips has independent compatibles in
the mfd binding, to allow for chip specific configuration.

As the conversion is a full re-write there was no tie to the original
license, and the standard license for bindings is used.

Signed-off-by: Sam Ravnborg 
Cc: Boris Brezillon 
---
 .../bindings/display/atmel/hlcdc-dc.txt   |  75 -
 .../bindings/display/atmel/hlcdc-dc.yaml  | 102 ++
 .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  55 --
 .../devicetree/bindings/mfd/atmel-hlcdc.yaml  |  78 ++
 .../bindings/pwm/atmel-hlcdc-pwm.txt  |  29 -
 .../bindings/pwm/atmel-hlcdc-pwm.yaml |  39 +++
 6 files changed, 219 insertions(+), 159 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
 create mode 100644 
Documentation/devicetree/bindings/display/atmel/hlcdc-dc.yaml
 delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.yaml
 delete mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
 create mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.yaml

diff --git a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt 
b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
deleted file mode 100644
index 0398aec488ac..
--- a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
+++ /dev/null
@@ -1,75 +0,0 @@
-Device-Tree bindings for Atmel's HLCDC (High LCD Controller) DRM driver
-
-The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD device.
-See ../../mfd/atmel-hlcdc.txt for more details.
-
-Required properties:
- - compatible: value should be "atmel,hlcdc-display-controller"
- - pinctrl-names: the pin control state names. Should contain "default".
- - pinctrl-0: should contain the default pinctrl states.
- - #address-cells: should be set to 1.
- - #size-cells: should be set to 0.
-
-Required children nodes:
- Children nodes are encoding available output ports and their connections
- to external devices using the OF graph reprensentation (see ../graph.txt).
- At least one port node is required.
-
-Optional properties in grandchild nodes:
- Any endpoint grandchild node may specify a desired video interface
- according to ../../media/video-interfaces.txt, specifically
- - bus-width: recognized values are <12>, <16>, <18> and <24>, and
-   override any output mode selection heuristic, forcing "rgb444",
-   "rgb565", "rgb666" and "rgb888" respectively.
-
-Example:
-
-   hlcdc: hlcdc@f003 {
-   compatible = "atmel,sama5d3-hlcdc";
-   reg = <0xf003 0x2000>;
-   interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
-   clocks = <_clk>, <>, <>;
-   clock-names = "periph_clk","sys_clk", "slow_clk";
-
-   hlcdc-display-controller {
-   compatible = "atmel,hlcdc-display-controller";
-   pinctrl-names = "default";
-   pinctrl-0 = <_lcd_base _lcd_rgb888>;
-   #address-cells = <1>;
-   #size-cells = <0>;
-
-   port@0 {
-   #address-cells = <1>;
-   #size-cells = <0>;
-   reg = <0>;
-
-   hlcdc_panel_output: endpoint@0 {
-   reg = <0>;
-   remote-endpoint = <_input>;
-   };
-   };
-   };
-
-   hlcdc_pwm: hlcdc-pwm {
-   compatible = "atmel,hlcdc-pwm";
-   pinctrl-names = "default";
-   pinctrl-0 = <_lcd_pwm>;
-   #pwm-cells = <3>;
-   };
-   };
-
-Example 2: With a video interface override to force rgb565; as above
-but with these changes/additions:
-
-{
-   hlcdc-display-controller {
-   pinctrl-names = "default";
-   pinctrl-0 = <_lcd_base _lcd_rgb565>;
-
-   port@0 {
-   hlcdc_panel_output: endpoint@0 {
-   bus-width = <16>;
-   };
-   };
-   };
-   };
diff --git a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.yaml 
b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.yaml
new file mode 100644
index ..7eb6266a25a2
--- 

[Bug 205497] Attempt to read amd gpu id causes a freeze

2020-04-12 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=205497

--- Comment #22 from Luya Tshimbalanga (l...@fedoraproject.org) ---
(In reply to albertogomezmarin from comment #21)
> (In reply to Luya Tshimbalanga from comment #20)
> > I confirm the fix landed on kernel 5.4. Thanks Alex for a quick
> > investigation. Closing this report.
> 
> For me It Is happening again, i dont know since what kernel. Ivhace an Asus
> with ryzen 5 3550H

Did the latest updated kernel resolve the issue?

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


Re: [PATCH v11 1/2] dt-bindings: display/bridge: Add binding for NWL mipi dsi host controller

2020-04-12 Thread Guido Günther
Hi Laurent,
On Fri, Apr 10, 2020 at 03:57:32PM +0300, Laurent Pinchart wrote:
> Hi Guido,
> 
> On Fri, Apr 10, 2020 at 02:45:16PM +0200, Guido Günther wrote:
> > On Fri, Apr 10, 2020 at 02:23:42PM +0300, Laurent Pinchart wrote:
> > > On Thu, Apr 09, 2020 at 12:42:01PM +0200, Guido Günther wrote:
> > > > The Northwest Logic MIPI DSI IP core can be found in NXPs i.MX8 SoCs.
> > > > 
> > > > Signed-off-by: Guido Günther 
> > > > Tested-by: Robert Chiras 
> > > > Reviewed-by: Rob Herring 
> > > > Acked-by: Sam Ravnborg 
> > > > Reviewed-by: Fabio Estevam 
> > > > ---
> > > >  .../bindings/display/bridge/nwl-dsi.yaml  | 226 ++
> > > >  1 file changed, 226 insertions(+)
> > > >  create mode 100644 
> > > > Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > > > 
> > > > diff --git 
> > > > a/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml 
> > > > b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > > > new file mode 100644
> > > > index ..8aff2d68fc33
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > > > @@ -0,0 +1,226 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/display/bridge/nwl-dsi.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Northwest Logic MIPI-DSI controller on i.MX SoCs
> > > > +
> > > > +maintainers:
> > > > +  - Guido Gúnther 
> > > > +  - Robert Chiras 
> > > > +
> > > > +description: |
> > > > +  NWL MIPI-DSI host controller found on i.MX8 platforms. This is a dsi 
> > > > bridge for
> > > > +  the SOCs NWL MIPI-DSI host controller.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +const: fsl,imx8mq-nwl-dsi
> > > > +
> > > > +  reg:
> > > > +maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +maxItems: 1
> > > > +
> > > > +  '#address-cells':
> > > > +const: 1
> > > > +
> > > > +  '#size-cells':
> > > > +const: 0
> > > > +
> > > > +  clocks:
> > > > +items:
> > > > +  - description: DSI core clock
> > > > +  - description: RX_ESC clock (used in escape mode)
> > > > +  - description: TX_ESC clock (used in escape mode)
> > > > +  - description: PHY_REF clock
> > > > +  - description: LCDIF clock
> > > > +
> > > > +  clock-names:
> > > > +items:
> > > > +  - const: core
> > > > +  - const: rx_esc
> > > > +  - const: tx_esc
> > > > +  - const: phy_ref
> > > > +  - const: lcdif
> > > > +
> > > > +  mux-controls:
> > > > +description:
> > > > +  mux controller node to use for operating the input mux
> > > > +
> > > > +  phys:
> > > > +maxItems: 1
> > > > +description:
> > > > +  A phandle to the phy module representing the DPHY
> > > > +
> > > > +  phy-names:
> > > > +items:
> > > > +  - const: dphy
> > > > +
> > > > +  power-domains:
> > > > +maxItems: 1
> > > > +
> > > > +  resets:
> > > > +items:
> > > > +  - description: dsi byte reset line
> > > > +  - description: dsi dpi reset line
> > > > +  - description: dsi esc reset line
> > > > +  - description: dsi pclk reset line
> > > > +
> > > > +  reset-names:
> > > > +items:
> > > > +  - const: byte
> > > > +  - const: dpi
> > > > +  - const: esc
> > > > +  - const: pclk
> > > > +
> > > > +  ports:
> > > > +type: object
> > > > +description:
> > > > +  A node containing DSI input & output port nodes with endpoint
> > > > +  definitions as documented in
> > > > +  Documentation/devicetree/bindings/graph.txt.
> > > > +properties:
> > > > +  port@0:
> > > > +type: object
> > > > +description:
> > > > +  Input port node to receive pixel data from the
> > > > +  display controller. Exactly one endpoint must be
> > > > +  specified.
> > > > +properties:
> > > > +  '#address-cells':
> > > > +const: 1
> > > > +
> > > > +  '#size-cells':
> > > > +const: 0
> > > > +
> > > > +  endpoint@0:
> > > > +description: sub-node describing the input from LCDIF
> > > > +type: object
> > > > +
> > > > +  endpoint@1:
> > > > +description: sub-node describing the input from DCSS
> > > > +type: object
> > > 
> > > This models the two inputs to the IP core, that are connected to a mux
> > > internally, controlled through mux-controls, right ? Why is a single
> > > endpoint supported then, if there are two connections at the hardware
> > > level, and why is this using endpoints instead of ports as there are
> > > really two input ports ?
> > 
> > That came out of
> > 
> > https://lore.kernel.org/linux-arm-kernel/c86b7ca2-7799-eafd-c380-e4b551520...@samsung.com/
> > 
> > # If the ip has separate lines for DCSS and LCDIF you should distinguish
> > # by port number. If they are 

Re: [PATCH 00/21] drm: mxsfb: Add i.MX7 support

2020-04-12 Thread Guido Günther
Hi,
On Mon, Mar 09, 2020 at 09:51:55PM +0200, Laurent Pinchart wrote:
> Hello,
> 
> This patch series adds i.MX7 support to the mxsfb driver. The eLCDIF
> instance found in the i.MX7 is backward-compatible with the already
> supported LCDC v4, but has extended features amongst which the most
> notable one is a second plane.
> 
> The first 9 patches (01/21 to 09/21) contain miscellaneous cleanups and
> refactoring to prepare for what is to come. Patch 10/21 starts the real
> work with removal of the DRM simple display pipeline helper, as it
> doesn't support multiple planes. The next patch (11/21) is an additional
> cleanup.
> 
> Patches 12/21 to 14/21 fix vblank handling that I found to be broken
> when testing on my device. Patch 15/21 then performs an additional small
> cleanup, and patch 16/21 starts official support for i.MX7 by mentioning
> it in Kconfig.
> 
> Patch 17/21 adds a new device model for the i.MX6SX and i.MX7 eLCDIF.
> After three additional cleanups in patches 18/21 to 20/21, patch 21/21
> finally adds support for the second plane.
> 
> The code is based on drm-misc-next and has been tested on an i.MX7D
> platform with a DPI panel.

I've tested this with an imx8mq using both the current fsl,imx28-lcdif
and fsl,imx6sx-lcdif with the nwl DSI bridge and a dsi panel and both
worked well.
Cheers,
 -- Guido

> 
> Laurent Pinchart (21):
>   drm: mxsfb: Remove fbdev leftovers
>   drm: mxsfb: Use drm_panel_bridge
>   drm: mxsfb: Use BIT() macro to define register bitfields
>   drm: mxsfb: Remove unused macros from mxsfb_regs.h
>   drm: mxsfb: Clarify format and bus width configuration
>   drm: mxsfb: Pass mxsfb_drm_private pointer to mxsfb_reset_block()
>   drm: mxsfb: Use LCDC_CTRL register name explicitly
>   drm: mxsfb: Remove register definitions from mxsfb_crtc.c
>   drm: mxsfb: Remove unneeded includes
>   drm: mxsfb: Stop using DRM simple display pipeline helper
>   drm: mxsfb: Rename mxsfb_crtc.c to mxsfb_kms.c
>   drm: mxsfb: Move vblank event arm to CRTC .atomic_flush()
>   drm: mxsfb: Don't touch AXI clock in IRQ context
>   drm: mxsfb: Enable vblank handling
>   drm: mxsfb: Remove mxsfb_devdata unused fields
>   drm: mxsfb: Add i.MX7 to the list of supported SoCs in Kconfig
>   drm: mxsfb: Update internal IP version number for i.MX6SX
>   drm: mxsfb: Drop non-OF support
>   drm: mxsfb: Turn mxsfb_set_pixel_fmt() into a void function
>   drm: mxsfb: Merge mxsfb_set_pixel_fmt() and mxsfb_set_bus_fmt()
>   drm: mxsfb: Support the alpha plane
> 
>  drivers/gpu/drm/mxsfb/Kconfig  |   4 +-
>  drivers/gpu/drm/mxsfb/Makefile |   2 +-
>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 343 -
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c  | 246 -
>  drivers/gpu/drm/mxsfb/mxsfb_drv.h  |  42 ++-
>  drivers/gpu/drm/mxsfb/mxsfb_kms.c  | 565 +
>  drivers/gpu/drm/mxsfb/mxsfb_out.c  |  99 -
>  drivers/gpu/drm/mxsfb/mxsfb_regs.h | 107 +++---
>  8 files changed, 730 insertions(+), 678 deletions(-)
>  delete mode 100644 drivers/gpu/drm/mxsfb/mxsfb_crtc.c
>  create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_kms.c
>  delete mode 100644 drivers/gpu/drm/mxsfb/mxsfb_out.c
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> ___
> 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


[Bug 207183] radeon.dpm=1 with second monitor runs hot

2020-04-12 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=207183

--- Comment #6 from Alex Deucher (alexdeuc...@gmail.com) ---
(In reply to Erik Huelsmann from comment #5)
> Is the fact that it's apparently not possible to switch the memory clock
> with 2 monitors attached a design error in the hardware? Unfortunately, I
> don't have the Windows setup anymore to investigate it.
> I'm extremely happy with my 'radeon.dpm=0' setup, because I'm only doing
> text-editing, programming and web browsing.
> 

The GPU dynamically changes the memory and gfx engine clocks at runtime based
on GPU load.  Unfortunately if changed the memory clock dynamically you'd see
flickering almost every frame if the GPU load changed.

You can manually force the clocks low or high via sysfs (as root):

force clocks low:
echo low > /sys/class/drm/card0/device/power_dpm_force_performance_level
force clocks high:
echo high > /sys/class/drm/card0/device/power_dpm_force_performance_level
dynamic:
echo auto > /sys/class/drm/card0/device/power_dpm_force_performance_level

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


Re: [PATCH] drm/hisilicon: Add the shutdown for hibmc_pci_driver

2020-04-12 Thread Sam Ravnborg
Hi Tian.

On Sat, Apr 11, 2020 at 10:49:30AM +0800, Tian Tao wrote:
> add the shutdown function to release the resource.
> 

Why it the release of the memory required in the shutdown method?
The memory is allocated using devm_ioremap() which
will let device management handle the release of the resources when
he driver is released.

The patch also introduces a pci_disable_device()
The better approch would be to use pcim_enable_device()
so you let the device management about releasing the
resources.

Sam

> Signed-off-by: Tian Tao 
> ---
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 23 +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c 
> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index a6fd0c2..126d4f4 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -232,6 +232,21 @@ static int hibmc_hw_map(struct hibmc_drm_private *priv)
>   return 0;
>  }
>  
> +static void hibmc_hw_unmap(struct hibmc_drm_private *priv)
> +{
> + struct drm_device *dev = priv->dev;
> +
> + if (priv->mmio) {
> + devm_iounmap(dev->dev, priv->mmio);
> + priv->mmio = NULL;
> + }
> +
> + if (priv->fb_map) {
> + devm_iounmap(dev->dev, priv->fb_map);
> + priv->fb_map = NULL;
> + }
> +}
> +
>  static int hibmc_hw_init(struct hibmc_drm_private *priv)
>  {
>   int ret;
> @@ -258,6 +273,7 @@ static int hibmc_unload(struct drm_device *dev)
>  
>   hibmc_kms_fini(priv);
>   hibmc_mm_fini(priv);
> + hibmc_hw_unmap(priv);
>   dev->dev_private = NULL;
>   return 0;
>  }
> @@ -374,6 +390,12 @@ static void hibmc_pci_remove(struct pci_dev *pdev)
>   drm_dev_unregister(dev);
>   hibmc_unload(dev);
>   drm_dev_put(dev);
> + pci_disable_device(pdev);
> +}
> +
> +static void hibmc_pci_shutdown(struct pci_dev *pdev)
> +{
> + hibmc_pci_remove(pdev);
>  }
>  
>  static struct pci_device_id hibmc_pci_table[] = {
> @@ -386,6 +408,7 @@ static struct pci_driver hibmc_pci_driver = {
>   .id_table = hibmc_pci_table,
>   .probe =hibmc_pci_probe,
>   .remove =   hibmc_pci_remove,
> + .shutdown = hibmc_pci_shutdown,
>   .driver.pm =_pm_ops,
>  };
>  
> -- 
> 2.7.4
> 
> ___
> 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 0/1] dt-bindings: fix warning in panel/advantech, idk-2121wr

2020-04-12 Thread Sam Ravnborg
The binding for the advantech,idk-2121wr uses an example
where a ports {} node is used.

The panel-lvds binding failed to support a ports node,
which the following patch fixes.

This fix was part of a longer series, but we need this fix
in upstream before -rc2.
So pull this out of the larger series.

Sam
Sam Ravnborg (1):
  dt-bindings: display: allow port and ports in panel-lvds

 Documentation/devicetree/bindings/display/panel/lvds.yaml | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)


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


[PATCH v2 1/1] dt-bindings: display: allow port and ports in panel-lvds

2020-04-12 Thread Sam Ravnborg
Both port and ports names may be used in a panel-lvds binding
  port - for a single port
  ports - if there is more than one port in sub-nodes

Fixes the following warning:
advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property

advantech,idk-2121wr.yaml needs several ports, so uses a ports node.

v2:
  - Use oneOf - makes the logic more obvious (Rob)
  - Added Fixes tag
  - Added port: true, ports:true

Signed-off-by: Sam Ravnborg 
Cc: Rob Herring 
Fixes: 8efef33eff50 ("dt-bindings: display: Add idk-2121wr binding")
Cc: Fabrizio Castro 
Cc: Lad Prabhakar 
Cc: Sam Ravnborg 
Cc: Thierry Reding 
Cc: dri-devel@lists.freedesktop.org
---
 .../devicetree/bindings/display/panel/lvds.yaml| 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml 
b/Documentation/devicetree/bindings/display/panel/lvds.yaml
index d0083301acbe..a5587c4f5ad0 100644
--- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
+++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
@@ -96,12 +96,20 @@ properties:
   If set, reverse the bit order described in the data mappings below on all
   data lanes, transmitting bits for slots 6 to 0 instead of 0 to 6.
 
+  port: true
+  ports: true
+
 required:
   - compatible
   - data-mapping
   - width-mm
   - height-mm
   - panel-timing
-  - port
+
+oneOf:
+  - required:
+- port
+  - required:
+- ports
 
 ...
-- 
2.20.1

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