[PATCH 1/4] drm: Include internal header for managed function declarations

[PATCH 1/4] drm: Include internal header for managed function declarations

[Chris Wilson]

drivers/gpu/drm/drm_managed.c:61:6: warning: symbol 'drm_managed_release' was not declared. Should it be static?
  CC      drivers/gpu/drm/drm_managed.o
drivers/gpu/drm/drm_managed.c:61:6: warning: no previous prototype for ‘drm_managed_release’ [-Wmissing-prototypes]
 void drm_managed_release(struct drm_device *dev)

Fixes: c6603c740e0e ("drm: add managed resources tied to drm_device")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/drm_managed.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index 9cebfe370a65..1e1356560c2e 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -14,6 +14,8 @@
 #include <drm/drm_device.h>
 #include <drm/drm_print.h>
 
+#include "drm_internal.h"
+
 /**
  * DOC: managed resources
  *
-- 
2.20.1

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

[PATCH 2/4] drm: Help unconfuse gcc, avoid accidental impossible unsigned comparisons

[Chris Wilson]

drivers/gpu/drm/drm_client_modeset.c: In function ‘drm_client_firmware_config’:
./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
   __builtin_constant_p((l) > (h)), (l) > (h), 0)))

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_client_modeset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 7443114bd713..6e9530df0737 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -563,7 +563,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
 				       struct drm_client_offset *offsets,
 				       bool *enabled, int width, int height)
 {
-	unsigned int count = min_t(unsigned int, connector_count, BITS_PER_LONG);
+	const int count = min_t(unsigned int, connector_count, BITS_PER_LONG);
 	unsigned long conn_configured, conn_seq, mask;
 	struct drm_device *dev = client->dev;
 	int i, j;
-- 
2.20.1

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

[PATCH 3/4] drm: Describe dp_rev for drm_dp_set_phy_test_pattern

[Chris Wilson]

drivers/gpu/drm/drm_dp_helper.c:1598: warning: Function parameter or member 'dp_rev' not described in 'drm_dp_set_phy_test_pattern'

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_dp_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 43e57632b00a..532b07118542 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1590,6 +1590,7 @@ EXPORT_SYMBOL(drm_dp_get_phy_test_pattern);
  * drm_dp_set_phy_test_pattern() - set the pattern to the sink.
  * @aux: DisplayPort AUX channel
  * @data: DP phy compliance test parameters.
+ * @dp_rev: DP revision to use for compliance testing
  *
  * Returns 0 on success or a negative error code on failure.
  */
-- 
2.20.1

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

[PATCH 4/4] drm: Match drm_dp_send_clear_payload_id_table definition to declaration

[Chris Wilson]

drivers/gpu/drm/drm_dp_mst_topology.c:2898:6: warning: symbol 'drm_dp_send_clear_payload_id_table' was not declared. Should it be static?
drivers/gpu/drm/drm_dp_mst_topology.c:5451:37: warning: missing braces around initializer
drivers/gpu/drm/drm_dp_mst_topology.c:5451:37: warning: missing braces around initializer

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 70455e304a26..1bdf3cfeeebb 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2895,8 +2895,9 @@ static int drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
 	return ret < 0 ? ret : changed;
 }
 
-void drm_dp_send_clear_payload_id_table(struct drm_dp_mst_topology_mgr *mgr,
-					struct drm_dp_mst_branch *mstb)
+static void
+drm_dp_send_clear_payload_id_table(struct drm_dp_mst_topology_mgr *mgr,
+				   struct drm_dp_mst_branch *mstb)
 {
 	struct drm_dp_sideband_msg_tx *txmsg;
 	int ret;
@@ -5448,7 +5449,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
 {
 	struct drm_dp_mst_port *immediate_upstream_port;
 	struct drm_dp_mst_port *fec_port;
-	struct drm_dp_desc desc = { 0 };
+	struct drm_dp_desc desc = {};
 	u8 endpoint_fec;
 	u8 endpoint_dsc;
 
-- 
2.20.1

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

Re: [PATCH 1/4] drm: Include internal header for managed function declarations

[Daniel Vetter]

On Sat, May 16, 2020 at 10:23:27PM +0100, Chris Wilson wrote:
> drivers/gpu/drm/drm_managed.c:61:6: warning: symbol 'drm_managed_release' was not declared. Should it be static?
>   CC      drivers/gpu/drm/drm_managed.o
> drivers/gpu/drm/drm_managed.c:61:6: warning: no previous prototype for ‘drm_managed_release’ [-Wmissing-prototypes]
>  void drm_managed_release(struct drm_device *dev)

Drat! Thanks for fixing.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> 
> Fixes: c6603c740e0e ("drm: add managed resources tied to drm_device")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_managed.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> index 9cebfe370a65..1e1356560c2e 100644
> --- a/drivers/gpu/drm/drm_managed.c
> +++ b/drivers/gpu/drm/drm_managed.c
> @@ -14,6 +14,8 @@
>  #include <drm/drm_device.h>
>  #include <drm/drm_print.h>
>  
> +#include "drm_internal.h"
> +
>  /**
>   * DOC: managed resources
>   *
> -- 
> 2.20.1
> 

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

Re: [PATCH 2/4] drm: Help unconfuse gcc, avoid accidental impossible unsigned comparisons

[Daniel Vetter]

On Sat, May 16, 2020 at 10:23:28PM +0100, Chris Wilson wrote:
> drivers/gpu/drm/drm_client_modeset.c: In function ‘drm_client_firmware_config’:
> ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>    __builtin_constant_p((l) > (h)), (l) > (h), 0)))
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Took me a while to spot where this goes boom, kinda wonder whether we
should have an

	if (WARN_ON(!connector_count))
		return -EINVAL;

somewhere in here. Just for belts&suspenders approach.

Anyway this looks good.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_client_modeset.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 7443114bd713..6e9530df0737 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -563,7 +563,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
>  				       struct drm_client_offset *offsets,
>  				       bool *enabled, int width, int height)
>  {
> -	unsigned int count = min_t(unsigned int, connector_count, BITS_PER_LONG);
> +	const int count = min_t(unsigned int, connector_count, BITS_PER_LONG);
>  	unsigned long conn_configured, conn_seq, mask;
>  	struct drm_device *dev = client->dev;
>  	int i, j;
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 3/4] drm: Describe dp_rev for drm_dp_set_phy_test_pattern

[Daniel Vetter]

On Sat, May 16, 2020 at 10:23:29PM +0100, Chris Wilson wrote:
> drivers/gpu/drm/drm_dp_helper.c:1598: warning: Function parameter or member 'dp_rev' not described in 'drm_dp_set_phy_test_pattern'
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_dp_helper.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 43e57632b00a..532b07118542 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -1590,6 +1590,7 @@ EXPORT_SYMBOL(drm_dp_get_phy_test_pattern);
>   * drm_dp_set_phy_test_pattern() - set the pattern to the sink.
>   * @aux: DisplayPort AUX channel
>   * @data: DP phy compliance test parameters.
> + * @dp_rev: DP revision to use for compliance testing
>   *
>   * Returns 0 on success or a negative error code on failure.
>   */
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 2/4] drm: Help unconfuse gcc, avoid accidental impossible unsigned comparisons

[Chris Wilson]

Quoting Daniel Vetter (2020-05-18 15:47:44)
> On Sat, May 16, 2020 at 10:23:28PM +0100, Chris Wilson wrote:
> > drivers/gpu/drm/drm_client_modeset.c: In function ‘drm_client_firmware_config’:
> > ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
> >    __builtin_constant_p((l) > (h)), (l) > (h), 0)))
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Took me a while to spot where this goes boom, kinda wonder whether we
> should have an
> 
>         if (WARN_ON(!connector_count))
>                 return -EINVAL;

Atm, drm_client_firmware_config() is called only if connector_count!=0.
But if count==0, we would hit goto retry indefinitely, seems like that
would be worth a WARN.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm: Match drm_dp_send_clear_payload_id_table definition to declaration

[Daniel Vetter]

On Sat, May 16, 2020 at 10:23:30PM +0100, Chris Wilson wrote:
> drivers/gpu/drm/drm_dp_mst_topology.c:2898:6: warning: symbol 'drm_dp_send_clear_payload_id_table' was not declared. Should it be static?

Compiler seems a bit confused, because there is a static declaration a bit
further up. So should have complained about inconsistent decls, but then
my understanding of C is rather shoddy.

> drivers/gpu/drm/drm_dp_mst_topology.c:5451:37: warning: missing braces around initializer
> drivers/gpu/drm/drm_dp_mst_topology.c:5451:37: warning: missing braces around initializer
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 70455e304a26..1bdf3cfeeebb 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2895,8 +2895,9 @@ static int drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
>  	return ret < 0 ? ret : changed;
>  }
>  
> -void drm_dp_send_clear_payload_id_table(struct drm_dp_mst_topology_mgr *mgr,
> -					struct drm_dp_mst_branch *mstb)
> +static void
> +drm_dp_send_clear_payload_id_table(struct drm_dp_mst_topology_mgr *mgr,
> +				   struct drm_dp_mst_branch *mstb)
>  {
>  	struct drm_dp_sideband_msg_tx *txmsg;
>  	int ret;
> @@ -5448,7 +5449,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
>  {
>  	struct drm_dp_mst_port *immediate_upstream_port;
>  	struct drm_dp_mst_port *fec_port;
> -	struct drm_dp_desc desc = { 0 };
> +	struct drm_dp_desc desc = {};
>  	u8 endpoint_fec;
>  	u8 endpoint_dsc;
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 2/4] drm: Help unconfuse gcc, avoid accidental impossible unsigned comparisons

[Emil Velikov]

On Sat, 16 May 2020 at 22:23, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> drivers/gpu/drm/drm_client_modeset.c: In function ‘drm_client_firmware_config’:
> ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>    __builtin_constant_p((l) > (h)), (l) > (h), 0)))
>
Hmm this seems like a hack around a macro bug. I'll send a fix for the
macro in a few minutes.
With that you don't have to tinker to make it unsigned, although const
will be appreciated.

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

Re: [PATCH 2/4] drm: Help unconfuse gcc, avoid accidental impossible unsigned comparisons

[Chris Wilson]

Quoting Emil Velikov (2020-05-18 17:23:21)
> On Sat, 16 May 2020 at 22:23, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > drivers/gpu/drm/drm_client_modeset.c: In function ‘drm_client_firmware_config’:
> > ./include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
> >    __builtin_constant_p((l) > (h)), (l) > (h), 0)))
> >
> Hmm this seems like a hack around a macro bug. I'll send a fix for the
> macro in a few minutes.
> With that you don't have to tinker to make it unsigned, although const
> will be appreciated.

Streams crossed. I added Daniel's suggestion for a WARN_ON to make it
clear that we should never pass connectors_count==0 to
drm_client_firmware_config
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel