Re: [PATCH 1/2] drm/amdgpu/gfx10: update golden settings for navi14

[Yuan, Xiaojie]

Reviewed-by: Xiaojie Yuan 

BR,
Xiaojie


From: amd-gfx  on behalf of Alex Deucher 

Sent: Saturday, July 27, 2019 3:16 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: [PATCH 1/2] drm/amdgpu/gfx10: update golden settings for navi14

Updated settings for hw team.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 6162703e20d2..8e7e62492bfb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -138,7 +138,6 @@ static const struct soc15_reg_golden 
golden_settings_gc_10_1_1[] =
SOC15_REG_GOLDEN_VALUE(GC, 0, mmDB_DEBUG4, 0x, 0x0490),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmDB_DFSM_TILES_IN_FLIGHT, 0x, 
0x003f),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmDB_LAST_OF_BURST_CONFIG, 0x, 
0x03860204),
-   SOC15_REG_GOLDEN_VALUE(GC, 0, mmGB_ADDR_CONFIG, 0x0c1800ff, 0x0043),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmGCR_GENERAL_CNTL, 0x1ff0, 
0x0500),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmGE_PRIV_CONTROL, 0x07ff, 
0x01fe),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmGL1_PIPE_STEER, 0x, 0xe4e4e4e4),
--
2.20.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdkfd: Remove GPU ID in GWS queue creation

[Greathouse, Joseph]

The gpu_id argument is not needed when enabling GWS on a queue.
The queue can only be associated with one device, so the only
possible situations for the call as previously defined were:
1) the gpu_id was for the device associated with the target queue
and things worked as expected, or 2) the gpu_id was for a device
not associated with the target queue and the request was undefined.

In particular, the previous result of the undefined operation is
that you would allocate the number of GWS entries available on the
gpu_id device, even if the queue was on a device with a different
number available. For example: a queue on a device without GWS
capability, but the user passes in a gpu_id for a device with GWS.
We would end up trying to allocate GWS on the device that does not
support it.

Rather than leaving this footgun around and making life more
difficult for user space, we can instead grab the gpu_id from the
target queue. The gpu_id argument being passed in is thus not
needed. We thus change the field in the ioctl struct to be reserved
so that nobody expects it to do anything. However, we do not remove
because that would break user-land API compatibility.

Change-Id: I861cebc8a0a7eab5360da10971a73d5a4700c6d8
Signed-off-by: Joseph Greathouse 
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c  | 19 +--
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h |  2 ++
 .../amd/amdkfd/kfd_process_queue_manager.c| 12 
 include/uapi/linux/kfd_ioctl.h|  4 ++--
 4 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index f91126f5f1be..46005b1dcf79 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1572,20 +1572,27 @@ static int kfd_ioctl_alloc_queue_gws(struct file *filep,
 {
int retval;
struct kfd_ioctl_alloc_queue_gws_args *args = data;
+   struct queue *q;
struct kfd_dev *dev;
 
if (!hws_gws_support)
return -ENODEV;
 
-   dev = kfd_device_by_id(args->gpu_id);
-   if (!dev) {
-   pr_debug("Could not find gpu id 0x%x\n", args->gpu_id);
-   return -ENODEV;
+   mutex_lock(&p->mutex);
+   q = pqm_get_user_queue(&p->pqm, args->queue_id);
+
+   if (q)
+   dev = q->device;
+   else {
+   mutex_unlock(&p->mutex);
+   return -EINVAL;
}
-   if (dev->dqm->sched_policy == KFD_SCHED_POLICY_NO_HWS)
+
+   if (dev->dqm->sched_policy == KFD_SCHED_POLICY_NO_HWS) {
+   mutex_unlock(&p->mutex);
return -ENODEV;
+   }
 
-   mutex_lock(&p->mutex);
retval = pqm_set_gws(&p->pqm, args->queue_id, args->num_gws ? dev->gws 
: NULL);
mutex_unlock(&p->mutex);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index aa7bf20d20f8..9b9a8da187c8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -915,6 +915,8 @@ int pqm_set_gws(struct process_queue_manager *pqm, unsigned 
int qid,
void *gws);
 struct kernel_queue *pqm_get_kernel_queue(struct process_queue_manager *pqm,
unsigned int qid);
+struct queue *pqm_get_user_queue(struct process_queue_manager *pqm,
+   unsigned int qid);
 int pqm_get_wave_state(struct process_queue_manager *pqm,
   unsigned int qid,
   void __user *ctl_stack,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 7e6c3ee82f5b..20dae1fdb16a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -473,6 +473,18 @@ struct kernel_queue *pqm_get_kernel_queue(
return NULL;
 }
 
+struct queue *pqm_get_user_queue(struct process_queue_manager *pqm,
+   unsigned int qid)
+{
+   struct process_queue_node *pqn;
+
+   pqn = get_queue_by_qid(pqm, qid);
+   if (pqn && pqn->q)
+   return pqn->q;
+
+   return NULL;
+}
+
 int pqm_get_wave_state(struct process_queue_manager *pqm,
   unsigned int qid,
   void __user *ctl_stack,
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index 070d1bc7e725..0cbd25d2bb38 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -412,14 +412,14 @@ struct kfd_ioctl_unmap_memory_from_gpu_args {
 
 /* Allocate GWS for specific queue
  *
- * @gpu_id:  device identifier
+ * @reserved:reserved for ABI compatibility. value is ignored.
  * @queue_id:queue's id that GWS is allocated for
  * @num_gws: how many GWS to allocate
  * @first_gws:   index of the first G

Re: [PATCH v6 22/24] drm/amdgpu: Provide ddc symlink in connector sysfs directory

[Alex Deucher]

On Fri, Jul 26, 2019 at 3:42 PM Andrzej Pietrasiewicz
 wrote:
>
> Hi Alex,
>
>
> W dniu 26.07.2019 o 21:28, Alex Deucher pisze:
> > On Fri, Jul 26, 2019 at 1:28 PM Andrzej Pietrasiewicz
> >  wrote:
> >>
> >> Use the ddc pointer provided by the generic connector.
> >>
> >> Signed-off-by: Andrzej Pietrasiewicz 
> >
> > Note that this only covers the legacy display code.  The new DC
> > display code also needs to be converted.  See:
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>
> In amdgpu_dm_connector_init() the ddc is &i2c->base, is it?

Yes.

>
> But it is not clear to me how can I find ddc pointer in
> dm_dp_add_mst_connector()?

+ Harry and Nick.

hmmm, not sure about MST.  Maybe just skip them for now.

Alex

>
> Andrzej
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v6 22/24] drm/amdgpu: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Hi Alex,


W dniu 26.07.2019 o 21:28, Alex Deucher pisze:

On Fri, Jul 26, 2019 at 1:28 PM Andrzej Pietrasiewicz
 wrote:


Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 


Note that this only covers the legacy display code.  The new DC
display code also needs to be converted.  See:
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c


In amdgpu_dm_connector_init() the ddc is &i2c->base, is it?

But it is not clear to me how can I find ddc pointer in
dm_dp_add_mst_connector()?

Andrzej

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v6 20/24] drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory

[Jernej Škrabec]

Hi!

Dne petek, 26. julij 2019 ob 19:23:14 CEST je Andrzej Pietrasiewicz 
napisal(a):
> Use the ddc pointer provided by the generic connector.
> 
> Signed-off-by: Andrzej Pietrasiewicz 

Acked-by: Jernej Skrabec 

Thanks!

Best regards,
Jernej

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> 218a7b2308f7..83b94b66e464 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2200,8 +2200,10 @@ static int dw_hdmi_bridge_attach(struct drm_bridge
> *bridge)
> 
>   drm_connector_helper_add(connector, 
&dw_hdmi_connector_helper_funcs);
> 
> - drm_connector_init(bridge->dev, connector, 
&dw_hdmi_connector_funcs,
> -DRM_MODE_CONNECTOR_HDMIA);
> + drm_connector_init_with_ddc(bridge->dev, connector,
> + &dw_hdmi_connector_funcs,
> + DRM_MODE_CONNECTOR_HDMIA,
> + hdmi->ddc);
> 
>   drm_connector_attach_encoder(connector, encoder);




___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v6 22/24] drm/amdgpu: Provide ddc symlink in connector sysfs directory

[Alex Deucher]

On Fri, Jul 26, 2019 at 1:28 PM Andrzej Pietrasiewicz
 wrote:
>
> Use the ddc pointer provided by the generic connector.
>
> Signed-off-by: Andrzej Pietrasiewicz 

Note that this only covers the legacy display code.  The new DC
display code also needs to be converted.  See:
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
With those updated as well:
Acked-by: Alex Deucher 

> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 96 ++-
>  1 file changed, 70 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index 73b2ede773d3..ece55c8fa673 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -1505,6 +1505,7 @@ amdgpu_connector_add(struct amdgpu_device *adev,
> struct amdgpu_connector_atom_dig *amdgpu_dig_connector;
> struct drm_encoder *encoder;
> struct amdgpu_encoder *amdgpu_encoder;
> +   struct i2c_adapter *ddc = NULL;
> uint32_t subpixel_order = SubPixelNone;
> bool shared_ddc = false;
> bool is_dp_bridge = false;
> @@ -1574,17 +1575,21 @@ amdgpu_connector_add(struct amdgpu_device *adev,
> amdgpu_connector->con_priv = amdgpu_dig_connector;
> if (i2c_bus->valid) {
> amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, 
> i2c_bus);
> -   if (amdgpu_connector->ddc_bus)
> +   if (amdgpu_connector->ddc_bus) {
> has_aux = true;
> -   else
> +   ddc = &amdgpu_connector->ddc_bus->adapter;
> +   } else {
> DRM_ERROR("DP: Failed to assign ddc bus! 
> Check dmesg for i2c errors.\n");
> +   }
> }
> switch (connector_type) {
> case DRM_MODE_CONNECTOR_VGA:
> case DRM_MODE_CONNECTOR_DVIA:
> default:
> -   drm_connector_init(dev, &amdgpu_connector->base,
> -  &amdgpu_connector_dp_funcs, 
> connector_type);
> +   drm_connector_init_with_ddc(dev, 
> &amdgpu_connector->base,
> +   
> &amdgpu_connector_dp_funcs,
> +   connector_type,
> +   ddc);
> drm_connector_helper_add(&amdgpu_connector->base,
>  
> &amdgpu_connector_dp_helper_funcs);
> connector->interlace_allowed = true;
> @@ -1602,8 +1607,10 @@ amdgpu_connector_add(struct amdgpu_device *adev,
> case DRM_MODE_CONNECTOR_HDMIA:
> case DRM_MODE_CONNECTOR_HDMIB:
> case DRM_MODE_CONNECTOR_DisplayPort:
> -   drm_connector_init(dev, &amdgpu_connector->base,
> -  &amdgpu_connector_dp_funcs, 
> connector_type);
> +   drm_connector_init_with_ddc(dev, 
> &amdgpu_connector->base,
> +   
> &amdgpu_connector_dp_funcs,
> +   connector_type,
> +   ddc);
> drm_connector_helper_add(&amdgpu_connector->base,
>  
> &amdgpu_connector_dp_helper_funcs);
> 
> drm_object_attach_property(&amdgpu_connector->base.base,
> @@ -1644,8 +1651,10 @@ amdgpu_connector_add(struct amdgpu_device *adev,
> break;
> case DRM_MODE_CONNECTOR_LVDS:
> case DRM_MODE_CONNECTOR_eDP:
> -   drm_connector_init(dev, &amdgpu_connector->base,
> -  &amdgpu_connector_edp_funcs, 
> connector_type);
> +   drm_connector_init_with_ddc(dev, 
> &amdgpu_connector->base,
> +   
> &amdgpu_connector_edp_funcs,
> +   connector_type,
> +   ddc);
> drm_connector_helper_add(&amdgpu_connector->base,
>  
> &amdgpu_connector_dp_helper_funcs);
> 
> drm_object_attach_property(&amdgpu_connector->base.base,
> @@ -1659,13 +1668,18 @@ amdgpu_connector_add(struct amdgpu_device *adev,
> } else {
> switch (connector_type) {
> case DRM_MODE_CONNECTOR_VGA:
> -   drm_connector_init(dev, &amdgpu_connector->base, 
> &amdgpu_connector_vga_f

Re: [PATCH v6 23/24] drm/radeon: Provide ddc symlink in connector sysfs directory

[Alex Deucher]

On Fri, Jul 26, 2019 at 1:29 PM Andrzej Pietrasiewicz
 wrote:
>
> Use the ddc pointer provided by the generic connector.
>
> Signed-off-by: Andrzej Pietrasiewicz 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/radeon/radeon_connectors.c | 142 +++--
>  1 file changed, 106 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
> b/drivers/gpu/drm/radeon/radeon_connectors.c
> index c60d1a44d22a..b3ad8d890801 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -1870,6 +1870,7 @@ radeon_add_atom_connector(struct drm_device *dev,
> struct radeon_connector_atom_dig *radeon_dig_connector;
> struct drm_encoder *encoder;
> struct radeon_encoder *radeon_encoder;
> +   struct i2c_adapter *ddc;
> uint32_t subpixel_order = SubPixelNone;
> bool shared_ddc = false;
> bool is_dp_bridge = false;
> @@ -1947,17 +1948,21 @@ radeon_add_atom_connector(struct drm_device *dev,
> radeon_connector->con_priv = radeon_dig_connector;
> if (i2c_bus->valid) {
> radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, 
> i2c_bus);
> -   if (radeon_connector->ddc_bus)
> +   if (radeon_connector->ddc_bus) {
> has_aux = true;
> -   else
> +   ddc = &radeon_connector->ddc_bus->adapter;
> +   } else {
> DRM_ERROR("DP: Failed to assign ddc bus! 
> Check dmesg for i2c errors.\n");
> +   }
> }
> switch (connector_type) {
> case DRM_MODE_CONNECTOR_VGA:
> case DRM_MODE_CONNECTOR_DVIA:
> default:
> -   drm_connector_init(dev, &radeon_connector->base,
> -  &radeon_dp_connector_funcs, 
> connector_type);
> +   drm_connector_init_with_ddc(dev, 
> &radeon_connector->base,
> +   
> &radeon_dp_connector_funcs,
> +   connector_type,
> +   ddc);
> drm_connector_helper_add(&radeon_connector->base,
>  
> &radeon_dp_connector_helper_funcs);
> connector->interlace_allowed = true;
> @@ -1979,8 +1984,10 @@ radeon_add_atom_connector(struct drm_device *dev,
> case DRM_MODE_CONNECTOR_HDMIA:
> case DRM_MODE_CONNECTOR_HDMIB:
> case DRM_MODE_CONNECTOR_DisplayPort:
> -   drm_connector_init(dev, &radeon_connector->base,
> -  &radeon_dp_connector_funcs, 
> connector_type);
> +   drm_connector_init_with_ddc(dev, 
> &radeon_connector->base,
> +   
> &radeon_dp_connector_funcs,
> +   connector_type,
> +   ddc);
> drm_connector_helper_add(&radeon_connector->base,
>  
> &radeon_dp_connector_helper_funcs);
> 
> drm_object_attach_property(&radeon_connector->base.base,
> @@ -2027,8 +2034,10 @@ radeon_add_atom_connector(struct drm_device *dev,
> break;
> case DRM_MODE_CONNECTOR_LVDS:
> case DRM_MODE_CONNECTOR_eDP:
> -   drm_connector_init(dev, &radeon_connector->base,
> -  
> &radeon_lvds_bridge_connector_funcs, connector_type);
> +   drm_connector_init_with_ddc(dev, 
> &radeon_connector->base,
> +   
> &radeon_lvds_bridge_connector_funcs,
> +   connector_type,
> +   ddc);
> drm_connector_helper_add(&radeon_connector->base,
>  
> &radeon_dp_connector_helper_funcs);
> 
> drm_object_attach_property(&radeon_connector->base.base,
> @@ -2042,13 +2051,18 @@ radeon_add_atom_connector(struct drm_device *dev,
> } else {
> switch (connector_type) {
> case DRM_MODE_CONNECTOR_VGA:
> -   drm_connector_init(dev, &radeon_connector->base, 
> &radeon_vga_connector_funcs, connector_type);
> -   drm_connector_helper_add(&radeon_connector->base, 
> &radeon_vga_connector_helper_funcs);
> if (i2c_bus->valid) {
> radeon_connector->ddc_bus = 

[PATCH 1/2] drm/amdgpu/gfx10: update golden settings for navi14

[Alex Deucher]

Updated settings for hw team.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 6162703e20d2..8e7e62492bfb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -138,7 +138,6 @@ static const struct soc15_reg_golden 
golden_settings_gc_10_1_1[] =
SOC15_REG_GOLDEN_VALUE(GC, 0, mmDB_DEBUG4, 0x, 0x0490),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmDB_DFSM_TILES_IN_FLIGHT, 0x, 
0x003f),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmDB_LAST_OF_BURST_CONFIG, 0x, 
0x03860204),
-   SOC15_REG_GOLDEN_VALUE(GC, 0, mmGB_ADDR_CONFIG, 0x0c1800ff, 0x0043),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmGCR_GENERAL_CNTL, 0x1ff0, 
0x0500),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmGE_PRIV_CONTROL, 0x07ff, 
0x01fe),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmGL1_PIPE_STEER, 0x, 0xe4e4e4e4),
-- 
2.20.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdkfd: enable KFD support for navi14

[Alex Deucher]

Same as navi10.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index f052c70e4659..97f7c5235cc9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -91,6 +91,7 @@ void amdgpu_amdkfd_device_probe(struct amdgpu_device *adev)
kfd2kgd = amdgpu_amdkfd_arcturus_get_functions();
break;
case CHIP_NAVI10:
+   case CHIP_NAVI14:
kfd2kgd = amdgpu_amdkfd_gfx_10_0_get_functions();
break;
default:
-- 
2.20.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Review required [Was: Associate ddc adapters with connectors]

[Sam Ravnborg]

Hi all.

Andrzej have done a good job following up on feedback and this series is
now ready.

We need ack on the patches touching the individual drivers before we can
proceed.
Please check your drivers and get back.

Sam

> Hi Andezej.
> 
> On Fri, Jul 26, 2019 at 07:22:54PM +0200, Andrzej Pietrasiewicz wrote:
> > It is difficult for a user to know which of the i2c adapters is for which
> > drm connector. This series addresses this problem.
> > 
> > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> > 
> > ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> > -> ../../../../soc/1388.i2c/i2c-2
> > 
> > The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
> > ddcutil:
> > 
> > ddcutil -b 2 getvcp 0x10
> > VCP code 0x10 (Brightness): current value =90, max value =   100
> > 
> > The first patch in the series adds struct i2c_adapter pointer to struct
> > drm_connector. If the field is used by a particular driver, then an
> > appropriate symbolic link is created by the generic code, which is also 
> > added
> > by this patch.
> > 
> > Patch 2 adds a new variant of drm_connector_init(), see the changelog
> > below.
> > 
> > Patches 3..24 are examples of how to convert a driver to this new scheme.
> > 
> ...
> > 
> > v5..v6:
> > 
> > - improved subject line of patch 1
> > - added kernel-doc for drm_connector_init_with_ddc()
> > - improved kernel-doc for the ddc field of struct drm_connector
> > - added Reviewed-by in patches 17 and 18
> > - added Acked-by in patch 2
> > - made the ownership of ddc i2c_adapter explicit in all patches,
> > this made the affected patches much simpler
> 
> Looks good now.
> Patch 1 and 2 are:
> Reviewed-by: Sam Ravnborg 
> 
> The remaining patches are:
> Acked-by: Sam Ravnborg 
> 
>   Sam
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v6 00/24] Associate ddc adapters with connectors

[Sam Ravnborg]

Hi Andezej.

On Fri, Jul 26, 2019 at 07:22:54PM +0200, Andrzej Pietrasiewicz wrote:
> It is difficult for a user to know which of the i2c adapters is for which
> drm connector. This series addresses this problem.
> 
> The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> 
> ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
>   -> ../../../../soc/1388.i2c/i2c-2
> 
> The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
> ddcutil:
> 
> ddcutil -b 2 getvcp 0x10
> VCP code 0x10 (Brightness): current value =90, max value =   100
> 
> The first patch in the series adds struct i2c_adapter pointer to struct
> drm_connector. If the field is used by a particular driver, then an
> appropriate symbolic link is created by the generic code, which is also added
> by this patch.
> 
> Patch 2 adds a new variant of drm_connector_init(), see the changelog
> below.
> 
> Patches 3..24 are examples of how to convert a driver to this new scheme.
> 
...
> 
> v5..v6:
> 
> - improved subject line of patch 1
> - added kernel-doc for drm_connector_init_with_ddc()
> - improved kernel-doc for the ddc field of struct drm_connector
> - added Reviewed-by in patches 17 and 18
> - added Acked-by in patch 2
> - made the ownership of ddc i2c_adapter explicit in all patches,
> this made the affected patches much simpler

Looks good now.
Patch 1 and 2 are:
Reviewed-by: Sam Ravnborg 

The remaining patches are:
Acked-by: Sam Ravnborg 

Sam
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 21/24] drm/bridge: ti-tfp410: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/bridge/ti-tfp410.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c 
b/drivers/gpu/drm/bridge/ti-tfp410.c
index dbf35c7bc85e..61cc2354ef1b 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -134,8 +134,10 @@ static int tfp410_attach(struct drm_bridge *bridge)
 
drm_connector_helper_add(&dvi->connector,
 &tfp410_con_helper_funcs);
-   ret = drm_connector_init(bridge->dev, &dvi->connector,
-&tfp410_con_funcs, dvi->connector_type);
+   ret = drm_connector_init_with_ddc(bridge->dev, &dvi->connector,
+ &tfp410_con_funcs,
+ dvi->connector_type,
+ dvi->ddc);
if (ret) {
dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret);
return ret;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 20/24] drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 218a7b2308f7..83b94b66e464 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2200,8 +2200,10 @@ static int dw_hdmi_bridge_attach(struct drm_bridge 
*bridge)
 
drm_connector_helper_add(connector, &dw_hdmi_connector_helper_funcs);
 
-   drm_connector_init(bridge->dev, connector, &dw_hdmi_connector_funcs,
-  DRM_MODE_CONNECTOR_HDMIA);
+   drm_connector_init_with_ddc(bridge->dev, connector,
+   &dw_hdmi_connector_funcs,
+   DRM_MODE_CONNECTOR_HDMIA,
+   hdmi->ddc);
 
drm_connector_attach_encoder(connector, encoder);
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 24/24] drm/i915: Provide ddc symlink in hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 9bf28de10401..268f1bd20b99 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -3067,6 +3067,7 @@ void intel_hdmi_init_connector(struct intel_digital_port 
*intel_dig_port,
struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct drm_device *dev = intel_encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
+   struct i2c_adapter *ddc;
enum port port = intel_encoder->port;
 
DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
@@ -3077,8 +3078,13 @@ void intel_hdmi_init_connector(struct intel_digital_port 
*intel_dig_port,
 intel_dig_port->max_lanes, port_name(port)))
return;
 
-   drm_connector_init(dev, connector, &intel_hdmi_connector_funcs,
-  DRM_MODE_CONNECTOR_HDMIA);
+   intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
+   ddc = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
+
+   drm_connector_init_with_ddc(dev, connector,
+   &intel_hdmi_connector_funcs,
+   DRM_MODE_CONNECTOR_HDMIA,
+   ddc);
drm_connector_helper_add(connector, &intel_hdmi_connector_helper_funcs);
 
connector->interlace_allowed = 1;
@@ -3088,8 +3094,6 @@ void intel_hdmi_init_connector(struct intel_digital_port 
*intel_dig_port,
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
connector->ycbcr_420_allowed = true;
 
-   intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
-
if (WARN_ON(port == PORT_A))
return;
intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 23/24] drm/radeon: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/radeon/radeon_connectors.c | 142 +++--
 1 file changed, 106 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c 
b/drivers/gpu/drm/radeon/radeon_connectors.c
index c60d1a44d22a..b3ad8d890801 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1870,6 +1870,7 @@ radeon_add_atom_connector(struct drm_device *dev,
struct radeon_connector_atom_dig *radeon_dig_connector;
struct drm_encoder *encoder;
struct radeon_encoder *radeon_encoder;
+   struct i2c_adapter *ddc;
uint32_t subpixel_order = SubPixelNone;
bool shared_ddc = false;
bool is_dp_bridge = false;
@@ -1947,17 +1948,21 @@ radeon_add_atom_connector(struct drm_device *dev,
radeon_connector->con_priv = radeon_dig_connector;
if (i2c_bus->valid) {
radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, 
i2c_bus);
-   if (radeon_connector->ddc_bus)
+   if (radeon_connector->ddc_bus) {
has_aux = true;
-   else
+   ddc = &radeon_connector->ddc_bus->adapter;
+   } else {
DRM_ERROR("DP: Failed to assign ddc bus! Check 
dmesg for i2c errors.\n");
+   }
}
switch (connector_type) {
case DRM_MODE_CONNECTOR_VGA:
case DRM_MODE_CONNECTOR_DVIA:
default:
-   drm_connector_init(dev, &radeon_connector->base,
-  &radeon_dp_connector_funcs, 
connector_type);
+   drm_connector_init_with_ddc(dev, 
&radeon_connector->base,
+   &radeon_dp_connector_funcs,
+   connector_type,
+   ddc);
drm_connector_helper_add(&radeon_connector->base,
 
&radeon_dp_connector_helper_funcs);
connector->interlace_allowed = true;
@@ -1979,8 +1984,10 @@ radeon_add_atom_connector(struct drm_device *dev,
case DRM_MODE_CONNECTOR_HDMIA:
case DRM_MODE_CONNECTOR_HDMIB:
case DRM_MODE_CONNECTOR_DisplayPort:
-   drm_connector_init(dev, &radeon_connector->base,
-  &radeon_dp_connector_funcs, 
connector_type);
+   drm_connector_init_with_ddc(dev, 
&radeon_connector->base,
+   &radeon_dp_connector_funcs,
+   connector_type,
+   ddc);
drm_connector_helper_add(&radeon_connector->base,
 
&radeon_dp_connector_helper_funcs);
drm_object_attach_property(&radeon_connector->base.base,
@@ -2027,8 +2034,10 @@ radeon_add_atom_connector(struct drm_device *dev,
break;
case DRM_MODE_CONNECTOR_LVDS:
case DRM_MODE_CONNECTOR_eDP:
-   drm_connector_init(dev, &radeon_connector->base,
-  &radeon_lvds_bridge_connector_funcs, 
connector_type);
+   drm_connector_init_with_ddc(dev, 
&radeon_connector->base,
+   
&radeon_lvds_bridge_connector_funcs,
+   connector_type,
+   ddc);
drm_connector_helper_add(&radeon_connector->base,
 
&radeon_dp_connector_helper_funcs);
drm_object_attach_property(&radeon_connector->base.base,
@@ -2042,13 +2051,18 @@ radeon_add_atom_connector(struct drm_device *dev,
} else {
switch (connector_type) {
case DRM_MODE_CONNECTOR_VGA:
-   drm_connector_init(dev, &radeon_connector->base, 
&radeon_vga_connector_funcs, connector_type);
-   drm_connector_helper_add(&radeon_connector->base, 
&radeon_vga_connector_helper_funcs);
if (i2c_bus->valid) {
radeon_connector->ddc_bus = 
radeon_i2c_lookup(rdev, i2c_bus);
if (!radeon_connector->ddc_bus)
DRM_ERROR("VGA: Failed to assign ddc 
bus! Check dmesg for i2c errors.\n");
+   else
+

[PATCH v6 22/24] drm/amdgpu: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 96 ++-
 1 file changed, 70 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 73b2ede773d3..ece55c8fa673 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -1505,6 +1505,7 @@ amdgpu_connector_add(struct amdgpu_device *adev,
struct amdgpu_connector_atom_dig *amdgpu_dig_connector;
struct drm_encoder *encoder;
struct amdgpu_encoder *amdgpu_encoder;
+   struct i2c_adapter *ddc = NULL;
uint32_t subpixel_order = SubPixelNone;
bool shared_ddc = false;
bool is_dp_bridge = false;
@@ -1574,17 +1575,21 @@ amdgpu_connector_add(struct amdgpu_device *adev,
amdgpu_connector->con_priv = amdgpu_dig_connector;
if (i2c_bus->valid) {
amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, 
i2c_bus);
-   if (amdgpu_connector->ddc_bus)
+   if (amdgpu_connector->ddc_bus) {
has_aux = true;
-   else
+   ddc = &amdgpu_connector->ddc_bus->adapter;
+   } else {
DRM_ERROR("DP: Failed to assign ddc bus! Check 
dmesg for i2c errors.\n");
+   }
}
switch (connector_type) {
case DRM_MODE_CONNECTOR_VGA:
case DRM_MODE_CONNECTOR_DVIA:
default:
-   drm_connector_init(dev, &amdgpu_connector->base,
-  &amdgpu_connector_dp_funcs, 
connector_type);
+   drm_connector_init_with_ddc(dev, 
&amdgpu_connector->base,
+   &amdgpu_connector_dp_funcs,
+   connector_type,
+   ddc);
drm_connector_helper_add(&amdgpu_connector->base,
 
&amdgpu_connector_dp_helper_funcs);
connector->interlace_allowed = true;
@@ -1602,8 +1607,10 @@ amdgpu_connector_add(struct amdgpu_device *adev,
case DRM_MODE_CONNECTOR_HDMIA:
case DRM_MODE_CONNECTOR_HDMIB:
case DRM_MODE_CONNECTOR_DisplayPort:
-   drm_connector_init(dev, &amdgpu_connector->base,
-  &amdgpu_connector_dp_funcs, 
connector_type);
+   drm_connector_init_with_ddc(dev, 
&amdgpu_connector->base,
+   &amdgpu_connector_dp_funcs,
+   connector_type,
+   ddc);
drm_connector_helper_add(&amdgpu_connector->base,
 
&amdgpu_connector_dp_helper_funcs);
drm_object_attach_property(&amdgpu_connector->base.base,
@@ -1644,8 +1651,10 @@ amdgpu_connector_add(struct amdgpu_device *adev,
break;
case DRM_MODE_CONNECTOR_LVDS:
case DRM_MODE_CONNECTOR_eDP:
-   drm_connector_init(dev, &amdgpu_connector->base,
-  &amdgpu_connector_edp_funcs, 
connector_type);
+   drm_connector_init_with_ddc(dev, 
&amdgpu_connector->base,
+   &amdgpu_connector_edp_funcs,
+   connector_type,
+   ddc);
drm_connector_helper_add(&amdgpu_connector->base,
 
&amdgpu_connector_dp_helper_funcs);
drm_object_attach_property(&amdgpu_connector->base.base,
@@ -1659,13 +1668,18 @@ amdgpu_connector_add(struct amdgpu_device *adev,
} else {
switch (connector_type) {
case DRM_MODE_CONNECTOR_VGA:
-   drm_connector_init(dev, &amdgpu_connector->base, 
&amdgpu_connector_vga_funcs, connector_type);
-   drm_connector_helper_add(&amdgpu_connector->base, 
&amdgpu_connector_vga_helper_funcs);
if (i2c_bus->valid) {
amdgpu_connector->ddc_bus = 
amdgpu_i2c_lookup(adev, i2c_bus);
if (!amdgpu_connector->ddc_bus)
DRM_ERROR("VGA: Failed to assign ddc 
bus! Check dmesg for i2c errors.\n");
+   else
+

[PATCH v6 18/24] drm/ast: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
Reviewed-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ast/ast_mode.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index c792362024a5..1c899a6e87b7 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -867,7 +867,14 @@ static int ast_connector_init(struct drm_device *dev)
return -ENOMEM;
 
connector = &ast_connector->base;
-   drm_connector_init(dev, connector, &ast_connector_funcs, 
DRM_MODE_CONNECTOR_VGA);
+   ast_connector->i2c = ast_i2c_create(dev);
+   if (!ast_connector->i2c)
+   DRM_ERROR("failed to add ddc bus for connector\n");
+
+   drm_connector_init_with_ddc(dev, connector,
+   &ast_connector_funcs,
+   DRM_MODE_CONNECTOR_VGA,
+   &ast_connector->i2c->adapter);
 
drm_connector_helper_add(connector, &ast_connector_helper_funcs);
 
@@ -881,10 +888,6 @@ static int ast_connector_init(struct drm_device *dev)
encoder = list_first_entry(&dev->mode_config.encoder_list, struct 
drm_encoder, head);
drm_connector_attach_encoder(connector, encoder);
 
-   ast_connector->i2c = ast_i2c_create(dev);
-   if (!ast_connector->i2c)
-   DRM_ERROR("failed to add ddc bus for connector\n");
-
return 0;
 }
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 19/24] drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c 
b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index d32885b906ae..8ef6539ae78a 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -111,8 +111,10 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
 
drm_connector_helper_add(&vga->connector,
 &dumb_vga_con_helper_funcs);
-   ret = drm_connector_init(bridge->dev, &vga->connector,
-&dumb_vga_con_funcs, DRM_MODE_CONNECTOR_VGA);
+   ret = drm_connector_init_with_ddc(bridge->dev, &vga->connector,
+ &dumb_vga_con_funcs,
+ DRM_MODE_CONNECTOR_VGA,
+ vga->ddc);
if (ret) {
DRM_ERROR("Failed to initialize connector\n");
return ret;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 17/24] drm/mgag200: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
Reviewed-by: Thomas Zimmermann 
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 822f2a13748f..5e778b5f1a10 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1678,18 +1678,19 @@ static struct drm_connector *mga_vga_init(struct 
drm_device *dev)
return NULL;
 
connector = &mga_connector->base;
+   mga_connector->i2c = mgag200_i2c_create(dev);
+   if (!mga_connector->i2c)
+   DRM_ERROR("failed to add ddc bus\n");
 
-   drm_connector_init(dev, connector,
-  &mga_vga_connector_funcs, DRM_MODE_CONNECTOR_VGA);
+   drm_connector_init_with_ddc(dev, connector,
+   &mga_vga_connector_funcs,
+   DRM_MODE_CONNECTOR_VGA,
+   &mga_connector->i2c->adapter);
 
drm_connector_helper_add(connector, &mga_vga_connector_helper_funcs);
 
drm_connector_register(connector);
 
-   mga_connector->i2c = mgag200_i2c_create(dev);
-   if (!mga_connector->i2c)
-   DRM_ERROR("failed to add ddc bus\n");
-
return connector;
 }
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 16/24] drm: sti: Provide ddc symlink in hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/sti/sti_hdmi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index f03d617edc4c..33d06e0a9168 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -1284,8 +1284,10 @@ static int sti_hdmi_bind(struct device *dev, struct 
device *master, void *data)
 
drm_connector->polled = DRM_CONNECTOR_POLL_HPD;
 
-   drm_connector_init(drm_dev, drm_connector,
-   &sti_hdmi_connector_funcs, DRM_MODE_CONNECTOR_HDMIA);
+   drm_connector_init_with_ddc(drm_dev, drm_connector,
+   &sti_hdmi_connector_funcs,
+   DRM_MODE_CONNECTOR_HDMIA,
+   hdmi->ddc_adapt);
drm_connector_helper_add(drm_connector,
&sti_hdmi_connector_helper_funcs);
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 15/24] drm/tilcdc: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c 
b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
index c6e4e52f32bc..d51776dd7a03 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
@@ -222,8 +222,10 @@ static struct drm_connector 
*tfp410_connector_create(struct drm_device *dev,
 
connector = &tfp410_connector->base;
 
-   drm_connector_init(dev, connector, &tfp410_connector_funcs,
-   DRM_MODE_CONNECTOR_DVID);
+   drm_connector_init_with_ddc(dev, connector,
+   &tfp410_connector_funcs,
+   DRM_MODE_CONNECTOR_DVID,
+   mod->i2c);
drm_connector_helper_add(connector, &tfp410_connector_helper_funcs);
 
connector->polled = DRM_CONNECTOR_POLL_CONNECT |
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 14/24] drm: zte: Provide ddc symlink in vga connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/zte/zx_vga.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/zte/zx_vga.c b/drivers/gpu/drm/zte/zx_vga.c
index 9b67e419280c..c4fa3bbaba78 100644
--- a/drivers/gpu/drm/zte/zx_vga.c
+++ b/drivers/gpu/drm/zte/zx_vga.c
@@ -165,8 +165,10 @@ static int zx_vga_register(struct drm_device *drm, struct 
zx_vga *vga)
 
vga->connector.polled = DRM_CONNECTOR_POLL_HPD;
 
-   ret = drm_connector_init(drm, connector, &zx_vga_connector_funcs,
-DRM_MODE_CONNECTOR_VGA);
+   ret = drm_connector_init_with_ddc(drm, connector,
+ &zx_vga_connector_funcs,
+ DRM_MODE_CONNECTOR_VGA,
+ &vga->ddc->adap);
if (ret) {
DRM_DEV_ERROR(dev, "failed to init connector: %d\n", ret);
goto clean_encoder;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 13/24] drm: zte: Provide ddc symlink in hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/zte/zx_hdmi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
index a50f5a1f09b8..b98a1420dcd3 100644
--- a/drivers/gpu/drm/zte/zx_hdmi.c
+++ b/drivers/gpu/drm/zte/zx_hdmi.c
@@ -319,8 +319,10 @@ static int zx_hdmi_register(struct drm_device *drm, struct 
zx_hdmi *hdmi)
 
hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD;
 
-   drm_connector_init(drm, &hdmi->connector, &zx_hdmi_connector_funcs,
-  DRM_MODE_CONNECTOR_HDMIA);
+   drm_connector_init_with_ddc(drm, &hdmi->connector,
+   &zx_hdmi_connector_funcs,
+   DRM_MODE_CONNECTOR_HDMIA,
+   &hdmi->ddc->adap);
drm_connector_helper_add(&hdmi->connector,
 &zx_hdmi_connector_helper_funcs);
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 12/24] drm/vc4: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index ee7d4e7b0ee3..eb57c907a256 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -267,7 +267,8 @@ static const struct drm_connector_helper_funcs 
vc4_hdmi_connector_helper_funcs =
 };
 
 static struct drm_connector *vc4_hdmi_connector_init(struct drm_device *dev,
-struct drm_encoder 
*encoder)
+struct drm_encoder 
*encoder,
+struct i2c_adapter *ddc)
 {
struct drm_connector *connector;
struct vc4_hdmi_connector *hdmi_connector;
@@ -281,8 +282,10 @@ static struct drm_connector 
*vc4_hdmi_connector_init(struct drm_device *dev,
 
hdmi_connector->encoder = encoder;
 
-   drm_connector_init(dev, connector, &vc4_hdmi_connector_funcs,
-  DRM_MODE_CONNECTOR_HDMIA);
+   drm_connector_init_with_ddc(dev, connector,
+   &vc4_hdmi_connector_funcs,
+   DRM_MODE_CONNECTOR_HDMIA,
+   ddc);
drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
 
/* Create and attach TV margin props to this connector. */
@@ -1395,7 +1398,8 @@ static int vc4_hdmi_bind(struct device *dev, struct 
device *master, void *data)
 DRM_MODE_ENCODER_TMDS, NULL);
drm_encoder_helper_add(hdmi->encoder, &vc4_hdmi_encoder_helper_funcs);
 
-   hdmi->connector = vc4_hdmi_connector_init(drm, hdmi->encoder);
+   hdmi->connector =
+   vc4_hdmi_connector_init(drm, hdmi->encoder, hdmi->ddc);
if (IS_ERR(hdmi->connector)) {
ret = PTR_ERR(hdmi->connector);
goto err_destroy_encoder;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 11/24] drm/imx: imx-tve: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/imx/imx-tve.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index 649515868f86..5bbfaa2cd0f4 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -484,8 +484,10 @@ static int imx_tve_register(struct drm_device *drm, struct 
imx_tve *tve)
 
drm_connector_helper_add(&tve->connector,
&imx_tve_connector_helper_funcs);
-   drm_connector_init(drm, &tve->connector, &imx_tve_connector_funcs,
-  DRM_MODE_CONNECTOR_VGA);
+   drm_connector_init_with_ddc(drm, &tve->connector,
+   &imx_tve_connector_funcs,
+   DRM_MODE_CONNECTOR_VGA,
+   tve->ddc);
 
drm_connector_attach_encoder(&tve->connector, &tve->encoder);
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 08/24] drm/mediatek: Provide ddc symlink in hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/mediatek/mtk_hdmi.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c 
b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index ce91b61364eb..f419765b7cc0 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1299,9 +1299,10 @@ static int mtk_hdmi_bridge_attach(struct drm_bridge 
*bridge)
struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge);
int ret;
 
-   ret = drm_connector_init(bridge->encoder->dev, &hdmi->conn,
-&mtk_hdmi_connector_funcs,
-DRM_MODE_CONNECTOR_HDMIA);
+   ret = drm_connector_init_with_ddc(bridge->encoder->dev, &hdmi->conn,
+ &mtk_hdmi_connector_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ hdmi->ddc_adpt);
if (ret) {
dev_err(hdmi->dev, "Failed to initialize connector: %d\n", ret);
return ret;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 10/24] drm/imx: imx-ldb: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/imx/imx-ldb.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index de62a4cd4827..db461b6a257f 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -462,9 +462,10 @@ static int imx_ldb_register(struct drm_device *drm,
 */
drm_connector_helper_add(&imx_ldb_ch->connector,
&imx_ldb_connector_helper_funcs);
-   drm_connector_init(drm, &imx_ldb_ch->connector,
-   &imx_ldb_connector_funcs,
-   DRM_MODE_CONNECTOR_LVDS);
+   drm_connector_init_with_ddc(drm, &imx_ldb_ch->connector,
+   &imx_ldb_connector_funcs,
+   DRM_MODE_CONNECTOR_LVDS,
+   imx_ldb_ch->ddc);
drm_connector_attach_encoder(&imx_ldb_ch->connector, encoder);
}
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 05/24] drm: rockchip: Provide ddc symlink in inno_hdmi sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/rockchip/inno_hdmi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c 
b/drivers/gpu/drm/rockchip/inno_hdmi.c
index ed344a795b4d..e5864e823020 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -624,8 +624,10 @@ static int inno_hdmi_register(struct drm_device *drm, 
struct inno_hdmi *hdmi)
 
drm_connector_helper_add(&hdmi->connector,
 &inno_hdmi_connector_helper_funcs);
-   drm_connector_init(drm, &hdmi->connector, &inno_hdmi_connector_funcs,
-  DRM_MODE_CONNECTOR_HDMIA);
+   drm_connector_init_with_ddc(drm, &hdmi->connector,
+   &inno_hdmi_connector_funcs,
+   DRM_MODE_CONNECTOR_HDMIA,
+   hdmi->ddc);
 
drm_connector_attach_encoder(&hdmi->connector, encoder);
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 09/24] drm/tegra: Provide ddc symlink in output connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/tegra/hdmi.c | 7 ---
 drivers/gpu/drm/tegra/sor.c  | 7 ---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 334c4d7d238b..416a2862a84b 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -1425,9 +1425,10 @@ static int tegra_hdmi_init(struct host1x_client *client)
 
hdmi->output.dev = client->dev;
 
-   drm_connector_init(drm, &hdmi->output.connector,
-  &tegra_hdmi_connector_funcs,
-  DRM_MODE_CONNECTOR_HDMIA);
+   drm_connector_init_with_ddc(drm, &hdmi->output.connector,
+   &tegra_hdmi_connector_funcs,
+   DRM_MODE_CONNECTOR_HDMIA,
+   hdmi->output.ddc);
drm_connector_helper_add(&hdmi->output.connector,
 &tegra_hdmi_connector_helper_funcs);
hdmi->output.connector.dpms = DRM_MODE_DPMS_OFF;
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 4ffe3794e6d3..3a69e387c62d 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -2832,9 +2832,10 @@ static int tegra_sor_init(struct host1x_client *client)
 
sor->output.dev = sor->dev;
 
-   drm_connector_init(drm, &sor->output.connector,
-  &tegra_sor_connector_funcs,
-  connector);
+   drm_connector_init_with_ddc(drm, &sor->output.connector,
+   &tegra_sor_connector_funcs,
+   connector,
+   sor->output.ddc);
drm_connector_helper_add(&sor->output.connector,
 &tegra_sor_connector_helper_funcs);
sor->output.connector.dpms = DRM_MODE_DPMS_OFF;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 07/24] drm/sun4i: hdmi: Provide ddc symlink in sun4i hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c 
b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index b2df76addc75..eb8071a4d6d0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -640,9 +640,10 @@ static int sun4i_hdmi_bind(struct device *dev, struct 
device *master,
 
drm_connector_helper_add(&hdmi->connector,
 &sun4i_hdmi_connector_helper_funcs);
-   ret = drm_connector_init(drm, &hdmi->connector,
-&sun4i_hdmi_connector_funcs,
-DRM_MODE_CONNECTOR_HDMIA);
+   ret = drm_connector_init_with_ddc(drm, &hdmi->connector,
+ &sun4i_hdmi_connector_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ hdmi->ddc_i2c);
if (ret) {
dev_err(dev,
"Couldn't initialise the HDMI connector\n");
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 06/24] drm/msm/hdmi: Provide ddc symlink in hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
index 07b4cb877d82..1f03262b8a52 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -450,8 +450,10 @@ struct drm_connector *msm_hdmi_connector_init(struct hdmi 
*hdmi)
 
connector = &hdmi_connector->base;
 
-   drm_connector_init(hdmi->dev, connector, &hdmi_connector_funcs,
-   DRM_MODE_CONNECTOR_HDMIA);
+   drm_connector_init_with_ddc(hdmi->dev, connector,
+   &hdmi_connector_funcs,
+   DRM_MODE_CONNECTOR_HDMIA,
+   hdmi->i2c);
drm_connector_helper_add(connector, &msm_hdmi_connector_helper_funcs);
 
connector->polled = DRM_CONNECTOR_POLL_CONNECT |
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 03/24] drm/exynos: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Switch to using the ddc provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index bc1565f1822a..d4a9c9e17436 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -940,8 +940,10 @@ static int hdmi_create_connector(struct drm_encoder 
*encoder)
connector->interlace_allowed = true;
connector->polled = DRM_CONNECTOR_POLL_HPD;
 
-   ret = drm_connector_init(hdata->drm_dev, connector,
-   &hdmi_connector_funcs, DRM_MODE_CONNECTOR_HDMIA);
+   ret = drm_connector_init_with_ddc(hdata->drm_dev, connector,
+ &hdmi_connector_funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ hdata->ddc_adpt);
if (ret) {
DRM_DEV_ERROR(hdata->dev,
  "Failed to initialize connector with drm\n");
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 00/24] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

It is difficult for a user to know which of the i2c adapters is for which
drm connector. This series addresses this problem.

The idea is to have a symbolic link in connector's sysfs directory, e.g.:

ls -l /sys/class/drm/card0-HDMI-A-1/ddc
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
-> ../../../../soc/1388.i2c/i2c-2

The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
ddcutil:

ddcutil -b 2 getvcp 0x10
VCP code 0x10 (Brightness): current value =90, max value =   100

The first patch in the series adds struct i2c_adapter pointer to struct
drm_connector. If the field is used by a particular driver, then an
appropriate symbolic link is created by the generic code, which is also added
by this patch.

Patch 2 adds a new variant of drm_connector_init(), see the changelog
below.

Patches 3..24 are examples of how to convert a driver to this new scheme.

v1..v2:

- used fixed name "ddc" for the symbolic link in order to make it easy for
userspace to find the i2c adapter

v2..v3:

- converted as many drivers as possible.

v3..v4:

- added Reviewed-by for patch 01/23
- moved "ddc" field assignment to before drm_connector_init() is called
in msm, vc4, sti, mgag200, ast, amdgpu, radeon
- simplified the code in amdgpu and radeon at the expense of some lines
exceeding 80 characters as per Alex Deucher's suggestion
- added i915

v4..v5:

- changed "include " to "struct i2c_adapter;"
in drm_connector.h, consequently, added "include "
in drm_sysfs.c.
- added "drm_connector_init_with_ddc()" variant to ensure that the ddc
field of drm_connector is preserved accross its invocation
- accordingly changed invocations of drm_connector_init() in the
touched drivers to use the new variant

v5..v6:

- improved subject line of patch 1
- added kernel-doc for drm_connector_init_with_ddc()
- improved kernel-doc for the ddc field of struct drm_connector
- added Reviewed-by in patches 17 and 18
- added Acked-by in patch 2
- made the ownership of ddc i2c_adapter explicit in all patches,
this made the affected patches much simpler

@Benjamin
@Shawn

There were your Acked-by or Reviewed-by for some patches in v4, but now
that the patches use the newly added function I'm not sure I can still
include those tags without you actually confirming. Can I? Or can you
please re-review? 

TODO: nouveau, gma500, omapdrm, panel-simple - if applicable.
Other drivers are either already converted or don't mention neither
"ddc" nor "i2c_adapter".

Andrzej Pietrasiewicz (24):
  drm: Add ddc link in sysfs created by drm_connector
  drm: Add drm_connector_init() variant with ddc
  drm/exynos: Provide ddc symlink in connector's sysfs
  drm: rockchip: Provide ddc symlink in rk3066_hdmi sysfs directory
  drm: rockchip: Provide ddc symlink in inno_hdmi sysfs directory
  drm/msm/hdmi: Provide ddc symlink in hdmi connector sysfs directory
  drm/sun4i: hdmi: Provide ddc symlink in sun4i hdmi connector sysfs
directory
  drm/mediatek: Provide ddc symlink in hdmi connector sysfs directory
  drm/tegra: Provide ddc symlink in output connector sysfs directory
  drm/imx: imx-ldb: Provide ddc symlink in connector's sysfs
  drm/imx: imx-tve: Provide ddc symlink in connector's sysfs
  drm/vc4: Provide ddc symlink in connector sysfs directory
  drm: zte: Provide ddc symlink in hdmi connector sysfs directory
  drm: zte: Provide ddc symlink in vga connector sysfs directory
  drm/tilcdc: Provide ddc symlink in connector sysfs directory
  drm: sti: Provide ddc symlink in hdmi connector sysfs directory
  drm/mgag200: Provide ddc symlink in connector sysfs directory
  drm/ast: Provide ddc symlink in connector sysfs directory
  drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs
directory
  drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory
  drm/bridge: ti-tfp410: Provide ddc symlink in connector sysfs
directory
  drm/amdgpu: Provide ddc symlink in connector sysfs directory
  drm/radeon: Provide ddc symlink in connector sysfs directory
  drm/i915: Provide ddc symlink in hdmi connector sysfs directory

 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c|  96 
 drivers/gpu/drm/ast/ast_mode.c|  13 +-
 drivers/gpu/drm/bridge/dumb-vga-dac.c |   6 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |   6 +-
 drivers/gpu/drm/bridge/ti-tfp410.c|   6 +-
 drivers/gpu/drm/drm_connector.c   |  35 +
 drivers/gpu/drm/drm_sysfs.c   |   8 +
 drivers/gpu/drm/exynos/exynos_hdmi.c  |   6 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c |  12 +-
 drivers/gpu/drm/imx/imx-ldb.c |   7 +-
 drivers/gpu/drm/imx/imx-tve.c |   6 +-
 drivers/gpu/drm/mediatek/mtk_hdmi.c   |   7 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c|  13 +-
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c |   6 +-
 drivers/gpu/drm/radeon/radeon_connectors.c| 142 +-
 drivers/gpu/drm/roc

[PATCH v6 04/24] drm: rockchip: Provide ddc symlink in rk3066_hdmi sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/gpu/drm/rockchip/rk3066_hdmi.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c 
b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index 85fc5f01f761..e874f5fdeec4 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -564,9 +564,10 @@ rk3066_hdmi_register(struct drm_device *drm, struct 
rk3066_hdmi *hdmi)
 
drm_connector_helper_add(&hdmi->connector,
 &rk3066_hdmi_connector_helper_funcs);
-   drm_connector_init(drm, &hdmi->connector,
-  &rk3066_hdmi_connector_funcs,
-  DRM_MODE_CONNECTOR_HDMIA);
+   drm_connector_init_with_ddc(drm, &hdmi->connector,
+   &rk3066_hdmi_connector_funcs,
+   DRM_MODE_CONNECTOR_HDMIA,
+   hdmi->ddc);
 
drm_connector_attach_encoder(&hdmi->connector, encoder);
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 01/24] drm: Add ddc link in sysfs created by drm_connector

[Andrzej Pietrasiewicz]

Add generic code which creates symbolic links in sysfs, pointing to ddc
interface used by a particular video output. For example:

ls -l /sys/class/drm/card0-HDMI-A-1/ddc
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
-> ../../../../soc/1388.i2c/i2c-2

This makes it easy for user to associate a display with its ddc adapter
and use e.g. ddcutil to control the chosen monitor.

This patch adds an i2c_adapter pointer to struct drm_connector. Particular
drivers can then use it instead of using their own private instance. If a
connector contains a ddc, then create a symbolic link in sysfs.

Signed-off-by: Andrzej Pietrasiewicz 
Acked-by: Daniel Vetter 
Reviewed-by: Andrzej Hajda 
---
 drivers/gpu/drm/drm_sysfs.c |  8 
 include/drm/drm_connector.h | 11 +++
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ad10810bc972..e962a9d45f7e 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -294,6 +295,9 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
/* Let userspace know we have a new connector */
drm_sysfs_hotplug_event(dev);
 
+   if (connector->ddc)
+   return sysfs_create_link(&connector->kdev->kobj,
+&connector->ddc->dev.kobj, "ddc");
return 0;
 }
 
@@ -301,6 +305,10 @@ void drm_sysfs_connector_remove(struct drm_connector 
*connector)
 {
if (!connector->kdev)
return;
+
+   if (connector->ddc)
+   sysfs_remove_link(&connector->kdev->kobj, "ddc");
+
DRM_DEBUG("removing \"%s\" from sysfs\n",
  connector->name);
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 4c30d751487a..33a6fff85fdb 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -41,6 +41,7 @@ struct drm_property;
 struct drm_property_blob;
 struct drm_printer;
 struct edid;
+struct i2c_adapter;
 
 enum drm_connector_force {
DRM_FORCE_UNSPECIFIED,
@@ -1311,6 +1312,16 @@ struct drm_connector {
 * [0]: progressive, [1]: interlaced
 */
int audio_latency[2];
+
+   /**
+* @ddc: associated ddc adapter.
+* A connector usually has its associated ddc adapter. If a driver uses
+* this field, then an appropriate symbolic link is created in connector
+* sysfs directory to make it easy for the user to tell which i2c
+* adapter is for a particular display.
+*/
+   struct i2c_adapter *ddc;
+
/**
 * @null_edid_counter: track sinks that give us all zeros for the EDID.
 * Needed to workaround some HW bugs where we get all 0s
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v6 02/24] drm: Add drm_connector_init() variant with ddc

[Andrzej Pietrasiewicz]

Allow passing ddc adapter pointer to the init function. Even if
drm_connector_init() sometime in the future decides to e.g. memset() all
connector fields to zeros, the newly added function ensures that at its
completion the ddc member of connector is correctly set.

Signed-off-by: Andrzej Pietrasiewicz 
Acked-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_connector.c | 35 +
 include/drm/drm_connector.h |  7 +++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index cbb548b3708f..d49e19f3de3a 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -297,6 +297,41 @@ int drm_connector_init(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_connector_init);
 
+/**
+ * drm_connector_init_with_ddc - Init a preallocated connector
+ * @dev: DRM device
+ * @connector: the connector to init
+ * @funcs: callbacks for this connector
+ * @connector_type: user visible type of the connector
+ * @ddc: pointer to the associated ddc adapter
+ *
+ * Initialises a preallocated connector. Connectors should be
+ * subclassed as part of driver connector objects.
+ *
+ * Ensures that the ddc field of the connector is correctly set.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_connector_init_with_ddc(struct drm_device *dev,
+   struct drm_connector *connector,
+   const struct drm_connector_funcs *funcs,
+   int connector_type,
+   struct i2c_adapter *ddc)
+{
+   int ret;
+
+   ret = drm_connector_init(dev, connector, funcs, connector_type);
+   if (ret)
+   return ret;
+
+   /* provide ddc symlink in sysfs */
+   connector->ddc = ddc;
+
+   return ret;
+}
+EXPORT_SYMBOL(drm_connector_init_with_ddc);
+
 /**
  * drm_connector_attach_edid_property - attach edid property.
  * @connector: the connector
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 33a6fff85fdb..fc5d08438333 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1319,6 +1319,8 @@ struct drm_connector {
 * this field, then an appropriate symbolic link is created in connector
 * sysfs directory to make it easy for the user to tell which i2c
 * adapter is for a particular display.
+*
+* The field should be set by calling drm_connector_init_with_ddc().
 */
struct i2c_adapter *ddc;
 
@@ -1410,6 +1412,11 @@ int drm_connector_init(struct drm_device *dev,
   struct drm_connector *connector,
   const struct drm_connector_funcs *funcs,
   int connector_type);
+int drm_connector_init_with_ddc(struct drm_device *dev,
+   struct drm_connector *connector,
+   const struct drm_connector_funcs *funcs,
+   int connector_type,
+   struct i2c_adapter *ddc);
 void drm_connector_attach_edid_property(struct drm_connector *connector);
 int drm_connector_register(struct drm_connector *connector);
 void drm_connector_unregister(struct drm_connector *connector);
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 2/7] mm/hmm: a few more C style and comment clean ups

[Ralph Campbell]

On 7/25/19 11:23 PM, Christoph Hellwig wrote:

Note: it seems like you've only CCed me on patches 2-7, but not on the
cover letter and patch 1.  I'll try to find them later, but to make Ccs
useful they should normally cover the whole series.

Otherwise this looks fine to me:

Reviewed-by: Christoph Hellwig 



Thanks for the review and sorry about the oversight on CCs.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu/display: fix the build without CONFIG_DRM_AMD_DC_DSC_SUPPORT

[Kazlauskas, Nicholas]

On 7/26/19 12:09 PM, Alex Deucher wrote:
> Some code was missing the CONFIG_DRM_AMD_DC_DSC_SUPPORT guard.
> 
> Signed-off-by: Alex Deucher 

Reviewed-by: Nicholas Kazlauskas 

We probably want to drop this and the DCN2 guard eventually though.

Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c 
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> index 69e4d0d96c7f..38b3c89b2a59 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> @@ -630,9 +630,11 @@ static void dcn20_init_hw(struct dc *dc)
>   }
>   }
>   
> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
>   /* Power gate DSCs */
>   for (i = 0; i < res_pool->res_cap->num_dsc; i++)
>   dcn20_dsc_pg_control(hws, res_pool->dscs[i]->inst, false);
> +#endif
>   
>   /* Blank pixel data with OPP DPG */
>   for (i = 0; i < dc->res_pool->timing_generator_count; i++) {
> 

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v5 2/4] drm/amdgpu: Create helper to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC

[Michel Dänzer]

On 2019-07-26 6:02 p.m., Christian König wrote:
> Am 26.07.19 um 16:53 schrieb Michel Dänzer:
>> On 2019-07-26 1:55 p.m., Christian König wrote:
>>> Am 26.07.19 um 10:54 schrieb Michel Dänzer:
 On 2019-07-26 9:11 a.m., Christian König wrote:
> Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:
>> Move the logic to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC in
>> amdgpu_bo_do_create into standalone helper so it can be reused
>> in other functions.
>>
>> v4:
>> Switch to return bool.
>>
>> v5: Fix typos.
>>
>> Signed-off-by: Andrey Grodzovsky 
>> ---
>>     drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 61
>> +-
>>     drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +
>>     2 files changed, 37 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 989b7b5..8702062 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -413,6 +413,40 @@ static bool amdgpu_bo_validate_size(struct
>> amdgpu_device *adev,
>>     return false;
>>     }
>>     +bool amdgpu_bo_support_uswc(u64 bo_flags)
>> +{
>> +
>> +#ifdef CONFIG_X86_32
>> +    /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
>> + * See https://bugs.freedesktop.org/show_bug.cgi?id=84627
>> + */
>> +    return false;
>> +#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
>> +    /* Don't try to enable write-combining when it can't work, or
>> things
>> + * may be slow
>> + * See https://bugs.freedesktop.org/show_bug.cgi?id=88758
>> + */
>> +
>> +#ifndef CONFIG_COMPILE_TEST
>> +#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better
>> performance \
>> + thanks to write-combining
>> +#endif
>> +
>> +    if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
>> +    DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT
>> for "
>> +  "better performance thanks to write-combining\n");
> I don't think this message belongs here.
>
> [...]
>> @@ -466,33 +500,8 @@ static int amdgpu_bo_do_create(struct
>> [...]
>> +    if (!amdgpu_bo_support_uswc(bo->flags))
>>     bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> Rather here we should do "if (bo_flags &
> AMDGPU_GEM_CREATE_CPU_GTT_USWC
> && !amdgpu_bo_support_uswc())" and then clear the flag and also print
> the warning.
 That would require duplicating the CONFIG_X86_PAT related logic here as
 well, which is a bit ugly.
>>> Actually I would say we should drop this extra check and always emit a
>>> message that USWC is disabled for this platform.
>>>
>>> We now need it for more than just better performance and it should be
>>> explicitly noted that this is not available in the logs.
>> A log message which doesn't explain why it's disabled / how to enable it
>> would probably cause us user support pain.
> 
> Mhm, sounds like you didn't got what I wanted to say.
> 
> No log message was fine as long as USWC was only a performance
> optimization, but now it becomes mandatory for correct operation in some
> settings.
> 
> In other words in very low VRAM configurations it can be that we can't
> enable higher resolution because the kernel is not compiled with the
> necessary flags for USWC support.

With an APU which supports scatter/gather scanout, sure.

> Printing that when the driver loads sounds like the best place to me.

Works for me, but it still needs to explain why it's disabled / how to
enable it... Something like "enable PAT" or "use a 64-bit kernel".


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu/display: fix the build without CONFIG_DRM_AMD_DC_DSC_SUPPORT

[Alex Deucher]

Some code was missing the CONFIG_DRM_AMD_DC_DSC_SUPPORT guard.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
index 69e4d0d96c7f..38b3c89b2a59 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
@@ -630,9 +630,11 @@ static void dcn20_init_hw(struct dc *dc)
}
}
 
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
/* Power gate DSCs */
for (i = 0; i < res_pool->res_cap->num_dsc; i++)
dcn20_dsc_pg_control(hws, res_pool->dscs[i]->inst, false);
+#endif
 
/* Blank pixel data with OPP DPG */
for (i = 0; i < dc->res_pool->timing_generator_count; i++) {
-- 
2.20.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v5 2/4] drm/amdgpu: Create helper to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC

[Christian König]

Am 26.07.19 um 16:53 schrieb Michel Dänzer:

On 2019-07-26 1:55 p.m., Christian König wrote:

Am 26.07.19 um 10:54 schrieb Michel Dänzer:

On 2019-07-26 9:11 a.m., Christian König wrote:

Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:

Move the logic to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC in
amdgpu_bo_do_create into standalone helper so it can be reused
in other functions.

v4:
Switch to return bool.

v5: Fix typos.

Signed-off-by: Andrey Grodzovsky 
---
    drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 61
+-
    drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +
    2 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 989b7b5..8702062 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -413,6 +413,40 @@ static bool amdgpu_bo_validate_size(struct
amdgpu_device *adev,
    return false;
    }
    +bool amdgpu_bo_support_uswc(u64 bo_flags)
+{
+
+#ifdef CONFIG_X86_32
+    /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
+ * See https://bugs.freedesktop.org/show_bug.cgi?id=84627
+ */
+    return false;
+#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
+    /* Don't try to enable write-combining when it can't work, or
things
+ * may be slow
+ * See https://bugs.freedesktop.org/show_bug.cgi?id=88758
+ */
+
+#ifndef CONFIG_COMPILE_TEST
+#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better
performance \
+ thanks to write-combining
+#endif
+
+    if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
+    DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT
for "
+  "better performance thanks to write-combining\n");

I don't think this message belongs here.

[...]

@@ -466,33 +500,8 @@ static int amdgpu_bo_do_create(struct
[...]
+    if (!amdgpu_bo_support_uswc(bo->flags))
    bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;

Rather here we should do "if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC
&& !amdgpu_bo_support_uswc())" and then clear the flag and also print
the warning.

That would require duplicating the CONFIG_X86_PAT related logic here as
well, which is a bit ugly.

Actually I would say we should drop this extra check and always emit a
message that USWC is disabled for this platform.

We now need it for more than just better performance and it should be
explicitly noted that this is not available in the logs.

A log message which doesn't explain why it's disabled / how to enable it
would probably cause us user support pain.


Mhm, sounds like you didn't got what I wanted to say.

No log message was fine as long as USWC was only a performance 
optimization, but now it becomes mandatory for correct operation in some 
settings.


In other words in very low VRAM configurations it can be that we can't 
enable higher resolution because the kernel is not compiled with the 
necessary flags for USWC support.


Printing that when the driver loads sounds like the best place to me.

Regards,
Christian.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 5/7] mm/hmm: make full use of walk_page_range()

[Jason Gunthorpe]

On Thu, Jul 25, 2019 at 05:56:48PM -0700, Ralph Campbell wrote:
> hmm_range_fault() calls find_vma() and walk_page_range() in a loop.
> This is unnecessary duplication since walk_page_range() calls find_vma()
> in a loop already.
> Simplify hmm_range_fault() by defining a walk_test() callback function
> to filter unhandled vmas.
> 
> Signed-off-by: Ralph Campbell 
> Cc: "Jérôme Glisse" 
> Cc: Jason Gunthorpe 
> Cc: Christoph Hellwig 
>  mm/hmm.c | 130 ---
>  1 file changed, 57 insertions(+), 73 deletions(-)
> 
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 1bc014cddd78..838cd1d50497 100644
> +++ b/mm/hmm.c
> @@ -840,13 +840,44 @@ static int hmm_vma_walk_hugetlb_entry(pte_t *pte, 
> unsigned long hmask,
>  #endif
>  }
>  
> -static void hmm_pfns_clear(struct hmm_range *range,
> -uint64_t *pfns,
> -unsigned long addr,
> -unsigned long end)
> +static int hmm_vma_walk_test(unsigned long start,
> +  unsigned long end,
> +  struct mm_walk *walk)
>  {
> - for (; addr < end; addr += PAGE_SIZE, pfns++)
> - *pfns = range->values[HMM_PFN_NONE];
> + struct hmm_vma_walk *hmm_vma_walk = walk->private;
> + struct hmm_range *range = hmm_vma_walk->range;
> + struct vm_area_struct *vma = walk->vma;
> +
> + /* If range is no longer valid, force retry. */
> + if (!range->valid)
> + return -EBUSY;
> +
> + /*
> +  * Skip vma ranges that don't have struct page backing them or
> +  * map I/O devices directly.
> +  * TODO: handle peer-to-peer device mappings.
> +  */
> + if (vma->vm_flags & (VM_IO | VM_PFNMAP | VM_MIXEDMAP))
> + return -EFAULT;
> +
> + if (is_vm_hugetlb_page(vma)) {
> + if (huge_page_shift(hstate_vma(vma)) != range->page_shift &&
> + range->page_shift != PAGE_SHIFT)
> + return -EINVAL;
> + } else {
> + if (range->page_shift != PAGE_SHIFT)
> + return -EINVAL;
> + }
> +
> + /*
> +  * If vma does not allow read access, then assume that it does not
> +  * allow write access, either. HMM does not support architectures
> +  * that allow write without read.
> +  */
> + if (!(vma->vm_flags & VM_READ))
> + return -EPERM;
> +
> + return 0;
>  }
>  
>  /*
> @@ -965,82 +996,35 @@ EXPORT_SYMBOL(hmm_range_unregister);
>   */
>  long hmm_range_fault(struct hmm_range *range, unsigned int flags)
>  {
> - const unsigned long device_vma = VM_IO | VM_PFNMAP | VM_MIXEDMAP;
> - unsigned long start = range->start, end;
> - struct hmm_vma_walk hmm_vma_walk;
> + unsigned long start = range->start;
> + struct hmm_vma_walk hmm_vma_walk = {};
>   struct hmm *hmm = range->hmm;
> - struct vm_area_struct *vma;
> - struct mm_walk mm_walk;
> + struct mm_walk mm_walk = {};
>   int ret;
>  
>   lockdep_assert_held(&hmm->mm->mmap_sem);
>  
> - do {
> - /* If range is no longer valid force retry. */
> - if (!range->valid)
> - return -EBUSY;
> + hmm_vma_walk.range = range;
> + hmm_vma_walk.last = start;
> + hmm_vma_walk.flags = flags;
> + mm_walk.private = &hmm_vma_walk;
>  
> - vma = find_vma(hmm->mm, start);
> - if (vma == NULL || (vma->vm_flags & device_vma))
> - return -EFAULT;

It is hard to tell what is a confused/wrong and what is deliberate in
this code...

Currently the hmm_range_fault invokes walk_page_range on a VMA by VMA
basis, and the above prevents some cases of walk->vma becoming
NULL, but not all - for instance it doesn't check for start < vma->vm_start.

However, checking if it can actually tolerate the walk->vma == NULL it
looks like no:

 walk_page_range
  find_vma == NULL || start < vm_start -> walk->vma == NULL
  __walk_page_range
walk_pgd_range
  pte_hole / hmm_vma_walk_hole
hmm_vma_walk_hole_
 hmm_vma_do_fault
handle_mm_fault(walk->vma, addr, flags)
  vma->vm_mm <-- OOPS

Which kind of suggests the find_vma above was about preventing
walk->vma == NULL? Does something else tricky prevent this?

This patch also changes behavior so that missing VMAs don't always
trigger EFAULT (which is a good thing, but needs to be in the commit
message)

I strongly believe this is the correct direction to go in, and the fact
that this function returns EFAULT if there is no VMA/incompatible VMA
is actually a semantic bug we need to fix before it is a usable API.

Ie consider the user does something like
  ptr = mmap(0, PAGE_SIZE ..)
  mr = ib_reg_mr(ptr - PAGE_SIZE, ptr + 3*PAGE_SIZE, IBV_ACCESS_ON_DEMAND)

Then in the kernel I want to do hmm_range_fault(HMM_FAULT_SNAPSHOT)
across the MR VA and get a pfns array that says PAGE 0 is FAULT, PAGE
1 is R/W, PAGE 2 is FAULT.

Ins

[PATCH] drm/amdkfd: remove set but not used variable 'pdd'

[YueHaibing]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_process.c: In function 
restore_process_worker:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_process.c:949:29: warning:
 variable pdd set but not used [-Wunused-but-set-variable]

It is not used since
commit 5b87245faf57 ("drm/amdkfd: Simplify kfd2kgd interface")

Reported-by: Hulk Robot 
Signed-off-by: YueHaibing 
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 8f1076c..240bf68 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1042,7 +1042,6 @@ static void restore_process_worker(struct work_struct 
*work)
 {
struct delayed_work *dwork;
struct kfd_process *p;
-   struct kfd_process_device *pdd;
int ret = 0;
 
dwork = to_delayed_work(work);
@@ -1051,16 +1050,6 @@ static void restore_process_worker(struct work_struct 
*work)
 * lifetime of this thread, kfd_process p will be valid
 */
p = container_of(dwork, struct kfd_process, restore_work);
-
-   /* Call restore_process_bos on the first KGD device. This function
-* takes care of restoring the whole process including other devices.
-* Restore can fail if enough memory is not available. If so,
-* reschedule again.
-*/
-   pdd = list_first_entry(&p->per_device_data,
-  struct kfd_process_device,
-  per_device_list);
-
pr_debug("Started restoring pasid %d\n", p->pasid);
 
/* Setting last_restore_timestamp before successful restoration.
-- 
2.7.4


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 0/7] mm/hmm: more HMM clean up

[Jason Gunthorpe]

On Thu, Jul 25, 2019 at 05:56:43PM -0700, Ralph Campbell wrote:
> Here are seven more patches for things I found to clean up.
> This was based on top of Christoph's seven patches:
> "hmm_range_fault related fixes and legacy API removal v3".
> I assume this will go into Jason's tree since there will likely be
> more HMM changes in this cycle.
>
> Changes from v1 to v2:
> 
> Added AMD GPU to hmm_update removal.
> Added 2 patches from Christoph.
> Added 2 patches as a result of Jason's suggestions.
> 
> Christoph Hellwig (2):
>   mm/hmm: replace the block argument to hmm_range_fault with a flags
> value
>   mm: merge hmm_range_snapshot into hmm_range_fault
> 
> Ralph Campbell (5):
>   mm/hmm: replace hmm_update with mmu_notifier_range
>   mm/hmm: a few more C style and comment clean ups
>   mm/hmm: remove hugetlbfs check in hmm_vma_walk_pmd
>   mm/hmm: remove hmm_range vma

For all of these:

Reviewed-by: Jason Gunthorpe 

I've applied this to hmm.git, excluding:

>   mm/hmm: make full use of walk_page_range()

Pending further discussion.

Based on last cycle I've decided to move good patches into linux-next
earlier and rely on some rebase if needed. This is to help Andrew's
workflow.

So, if there are more tags/etc please continue to send them, I will
sort it..

Thanks,
Jason
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Fix amdgpu_display_supported_domains logic.

[Christian König]

Am 26.07.19 um 15:28 schrieb Andrey Grodzovsky:

Add restriction to dissallow GTT domain if the relevant BO
doesn't have USWC flag set to avoid the APU hang scenario.

Signed-off-by: Andrey Grodzovsky 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 16 +++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_display.h   |  3 ++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c   |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c| 12 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   |  2 +-
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
  6 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 73045a3..4a8b1b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -189,7 +189,8 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc 
*crtc,
}
  
  	if (!adev->enable_virtual_display) {

-   r = amdgpu_bo_pin(new_abo, 
amdgpu_display_supported_domains(adev));
+   r = amdgpu_bo_pin(new_abo,
+ amdgpu_display_supported_domains(adev, 
new_abo->flags));
if (unlikely(r != 0)) {
DRM_ERROR("failed to pin new abo buffer before flip\n");
goto unreserve;
@@ -493,20 +494,25 @@ static const struct drm_framebuffer_funcs amdgpu_fb_funcs 
= {
.create_handle = drm_gem_fb_create_handle,
  };
  
-uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev)

+uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
+ uint64_t bo_flags)
  {
uint32_t domain = AMDGPU_GEM_DOMAIN_VRAM;
  
  #if defined(CONFIG_DRM_AMD_DC)

/*
-* if amdgpu_bo_validate_uswc returns false it means that USWC mappings
+* if amdgpu_bo_support_uswc returns false it means that USWC mappings
 * is not supported for this board. But this mapping is required
 * to avoid hang caused by placement of scanout BO in GTT on certain
 * APUs. So force the BO placement to VRAM in case this architecture
 * will not allow USWC mappings.
+* Also, don't allow GTT domain if the BO doens't have USWC falg set.
 */
-   if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <= CHIP_RAVEN &&
-   adev->flags & AMD_IS_APU && amdgpu_bo_support_uswc(0) &&
+   if (adev->asic_type >= CHIP_CARRIZO &&
+   adev->asic_type <= CHIP_RAVEN &&
+   (adev->flags & AMD_IS_APU) &&
+   (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) &&
+   amdgpu_bo_support_uswc(bo_flags) &&
amdgpu_device_asic_has_dc_support(adev->asic_type))
domain |= AMDGPU_GEM_DOMAIN_GTT;
  #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
index 06b922f..3620b24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
@@ -38,7 +38,8 @@
  int amdgpu_display_freesync_ioctl(struct drm_device *dev, void *data,
  struct drm_file *filp);
  void amdgpu_display_update_priority(struct amdgpu_device *adev);
-uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev);
+uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
+ uint64_t bo_flags);
  struct drm_framebuffer *
  amdgpu_display_user_framebuffer_create(struct drm_device *dev,
   struct drm_file *file_priv,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 4711cf1..6770eb3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -301,7 +301,7 @@ static int amdgpu_dma_buf_begin_cpu_access(struct dma_buf 
*dma_buf,
struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
struct ttm_operation_ctx ctx = { true, false };
-   u32 domain = amdgpu_display_supported_domains(adev);
+   u32 domain = amdgpu_display_supported_domains(adev, bo->flags);
int ret;
bool reads = (direction == DMA_BIDIRECTIONAL ||
  direction == DMA_FROM_DEVICE);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index bf0c61b..4a6f29e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -130,21 +130,21 @@ static int amdgpufb_create_pinned_object(struct 
amdgpu_fbdev *rfbdev,
int aligned_size, size;
int height = mode_cmd->height;
u32 cpp;
+   u64 flags = AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
+  AMDGPU_GEM_CREATE

Re: [PATCH v5 2/4] drm/amdgpu: Create helper to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC

[Michel Dänzer]

On 2019-07-26 1:55 p.m., Christian König wrote:
> Am 26.07.19 um 10:54 schrieb Michel Dänzer:
>> On 2019-07-26 9:11 a.m., Christian König wrote:
>>> Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:
 Move the logic to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC in
 amdgpu_bo_do_create into standalone helper so it can be reused
 in other functions.

 v4:
 Switch to return bool.

 v5: Fix typos.

 Signed-off-by: Andrey Grodzovsky 
 ---
    drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 61
 +-
    drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +
    2 files changed, 37 insertions(+), 26 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
 index 989b7b5..8702062 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
 @@ -413,6 +413,40 @@ static bool amdgpu_bo_validate_size(struct
 amdgpu_device *adev,
    return false;
    }
    +bool amdgpu_bo_support_uswc(u64 bo_flags)
 +{
 +
 +#ifdef CONFIG_X86_32
 +    /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
 + * See https://bugs.freedesktop.org/show_bug.cgi?id=84627
 + */
 +    return false;
 +#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
 +    /* Don't try to enable write-combining when it can't work, or
 things
 + * may be slow
 + * See https://bugs.freedesktop.org/show_bug.cgi?id=88758
 + */
 +
 +#ifndef CONFIG_COMPILE_TEST
 +#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better
 performance \
 + thanks to write-combining
 +#endif
 +
 +    if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
 +    DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT
 for "
 +  "better performance thanks to write-combining\n");
>>> I don't think this message belongs here.
>>>
>>> [...]
 @@ -466,33 +500,8 @@ static int amdgpu_bo_do_create(struct
 [...]
 +    if (!amdgpu_bo_support_uswc(bo->flags))
    bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>>> Rather here we should do "if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC
>>> && !amdgpu_bo_support_uswc())" and then clear the flag and also print
>>> the warning.
>> That would require duplicating the CONFIG_X86_PAT related logic here as
>> well, which is a bit ugly.
> 
> Actually I would say we should drop this extra check and always emit a
> message that USWC is disabled for this platform.
> 
> We now need it for more than just better performance and it should be
> explicitly noted that this is not available in the logs.

A log message which doesn't explain why it's disabled / how to enable it
would probably cause us user support pain.


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Fix amdgpu_display_supported_domains logic.

[Deucher, Alexander]

Reviewed-by: Alex Deucher 

From: amd-gfx  on behalf of Andrey 
Grodzovsky 
Sent: Friday, July 26, 2019 9:28 AM
To: amd-gfx@lists.freedesktop.org 
Cc: Deucher, Alexander ; Grodzovsky, Andrey 
; mic...@daenzer.net ; Koenig, 
Christian ; S, Shirish 
Subject: [PATCH] drm/amdgpu: Fix amdgpu_display_supported_domains logic.

Add restriction to dissallow GTT domain if the relevant BO
doesn't have USWC flag set to avoid the APU hang scenario.

Signed-off-by: Andrey Grodzovsky 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 16 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.h   |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c| 12 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   |  2 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
 6 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 73045a3..4a8b1b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -189,7 +189,8 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc 
*crtc,
 }

 if (!adev->enable_virtual_display) {
-   r = amdgpu_bo_pin(new_abo, 
amdgpu_display_supported_domains(adev));
+   r = amdgpu_bo_pin(new_abo,
+ amdgpu_display_supported_domains(adev, 
new_abo->flags));
 if (unlikely(r != 0)) {
 DRM_ERROR("failed to pin new abo buffer before 
flip\n");
 goto unreserve;
@@ -493,20 +494,25 @@ static const struct drm_framebuffer_funcs amdgpu_fb_funcs 
= {
 .create_handle = drm_gem_fb_create_handle,
 };

-uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev)
+uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
+ uint64_t bo_flags)
 {
 uint32_t domain = AMDGPU_GEM_DOMAIN_VRAM;

 #if defined(CONFIG_DRM_AMD_DC)
 /*
-* if amdgpu_bo_validate_uswc returns false it means that USWC mappings
+* if amdgpu_bo_support_uswc returns false it means that USWC mappings
  * is not supported for this board. But this mapping is required
  * to avoid hang caused by placement of scanout BO in GTT on certain
  * APUs. So force the BO placement to VRAM in case this architecture
  * will not allow USWC mappings.
+* Also, don't allow GTT domain if the BO doens't have USWC falg set.
  */
-   if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <= CHIP_RAVEN &&
-   adev->flags & AMD_IS_APU && amdgpu_bo_support_uswc(0) &&
+   if (adev->asic_type >= CHIP_CARRIZO &&
+   adev->asic_type <= CHIP_RAVEN &&
+   (adev->flags & AMD_IS_APU) &&
+   (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) &&
+   amdgpu_bo_support_uswc(bo_flags) &&
 amdgpu_device_asic_has_dc_support(adev->asic_type))
 domain |= AMDGPU_GEM_DOMAIN_GTT;
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
index 06b922f..3620b24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
@@ -38,7 +38,8 @@
 int amdgpu_display_freesync_ioctl(struct drm_device *dev, void *data,
   struct drm_file *filp);
 void amdgpu_display_update_priority(struct amdgpu_device *adev);
-uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev);
+uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
+ uint64_t bo_flags);
 struct drm_framebuffer *
 amdgpu_display_user_framebuffer_create(struct drm_device *dev,
struct drm_file *file_priv,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 4711cf1..6770eb3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -301,7 +301,7 @@ static int amdgpu_dma_buf_begin_cpu_access(struct dma_buf 
*dma_buf,
 struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
 struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
 struct ttm_operation_ctx ctx = { true, false };
-   u32 domain = amdgpu_display_supported_domains(adev);
+   u32 domain = amdgpu_display_supported_domains(adev, bo->flags);
 int ret;
 bool reads = (direction == DMA_BIDIRECTIONAL ||
   direction == DMA_FROM_DEVICE);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index bf0c61b..4a6f29e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -130,21 +13

Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature status function in smu

[Deucher, Alexander]

Sure.

Alex

From: Wang, Kevin(Yang) 
Sent: Friday, July 26, 2019 5:05 AM
To: StDenis, Tom ; Deucher, Alexander 
; Huang, Ray ; Quan, Evan 
; amd-gfx@lists.freedesktop.org 
; Zhang, Hawking 
Cc: Feng, Kenneth 
Subject: Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature status 
function in smu

@Deucher, Alexander

Hi Alex,

it seems not application will use this sysfs, can we rename it from 
"ppfeatures" to "pp_features"?
this patch sets is pending.

Best Regards,
Kevin

From: StDenis, Tom 
Sent: Thursday, July 25, 2019 9:14 PM
To: Deucher, Alexander ; Huang, Ray 
; Wang, Kevin(Yang) ; Quan, Evan 
; amd-gfx@lists.freedesktop.org 
; Zhang, Hawking 
Cc: Feng, Kenneth 
Subject: Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature status 
function in smu

No it doesn't.  We get clocks for --top from the sensors interface.


On 2019-07-25 9:01 a.m., Deucher, Alexander wrote:
> Tom, does umr use it?
>
> Alex
> 
> *From:* Huang, Ray 
> *Sent:* Thursday, July 25, 2019 4:49 AM
> *To:* Wang, Kevin(Yang) ; Quan, Evan
> ; amd-gfx@lists.freedesktop.org
> ; Zhang, Hawking
> ; Deucher, Alexander 
> *Cc:* Feng, Kenneth 
> *Subject:* RE: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> Any other user mode tool use the “ppfeature” sysfs interface?
>
> Thanks,
>
> Ray
>
> *From:* Wang, Kevin(Yang) 
> *Sent:* Thursday, July 25, 2019 4:44 PM
> *To:* Quan, Evan ; amd-gfx@lists.freedesktop.org;
> Zhang, Hawking ; Deucher, Alexander
> 
> *Cc:* Huang, Ray ; Feng, Kenneth 
> *Subject:* Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> in fact, i don't want to change this sysfs name from "ppfeatures" to
> "pp_features",
>
> but it seems that don't have same name format with other pp sysfs node.
>
> the other powerplay sysfs name have "pp_" prefix, i think we'd better
> to change it name to "pp_features"
>
> eg:
>
> pp_cur_statepp_dpm_fclk  pp_dpm_pcie  pp_dpm_socclk
>  pp_force_state  pp_num_states  pp_sclk_od
> pp_dpm_dcefclk  pp_dpm_mclk  pp_dpm_sclk  pp_featurespp_mclk_od
>pp_power_profile_mode  pp_table
>
> @Deucher, Alexander  @Zhang, Hawking
> 
>
> Could you give us some idea about it,
>
> Thanks.
>
> Best Regards,
> Kevin
>
> 
>
> *From:*Quan, Evan mailto:evan.q...@amd.com>>
> *Sent:* Thursday, July 25, 2019 4:30 PM
> *To:* Wang, Kevin(Yang)  >; amd-gfx@lists.freedesktop.org
>   >
> *Cc:* Deucher, Alexander  >; Huang, Ray  >; Feng, Kenneth  >
> *Subject:* RE: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> To keep backward compatibility, we cannot change the sysfs file naming.
>
> But it’s a good idea to summarize these as common APIs.
>
> Regards,
>
> Evan
>
> *From:* amd-gfx  > *On Behalf Of *Wang,
> Kevin(Yang)
> *Sent:* Thursday, July 25, 2019 4:10 PM
> *To:* amd-gfx@lists.freedesktop.org 
> *Cc:* Deucher, Alexander  >; Huang, Ray  >; Feng, Kenneth  >
> *Subject:* Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> add sample data from sysfs pp_features with this patch.
>
> print format:
>
> index. feature name (Hardware Message ID): state
>
> /sudo find /sys -name "pp_features" -exec cat {} \;/
>
> features high: 0x0623 low: 0xb3cdaffb
>
> 00. DPM_PREFETCHER   ( 0) : enabeld
>
> 01. DPM_GFXCLK   ( 1) : enabeld
>
> 02. DPM_UCLK ( 3) : enabeld
>
> 03. DPM_SOCCLK   ( 4) : enabeld
>
> 04. DPM_MP0CLK   ( 5) : enabeld
>
> 05. DPM_LINK ( 6) : enabeld
>
> 06. DPM_DCEFCLK  ( 7) : enabeld
>
> 07. DS_GFXCLK(10) : enabeld
>
> 08. DS_SOCCLK(11) : enabeld
>
> 09. DS_LCLK  (12) : disabled
>
> 10. PPT(23) : enabeld
>
> 11. TDC(24) : enabeld
>
> 12. THERMAL  (33) : enabeld
>
> 13. RM (35) : disabled
>
> 14. DS_DCEFCLK   (13) : enabeld
>
> 15. ACDC (28) : enabeld
>
> 16. VR0HOT   (29) : enabeld
>
> 17. VR1HOT   (30) : disabled
>
> 18. FW_CTF   (31) : enabeld
>
> 19. LED_DISPLAY  (36) : disabled
>
> 20. FAN_CONTROL  (32) : enabeld
>
> 21. GFX_EDC  (25) : enabeld
>
> 22. GFXOFF   (17) : disabled
>
> 23. DPM_GFX_PACE ( 2) : disabled
>
> 24. MEM_VDDCI_SCALING

[PATCH AUTOSEL 5.2 40/85] drm/amd/display: Expose audio inst from DC to DM

[Sasha Levin]

From: Nicholas Kazlauskas 

[ Upstream commit 5fdb7c4c7f2691efd760b0b0dc00da4a3699f1a6 ]

[Why]
In order to give pin notifications to the sound driver from DM we need
to know whether audio is enabled on a stream and what pin it's using
from DC.

[How]
Expose the instance via stream status if it's a mapped resource for
the stream. It will be -1 if there's no audio mapped.

Cc: Leo Li 
Cc: Harry Wentland 
Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Alex Deucher 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 3 +++
 drivers/gpu/drm/amd/display/dc/dc_stream.h| 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index eac7186e4f08..12142d13f22f 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -2034,6 +2034,9 @@ enum dc_status resource_map_pool_resources(
if (context->streams[i] == stream) {
context->stream_status[i].primary_otg_inst = 
pipe_ctx->stream_res.tg->inst;
context->stream_status[i].stream_enc_inst = 
pipe_ctx->stream_res.stream_enc->id;
+   context->stream_status[i].audio_inst =
+   pipe_ctx->stream_res.audio ? 
pipe_ctx->stream_res.audio->inst : -1;
+
return DC_OK;
}
 
diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h 
b/drivers/gpu/drm/amd/display/dc/dc_stream.h
index 189bdab929a5..c20803b71fa5 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -42,6 +42,7 @@ struct dc_stream_status {
int primary_otg_inst;
int stream_enc_inst;
int plane_count;
+   int audio_inst;
struct timing_sync_info timing_sync_info;
struct dc_plane_state *plane_states[MAX_SURFACE_NUM];
 };
-- 
2.20.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 5/7] mm/hmm: make full use of walk_page_range()

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 6/7] mm/hmm: remove hugetlbfs check in hmm_vma_walk_pmd

[Christoph Hellwig]

On Thu, Jul 25, 2019 at 05:56:49PM -0700, Ralph Campbell wrote:
> walk_page_range() will only call hmm_vma_walk_hugetlb_entry() for
> hugetlbfs pages and doesn't call hmm_vma_walk_pmd() in this case.
> Therefore, it is safe to remove the check for vma->vm_flags & VM_HUGETLB
> in hmm_vma_walk_pmd().
> 
> Signed-off-by: Ralph Campbell 

Looks good,

Reviewed-by: Christoph Hellwig 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 2/7] mm/hmm: a few more C style and comment clean ups

[Christoph Hellwig]

Note: it seems like you've only CCed me on patches 2-7, but not on the
cover letter and patch 1.  I'll try to find them later, but to make Ccs
useful they should normally cover the whole series.

Otherwise this looks fine to me:

Reviewed-by: Christoph Hellwig 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 7/7] mm/hmm: remove hmm_range vma

[Christoph Hellwig]

On Thu, Jul 25, 2019 at 05:56:50PM -0700, Ralph Campbell wrote:
> Since hmm_range_fault() doesn't use the struct hmm_range vma field,
> remove it.
> 
> Suggested-by: Jason Gunthorpe 
> Signed-off-by: Ralph Campbell 

Looks good,

Reviewed-by: Christoph Hellwig 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Fix amdgpu_display_supported_domains logic.

[Andrey Grodzovsky]

Add restriction to dissallow GTT domain if the relevant BO
doesn't have USWC flag set to avoid the APU hang scenario.

Signed-off-by: Andrey Grodzovsky 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 16 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.h   |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c| 12 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   |  2 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
 6 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 73045a3..4a8b1b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -189,7 +189,8 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc 
*crtc,
}
 
if (!adev->enable_virtual_display) {
-   r = amdgpu_bo_pin(new_abo, 
amdgpu_display_supported_domains(adev));
+   r = amdgpu_bo_pin(new_abo,
+ amdgpu_display_supported_domains(adev, 
new_abo->flags));
if (unlikely(r != 0)) {
DRM_ERROR("failed to pin new abo buffer before flip\n");
goto unreserve;
@@ -493,20 +494,25 @@ static const struct drm_framebuffer_funcs amdgpu_fb_funcs 
= {
.create_handle = drm_gem_fb_create_handle,
 };
 
-uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev)
+uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
+ uint64_t bo_flags)
 {
uint32_t domain = AMDGPU_GEM_DOMAIN_VRAM;
 
 #if defined(CONFIG_DRM_AMD_DC)
/*
-* if amdgpu_bo_validate_uswc returns false it means that USWC mappings
+* if amdgpu_bo_support_uswc returns false it means that USWC mappings
 * is not supported for this board. But this mapping is required
 * to avoid hang caused by placement of scanout BO in GTT on certain
 * APUs. So force the BO placement to VRAM in case this architecture
 * will not allow USWC mappings.
+* Also, don't allow GTT domain if the BO doens't have USWC falg set.
 */
-   if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <= CHIP_RAVEN &&
-   adev->flags & AMD_IS_APU && amdgpu_bo_support_uswc(0) &&
+   if (adev->asic_type >= CHIP_CARRIZO &&
+   adev->asic_type <= CHIP_RAVEN &&
+   (adev->flags & AMD_IS_APU) &&
+   (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) &&
+   amdgpu_bo_support_uswc(bo_flags) &&
amdgpu_device_asic_has_dc_support(adev->asic_type))
domain |= AMDGPU_GEM_DOMAIN_GTT;
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
index 06b922f..3620b24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h
@@ -38,7 +38,8 @@
 int amdgpu_display_freesync_ioctl(struct drm_device *dev, void *data,
  struct drm_file *filp);
 void amdgpu_display_update_priority(struct amdgpu_device *adev);
-uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev);
+uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,
+ uint64_t bo_flags);
 struct drm_framebuffer *
 amdgpu_display_user_framebuffer_create(struct drm_device *dev,
   struct drm_file *file_priv,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 4711cf1..6770eb3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -301,7 +301,7 @@ static int amdgpu_dma_buf_begin_cpu_access(struct dma_buf 
*dma_buf,
struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
struct ttm_operation_ctx ctx = { true, false };
-   u32 domain = amdgpu_display_supported_domains(adev);
+   u32 domain = amdgpu_display_supported_domains(adev, bo->flags);
int ret;
bool reads = (direction == DMA_BIDIRECTIONAL ||
  direction == DMA_FROM_DEVICE);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index bf0c61b..4a6f29e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -130,21 +130,21 @@ static int amdgpufb_create_pinned_object(struct 
amdgpu_fbdev *rfbdev,
int aligned_size, size;
int height = mode_cmd->height;
u32 cpp;
+   u64 flags = AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
+  AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
+  AMDGPU_GEM_CREATE_VRAM_CLEARED|
+ 

RE: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature status function in smu

[Huang, Ray]

I am fine to align the interface name if no user mode use.

Acked-by: Huang Rui 

From: Wang, Kevin(Yang) 
Sent: Friday, July 26, 2019 5:05 PM
To: StDenis, Tom ; Deucher, Alexander 
; Huang, Ray ; Quan, Evan 
; amd-gfx@lists.freedesktop.org; Zhang, Hawking 

Cc: Feng, Kenneth 
Subject: Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature status 
function in smu

@Deucher, Alexander

Hi Alex,

it seems not application will use this sysfs, can we rename it from 
"ppfeatures" to "pp_features"?
this patch sets is pending.

Best Regards,
Kevin

From: StDenis, Tom mailto:tom.stde...@amd.com>>
Sent: Thursday, July 25, 2019 9:14 PM
To: Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>; Huang, Ray 
mailto:ray.hu...@amd.com>>; Wang, Kevin(Yang) 
mailto:kevin1.w...@amd.com>>; Quan, Evan 
mailto:evan.q...@amd.com>>; 
amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>; Zhang, 
Hawking mailto:hawking.zh...@amd.com>>
Cc: Feng, Kenneth mailto:kenneth.f...@amd.com>>
Subject: Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature status 
function in smu

No it doesn't.  We get clocks for --top from the sensors interface.


On 2019-07-25 9:01 a.m., Deucher, Alexander wrote:
> Tom, does umr use it?
>
> Alex
> 
> *From:* Huang, Ray mailto:ray.hu...@amd.com>>
> *Sent:* Thursday, July 25, 2019 4:49 AM
> *To:* Wang, Kevin(Yang) mailto:kevin1.w...@amd.com>>; 
> Quan, Evan
> mailto:evan.q...@amd.com>>; 
> amd-gfx@lists.freedesktop.org
> mailto:amd-gfx@lists.freedesktop.org>>; Zhang, 
> Hawking
> mailto:hawking.zh...@amd.com>>; Deucher, Alexander 
> mailto:alexander.deuc...@amd.com>>
> *Cc:* Feng, Kenneth mailto:kenneth.f...@amd.com>>
> *Subject:* RE: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> Any other user mode tool use the "ppfeature" sysfs interface?
>
> Thanks,
>
> Ray
>
> *From:* Wang, Kevin(Yang) mailto:kevin1.w...@amd.com>>
> *Sent:* Thursday, July 25, 2019 4:44 PM
> *To:* Quan, Evan mailto:evan.q...@amd.com>>; 
> amd-gfx@lists.freedesktop.org;
> Zhang, Hawking mailto:hawking.zh...@amd.com>>; 
> Deucher, Alexander
> mailto:alexander.deuc...@amd.com>>
> *Cc:* Huang, Ray mailto:ray.hu...@amd.com>>; Feng, Kenneth 
> mailto:kenneth.f...@amd.com>>
> *Subject:* Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> in fact, i don't want to change this sysfs name from "ppfeatures" to
> "pp_features",
>
> but it seems that don't have same name format with other pp sysfs node.
>
> the other powerplay sysfs name have "pp_" prefix, i think we'd better
> to change it name to "pp_features"
>
> eg:
>
> pp_cur_statepp_dpm_fclk  pp_dpm_pcie  pp_dpm_socclk
>  pp_force_state  pp_num_states  pp_sclk_od
> pp_dpm_dcefclk  pp_dpm_mclk  pp_dpm_sclk  pp_featurespp_mclk_od
>pp_power_profile_mode  pp_table
>
> @Deucher, Alexander  @Zhang, Hawking
> 
>
> Could you give us some idea about it,
>
> Thanks.
>
> Best Regards,
> Kevin
>
> 
>
> *From:*Quan, Evan mailto:evan.q...@amd.com>>
> *Sent:* Thursday, July 25, 2019 4:30 PM
> *To:* Wang, Kevin(Yang) mailto:kevin1.w...@amd.com%20%0b>> >; 
amd-gfx@lists.freedesktop.org
>  mailto:amd-gfx@lists.freedesktop.org%20%0b>> 
>
> *Cc:* Deucher, Alexander mailto:alexander.deuc...@amd.com%20%0b>> >; 
Huang, Ray mailto:ray.hu...@amd.com%20%0b>> >; Feng, Kenneth 
mailto:kenneth.f...@amd.com%20%0b>> >
> *Subject:* RE: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> To keep backward compatibility, we cannot change the sysfs file naming.
>
> But it's a good idea to summarize these as common APIs.
>
> Regards,
>
> Evan
>
> *From:* amd-gfx mailto:amd-gfx-boun...@lists.freedesktop.org%20%0b>> 
> *On Behalf Of *Wang,
> Kevin(Yang)
> *Sent:* Thursday, July 25, 2019 4:10 PM
> *To:* amd-gfx@lists.freedesktop.org 
> 
> *Cc:* Deucher, Alexander mailto:alexander.deuc...@amd.com%20%0b>> >; 
Huang, Ray mailto:ray.hu...@amd.com%20%0b>> >; Feng, Kenneth 
mailto:kenneth.f...@amd.com%20%0b>> >
> *Subject:* Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> add sample data from sysfs pp_features with this patch.
>
> print format:
>
> index

Re: [PATCH v5 4/4] drm/amd/display: enable S/G for RAVEN chip

[Grodzovsky, Andrey]

Totally agree, me or Shirish will add a new patch for this as I've 
already commuted the series.

Andrey

On 7/26/19 5:03 AM, Michel Dänzer wrote:
> On 2019-07-26 9:14 a.m., Christian König wrote:
>> Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:
>>> From: Shirish S 
>>>
>>> enables gpu_vm_support in dm and adds
>>> AMDGPU_GEM_DOMAIN_GTT as supported domain
>>>
>>> v2:
>>> Move BO placement logic into amdgpu_display_supported_domains
>>>
>>> v3:
>>> Use amdgpu_bo_validate_uswc in amdgpu_display_supported_domains.
>>>
>>> v4:
>>> amdgpu_bo_validate_uswc moved to sepperate patch.
>>>
>>> Change-Id: If34300beaa60be2d36170b7b5b096ec644502b20
>>> Signed-off-by: Shirish S 
>>> Signed-off-by: Andrey Grodzovsky 
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 2 +-
>>>    drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> index cac9975..73045a3 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>>> @@ -505,7 +505,7 @@ uint32_t amdgpu_display_supported_domains(struct
>>> amdgpu_device *adev)
>>>     * APUs. So force the BO placement to VRAM in case this
>>> architecture
>>>     * will not allow USWC mappings.
>>>     */
>>> -    if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <
>>> CHIP_RAVEN &&
>>> +    if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <=
>>> CHIP_RAVEN &&
>> This whole approach is incorrect since we don't check the flags of the
>> actually BO used for scanout any more.
>>
>> As I wrote before it is still perfectly possible and valid that
>> userspace never sets this flag.
> Oh right, now I get what you meant before!
>
> I guess amdgpu_display_supported_domains needs to take the BO flags as a
> parameter, and also check that AMDGPU_GEM_CREATE_CPU_GTT_USWC is
> actually set.
>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v5 01/24] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Pietrasiewicz]

Hi Sam,

W dniu 26.07.2019 o 08:37, Sam Ravnborg pisze:

Hi Andrzej.

Patch looks good, but one kernel-doc detail.



Thanks, I will address both issues you found, in v6.

Andrzej
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v5 2/4] drm/amdgpu: Create helper to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC

[Christian König]

Am 26.07.19 um 10:54 schrieb Michel Dänzer:

On 2019-07-26 9:11 a.m., Christian König wrote:

Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:

Move the logic to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC in
amdgpu_bo_do_create into standalone helper so it can be reused
in other functions.

v4:
Switch to return bool.

v5: Fix typos.

Signed-off-by: Andrey Grodzovsky 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 61
+-
   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +
   2 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 989b7b5..8702062 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -413,6 +413,40 @@ static bool amdgpu_bo_validate_size(struct
amdgpu_device *adev,
   return false;
   }
   +bool amdgpu_bo_support_uswc(u64 bo_flags)
+{
+
+#ifdef CONFIG_X86_32
+    /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
+ * See https://bugs.freedesktop.org/show_bug.cgi?id=84627
+ */
+    return false;
+#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
+    /* Don't try to enable write-combining when it can't work, or things
+ * may be slow
+ * See https://bugs.freedesktop.org/show_bug.cgi?id=88758
+ */
+
+#ifndef CONFIG_COMPILE_TEST
+#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better
performance \
+ thanks to write-combining
+#endif
+
+    if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
+    DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT
for "
+  "better performance thanks to write-combining\n");

I don't think this message belongs here.

[...]

@@ -466,33 +500,8 @@ static int amdgpu_bo_do_create(struct
[...]
+    if (!amdgpu_bo_support_uswc(bo->flags))
   bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;

Rather here we should do "if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC
&& !amdgpu_bo_support_uswc())" and then clear the flag and also print
the warning.

That would require duplicating the CONFIG_X86_PAT related logic here as
well, which is a bit ugly.


Actually I would say we should drop this extra check and always emit a 
message that USWC is disabled for this platform.


We now need it for more than just better performance and it should be 
explicitly noted that this is not available in the logs.


Regards,
Christian.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amd/amdgpu/vcn_v2_0: Mark RB commands as KMD commands

[Christian König]

Good point, this patch is Reviewed-by: Christian König 
.


But please ping the firmware guys if that really could be an issue,
Christian.

Am 26.07.19 um 12:45 schrieb Thai, Thong:
Well, not through this particular piece of code, since this explicitly 
sets it. But I would imagine someone could set the bit in userspace 
and insert KMD commands in the BO as part of some IB instructions - 
I’ll have a look.



*From:* Christian König 
*Sent:* Friday, July 26, 2019 3:17:19 AM
*To:* Thai, Thong ; amd-gfx@lists.freedesktop.org 

*Subject:* Re: [PATCH 1/2] drm/amd/amdgpu/vcn_v2_0: Mark RB commands 
as KMD commands

Am 25.07.19 um 17:52 schrieb Thai, Thong:
> Sets the CMD_SOURCE bit for VCN 2.0 Decoder Ring Buffer commands. This
> bit was previously set by the RBC HW on older firmware. Newer firmware
> uses a SW RBC and this bit has to be set by the driver.

Mhm, another question came to my mind: Would it now be possible for user
space to set this flag and and gain access to the kernel driver commands?

Cause that could be a security problem.

Christian.

>
> Signed-off-by: Thong Thai 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 +
>   drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c   | 12 ++--
>   2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h

> index 5e2453ee6b29..4d3bf4adf1eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -30,6 +30,7 @@
>   #define AMDGPU_VCN_FIRMWARE_OFFSET  256
>   #define AMDGPU_VCN_MAX_ENC_RINGS    3
>
> +#define VCN_DEC_KMD_CMD 0x8000
>   #define VCN_DEC_CMD_FENCE   0x
>   #define VCN_DEC_CMD_TRAP    0x0001
>   #define VCN_DEC_CMD_WRITE_REG 0x0004
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c

> index bc9726787c97..7091aef95ff0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> @@ -1488,7 +1488,7 @@ static void 
vcn_v2_0_dec_ring_insert_start(struct amdgpu_ring *ring)
>    amdgpu_ring_write(ring, 
PACKET0(mmUVD_GPCOM_VCPU_DATA0_INTERNAL_OFFSET, 0));

>    amdgpu_ring_write(ring, 0);
>    amdgpu_ring_write(ring, 
PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));

> - amdgpu_ring_write(ring, VCN_DEC_CMD_PACKET_START << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | 
(VCN_DEC_CMD_PACKET_START << 1));

>   }
>
>   /**
> @@ -1501,7 +1501,7 @@ static void 
vcn_v2_0_dec_ring_insert_start(struct amdgpu_ring *ring)

>   static void vcn_v2_0_dec_ring_insert_end(struct amdgpu_ring *ring)
>   {
>    amdgpu_ring_write(ring, 
PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));

> - amdgpu_ring_write(ring, VCN_DEC_CMD_PACKET_END << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | 
(VCN_DEC_CMD_PACKET_END << 1));

>   }
>
>   /**
> @@ -1546,7 +1546,7 @@ static void 
vcn_v2_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64

>    amdgpu_ring_write(ring, upper_32_bits(addr) & 0xff);
>
>    amdgpu_ring_write(ring, 
PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));

> - amdgpu_ring_write(ring, VCN_DEC_CMD_FENCE << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_FENCE 
<< 1));

>
>    amdgpu_ring_write(ring, 
PACKET0(mmUVD_GPCOM_VCPU_DATA0_INTERNAL_OFFSET, 0));

>    amdgpu_ring_write(ring, 0);
> @@ -1556,7 +1556,7 @@ static void 
vcn_v2_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64

>
>    amdgpu_ring_write(ring, 
PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));

>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_TRAP << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_TRAP << 
1));

>   }
>
>   /**
> @@ -1600,7 +1600,7 @@ static void 
vcn_v2_0_dec_ring_emit_reg_wait(struct amdgpu_ring *ring,

>
>    amdgpu_ring_write(ring, 
PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));

>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_REG_READ_COND_WAIT << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | 
(VCN_DEC_CMD_REG_READ_COND_WAIT << 1));

>   }
>
>   static void vcn_v2_0_dec_ring_emit_vm_flush(struct amdgpu_ring *ring,
> @@ -1629,7 +1629,7 @@ static void vcn_v2_0_dec_ring_emit_wreg(struct 
amdgpu_ring *ring,

>
>    amdgpu_ring_write(ring, 
PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));

>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_WRITE_REG << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | 
(VCN_DEC_CMD_WRITE_REG << 1));

>   }
>
>   /**


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amd/amdgpu/vcn_v2_0: Mark RB commands as KMD commands

[Thai, Thong]

Well, not through this particular piece of code, since this explicitly sets it. 
But I would imagine someone could set the bit in userspace and insert KMD 
commands in the BO as part of some IB instructions - I’ll have a look.


From: Christian König 
Sent: Friday, July 26, 2019 3:17:19 AM
To: Thai, Thong ; amd-gfx@lists.freedesktop.org 

Subject: Re: [PATCH 1/2] drm/amd/amdgpu/vcn_v2_0: Mark RB commands as KMD 
commands

Am 25.07.19 um 17:52 schrieb Thai, Thong:
> Sets the CMD_SOURCE bit for VCN 2.0 Decoder Ring Buffer commands. This
> bit was previously set by the RBC HW on older firmware. Newer firmware
> uses a SW RBC and this bit has to be set by the driver.

Mhm, another question came to my mind: Would it now be possible for user
space to set this flag and and gain access to the kernel driver commands?

Cause that could be a security problem.

Christian.

>
> Signed-off-by: Thong Thai 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 +
>   drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c   | 12 ++--
>   2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 5e2453ee6b29..4d3bf4adf1eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -30,6 +30,7 @@
>   #define AMDGPU_VCN_FIRMWARE_OFFSET  256
>   #define AMDGPU_VCN_MAX_ENC_RINGS3
>
> +#define VCN_DEC_KMD_CMD  0x8000
>   #define VCN_DEC_CMD_FENCE   0x
>   #define VCN_DEC_CMD_TRAP0x0001
>   #define VCN_DEC_CMD_WRITE_REG   0x0004
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
> b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> index bc9726787c97..7091aef95ff0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> @@ -1488,7 +1488,7 @@ static void vcn_v2_0_dec_ring_insert_start(struct 
> amdgpu_ring *ring)
>amdgpu_ring_write(ring, 
> PACKET0(mmUVD_GPCOM_VCPU_DATA0_INTERNAL_OFFSET, 0));
>amdgpu_ring_write(ring, 0);
>amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 
> 0));
> - amdgpu_ring_write(ring, VCN_DEC_CMD_PACKET_START << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_PACKET_START << 
> 1));
>   }
>
>   /**
> @@ -1501,7 +1501,7 @@ static void vcn_v2_0_dec_ring_insert_start(struct 
> amdgpu_ring *ring)
>   static void vcn_v2_0_dec_ring_insert_end(struct amdgpu_ring *ring)
>   {
>amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 
> 0));
> - amdgpu_ring_write(ring, VCN_DEC_CMD_PACKET_END << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_PACKET_END << 
> 1));
>   }
>
>   /**
> @@ -1546,7 +1546,7 @@ static void vcn_v2_0_dec_ring_emit_fence(struct 
> amdgpu_ring *ring, u64 addr, u64
>amdgpu_ring_write(ring, upper_32_bits(addr) & 0xff);
>
>amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 
> 0));
> - amdgpu_ring_write(ring, VCN_DEC_CMD_FENCE << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_FENCE << 1));
>
>amdgpu_ring_write(ring, 
> PACKET0(mmUVD_GPCOM_VCPU_DATA0_INTERNAL_OFFSET, 0));
>amdgpu_ring_write(ring, 0);
> @@ -1556,7 +1556,7 @@ static void vcn_v2_0_dec_ring_emit_fence(struct 
> amdgpu_ring *ring, u64 addr, u64
>
>amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 
> 0));
>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_TRAP << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_TRAP << 1));
>   }
>
>   /**
> @@ -1600,7 +1600,7 @@ static void vcn_v2_0_dec_ring_emit_reg_wait(struct 
> amdgpu_ring *ring,
>
>amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 
> 0));
>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_REG_READ_COND_WAIT << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | 
> (VCN_DEC_CMD_REG_READ_COND_WAIT << 1));
>   }
>
>   static void vcn_v2_0_dec_ring_emit_vm_flush(struct amdgpu_ring *ring,
> @@ -1629,7 +1629,7 @@ static void vcn_v2_0_dec_ring_emit_wreg(struct 
> amdgpu_ring *ring,
>
>amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 
> 0));
>
> - amdgpu_ring_write(ring, VCN_DEC_CMD_WRITE_REG << 1);
> + amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_WRITE_REG << 1));
>   }
>
>   /**

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature status function in smu

[Wang, Kevin(Yang)]

@Deucher, Alexander

Hi Alex,

it seems not application will use this sysfs, can we rename it from 
"ppfeatures" to "pp_features"?
this patch sets is pending.

Best Regards,
Kevin

From: StDenis, Tom 
Sent: Thursday, July 25, 2019 9:14 PM
To: Deucher, Alexander ; Huang, Ray 
; Wang, Kevin(Yang) ; Quan, Evan 
; amd-gfx@lists.freedesktop.org 
; Zhang, Hawking 
Cc: Feng, Kenneth 
Subject: Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature status 
function in smu

No it doesn't.  We get clocks for --top from the sensors interface.


On 2019-07-25 9:01 a.m., Deucher, Alexander wrote:
> Tom, does umr use it?
>
> Alex
> 
> *From:* Huang, Ray 
> *Sent:* Thursday, July 25, 2019 4:49 AM
> *To:* Wang, Kevin(Yang) ; Quan, Evan
> ; amd-gfx@lists.freedesktop.org
> ; Zhang, Hawking
> ; Deucher, Alexander 
> *Cc:* Feng, Kenneth 
> *Subject:* RE: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> Any other user mode tool use the “ppfeature” sysfs interface?
>
> Thanks,
>
> Ray
>
> *From:* Wang, Kevin(Yang) 
> *Sent:* Thursday, July 25, 2019 4:44 PM
> *To:* Quan, Evan ; amd-gfx@lists.freedesktop.org;
> Zhang, Hawking ; Deucher, Alexander
> 
> *Cc:* Huang, Ray ; Feng, Kenneth 
> *Subject:* Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> in fact, i don't want to change this sysfs name from "ppfeatures" to
> "pp_features",
>
> but it seems that don't have same name format with other pp sysfs node.
>
> the other powerplay sysfs name have "pp_" prefix, i think we'd better
> to change it name to "pp_features"
>
> eg:
>
> pp_cur_statepp_dpm_fclk  pp_dpm_pcie  pp_dpm_socclk
>  pp_force_state  pp_num_states  pp_sclk_od
> pp_dpm_dcefclk  pp_dpm_mclk  pp_dpm_sclk  pp_featurespp_mclk_od
>pp_power_profile_mode  pp_table
>
> @Deucher, Alexander  @Zhang, Hawking
> 
>
> Could you give us some idea about it,
>
> Thanks.
>
> Best Regards,
> Kevin
>
> 
>
> *From:*Quan, Evan mailto:evan.q...@amd.com>>
> *Sent:* Thursday, July 25, 2019 4:30 PM
> *To:* Wang, Kevin(Yang)  >; amd-gfx@lists.freedesktop.org
>   >
> *Cc:* Deucher, Alexander  >; Huang, Ray  >; Feng, Kenneth  >
> *Subject:* RE: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> To keep backward compatibility, we cannot change the sysfs file naming.
>
> But it’s a good idea to summarize these as common APIs.
>
> Regards,
>
> Evan
>
> *From:* amd-gfx  > *On Behalf Of *Wang,
> Kevin(Yang)
> *Sent:* Thursday, July 25, 2019 4:10 PM
> *To:* amd-gfx@lists.freedesktop.org 
> *Cc:* Deucher, Alexander  >; Huang, Ray  >; Feng, Kenneth  >
> *Subject:* Re: [PATCH 5/5] drm/amd/powerplay: implment sysfs feature
> status function in smu
>
> add sample data from sysfs pp_features with this patch.
>
> print format:
>
> index. feature name (Hardware Message ID): state
>
> /sudo find /sys -name "pp_features" -exec cat {} \;/
>
> features high: 0x0623 low: 0xb3cdaffb
>
> 00. DPM_PREFETCHER   ( 0) : enabeld
>
> 01. DPM_GFXCLK   ( 1) : enabeld
>
> 02. DPM_UCLK ( 3) : enabeld
>
> 03. DPM_SOCCLK   ( 4) : enabeld
>
> 04. DPM_MP0CLK   ( 5) : enabeld
>
> 05. DPM_LINK ( 6) : enabeld
>
> 06. DPM_DCEFCLK  ( 7) : enabeld
>
> 07. DS_GFXCLK(10) : enabeld
>
> 08. DS_SOCCLK(11) : enabeld
>
> 09. DS_LCLK  (12) : disabled
>
> 10. PPT(23) : enabeld
>
> 11. TDC(24) : enabeld
>
> 12. THERMAL  (33) : enabeld
>
> 13. RM (35) : disabled
>
> 14. DS_DCEFCLK   (13) : enabeld
>
> 15. ACDC (28) : enabeld
>
> 16. VR0HOT   (29) : enabeld
>
> 17. VR1HOT   (30) : disabled
>
> 18. FW_CTF   (31) : enabeld
>
> 19. LED_DISPLAY  (36) : disabled
>
> 20. FAN_CONTROL  (32) : enabeld
>
> 21. GFX_EDC  (25) : enabeld
>
> 22. GFXOFF   (17) : disabled
>
> 23. DPM_GFX_PACE ( 2) : disabled
>
> 24. MEM_VDDCI_SCALING( 8) : enabeld
>
> 25. MEM_MVDD_SCALING ( 9) : enabeld
>
> 26. DS_UCLK  (14) : disabled
>
> 27. GFX_ULV  (15) : enabeld
>
> 28. FW_DSTATE(16) : enabeld
>
> 29. BACO (18) : enabeld
>
> 30. VCN_PG   (19) : enabeld
>
> 31. JPEG_PG  (20) : disabled
>
> 3

Re: [PATCH v5 4/4] drm/amd/display: enable S/G for RAVEN chip

[Michel Dänzer]

On 2019-07-26 9:14 a.m., Christian König wrote:
> Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:
>> From: Shirish S 
>>
>> enables gpu_vm_support in dm and adds
>> AMDGPU_GEM_DOMAIN_GTT as supported domain
>>
>> v2:
>> Move BO placement logic into amdgpu_display_supported_domains
>>
>> v3:
>> Use amdgpu_bo_validate_uswc in amdgpu_display_supported_domains.
>>
>> v4:
>> amdgpu_bo_validate_uswc moved to sepperate patch.
>>
>> Change-Id: If34300beaa60be2d36170b7b5b096ec644502b20
>> Signed-off-by: Shirish S 
>> Signed-off-by: Andrey Grodzovsky 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 2 +-
>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> index cac9975..73045a3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> @@ -505,7 +505,7 @@ uint32_t amdgpu_display_supported_domains(struct
>> amdgpu_device *adev)
>>    * APUs. So force the BO placement to VRAM in case this
>> architecture
>>    * will not allow USWC mappings.
>>    */
>> -    if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <
>> CHIP_RAVEN &&
>> +    if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <=
>> CHIP_RAVEN &&
> 
> This whole approach is incorrect since we don't check the flags of the
> actually BO used for scanout any more.
> 
> As I wrote before it is still perfectly possible and valid that
> userspace never sets this flag.

Oh right, now I get what you meant before!

I guess amdgpu_display_supported_domains needs to take the BO flags as a
parameter, and also check that AMDGPU_GEM_CREATE_CPU_GTT_USWC is
actually set.


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v5 2/4] drm/amdgpu: Create helper to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC

[Michel Dänzer]

On 2019-07-26 9:11 a.m., Christian König wrote:
> Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:
>> Move the logic to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC in
>> amdgpu_bo_do_create into standalone helper so it can be reused
>> in other functions.
>>
>> v4:
>> Switch to return bool.
>>
>> v5: Fix typos.
>>
>> Signed-off-by: Andrey Grodzovsky 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 61
>> +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +
>>   2 files changed, 37 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 989b7b5..8702062 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -413,6 +413,40 @@ static bool amdgpu_bo_validate_size(struct
>> amdgpu_device *adev,
>>   return false;
>>   }
>>   +bool amdgpu_bo_support_uswc(u64 bo_flags)
>> +{
>> +
>> +#ifdef CONFIG_X86_32
>> +    /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
>> + * See https://bugs.freedesktop.org/show_bug.cgi?id=84627
>> + */
>> +    return false;
>> +#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
>> +    /* Don't try to enable write-combining when it can't work, or things
>> + * may be slow
>> + * See https://bugs.freedesktop.org/show_bug.cgi?id=88758
>> + */
>> +
>> +#ifndef CONFIG_COMPILE_TEST
>> +#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better
>> performance \
>> + thanks to write-combining
>> +#endif
>> +
>> +    if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
>> +    DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT
>> for "
>> +  "better performance thanks to write-combining\n");
> 
> I don't think this message belongs here.
> 
> [...]
>> @@ -466,33 +500,8 @@ static int amdgpu_bo_do_create(struct
>> [...]
>> +    if (!amdgpu_bo_support_uswc(bo->flags))
>>   bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> 
> Rather here we should do "if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC
> && !amdgpu_bo_support_uswc())" and then clear the flag and also print
> the warning.

That would require duplicating the CONFIG_X86_PAT related logic here as
well, which is a bit ugly.


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v5 01/24] drm: Include ddc adapter pointer in struct drm_connector

[Sam Ravnborg]

Hi Andrzej.

After reading through the series a few more comments.

1) The subject of this patch could be improved.
   Consider something like:
   drm: add ddc link in sysfs created by drm_connector

   This spells out much better what the patch achieve.


2) The purpsoe of drm_connector.ddc is to provide drm_connector with
   info to create the symlink.
   Yet in many follow-up patches the drivers are changed so
   drm_connector.ddc is their only reference to struct i2c_adapter.

   But the ownership is not clear here.
   Who owns the reference to i2c_adapter - and who has the
   responsibility to call put() on the adapter.

   Looking at the conversions done then some drivers are converted
   so they only use drm_connector.ddc, and other drivers have their own
   reference to i2c_adapter.
   The latter looks like the correct solution as the ownership of the
   reference belongs to the driver and not drm_connector.

   In other words - a conversion where the drivers only assigned
   drm_connector.ddc (using drm_connector_init_with_ddc()),
   seems like a more clean design that does not mix up ownership.
   This is at least how I see it.
   I did not take this type of look at the patches before. Sorry
   for providing feedback this late.

Sam

On Fri, Jul 26, 2019 at 08:37:59AM +0200, Sam Ravnborg wrote:
> Hi Andrzej.
> 
> Patch looks good, but one kernel-doc detail.
> 
> On Wed, Jul 24, 2019 at 03:59:23PM +0200, Andrzej Pietrasiewicz wrote:
> > Add generic code which creates symbolic links in sysfs, pointing to ddc
> > interface used by a particular video output. For example:
> > 
> > ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> > -> ../../../../soc/1388.i2c/i2c-2
> > 
> > This makes it easy for user to associate a display with its ddc adapter
> > and use e.g. ddcutil to control the chosen monitor.
> > 
> > This patch adds an i2c_adapter pointer to struct drm_connector. Particular
> > drivers can then use it instead of using their own private instance. If a
> > connector contains a ddc, then create a symbolic link in sysfs.
> > 
> > Signed-off-by: Andrzej Pietrasiewicz 
> > Acked-by: Daniel Vetter 
> > Reviewed-by: Andrzej Hajda 
> > ---
> >  drivers/gpu/drm/drm_sysfs.c |  8 
> >  include/drm/drm_connector.h | 11 +++
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> > index ad10810bc972..e962a9d45f7e 100644
> > --- a/drivers/gpu/drm/drm_sysfs.c
> > +++ b/drivers/gpu/drm/drm_sysfs.c
> > @@ -14,6 +14,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -294,6 +295,9 @@ int drm_sysfs_connector_add(struct drm_connector 
> > *connector)
> > /* Let userspace know we have a new connector */
> > drm_sysfs_hotplug_event(dev);
> >  
> > +   if (connector->ddc)
> > +   return sysfs_create_link(&connector->kdev->kobj,
> > +&connector->ddc->dev.kobj, "ddc");
> > return 0;
> >  }
> >  
> > @@ -301,6 +305,10 @@ void drm_sysfs_connector_remove(struct drm_connector 
> > *connector)
> >  {
> > if (!connector->kdev)
> > return;
> > +
> > +   if (connector->ddc)
> > +   sysfs_remove_link(&connector->kdev->kobj, "ddc");
> > +
> > DRM_DEBUG("removing \"%s\" from sysfs\n",
> >   connector->name);
> >  
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 4c30d751487a..33a6fff85fdb 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -41,6 +41,7 @@ struct drm_property;
> >  struct drm_property_blob;
> >  struct drm_printer;
> >  struct edid;
> > +struct i2c_adapter;
> >  
> >  enum drm_connector_force {
> > DRM_FORCE_UNSPECIFIED,
> > @@ -1311,6 +1312,16 @@ struct drm_connector {
> >  * [0]: progressive, [1]: interlaced
> >  */
> > int audio_latency[2];
> > +
> > +   /**
> > +* @ddc: associated ddc adapter.
> > +* A connector usually has its associated ddc adapter. If a driver uses
> > +* this field, then an appropriate symbolic link is created in connector
> > +* sysfs directory to make it easy for the user to tell which i2c
> > +* adapter is for a particular display.
> > +*/
> > +   struct i2c_adapter *ddc;
> 
> To help the reader could you add in the above a reference to
> drm_connector_init_with_ddc() sp the reader is told how to init this
> field.
> 
> Either add it in PATCH 2 - or merge patch 1 and 2.
> 
>   Sam
> 
> > +
> > /**
> >  * @null_edid_counter: track sinks that give us all zeros for the EDID.
> >  * Needed to workaround some HW bugs where we get all 0s
> > -- 
> > 2.17.1
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/amd/display: remove duplicated include from dc_link.c

[YueHaibing]

Remove duplicated include.

Reported-by: Hulk Robot 
Signed-off-by: YueHaibing 
---
 drivers/gpu/drm/amd/display/dc/core/dc_link.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 193d6f1..a14785d 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -45,10 +45,6 @@
 #include "dpcd_defs.h"
 #include "dmcu.h"
 #include "hw/clk_mgr.h"
-#if defined(CONFIG_DRM_AMD_DC_DCN2_0)
-#include "resource.h"
-#endif
-#include "hw/clk_mgr.h"
 
 #define DC_LOGGER_INIT(logger)
 
-- 
2.7.4

Re: [PATCH 1/2] drm/amd/amdgpu/vcn_v2_0: Mark RB commands as KMD commands

[Christian König]

Am 25.07.19 um 17:52 schrieb Thai, Thong:

Sets the CMD_SOURCE bit for VCN 2.0 Decoder Ring Buffer commands. This
bit was previously set by the RBC HW on older firmware. Newer firmware
uses a SW RBC and this bit has to be set by the driver.


Mhm, another question came to my mind: Would it now be possible for user 
space to set this flag and and gain access to the kernel driver commands?


Cause that could be a security problem.

Christian.



Signed-off-by: Thong Thai 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 +
  drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c   | 12 ++--
  2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 5e2453ee6b29..4d3bf4adf1eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -30,6 +30,7 @@
  #define AMDGPU_VCN_FIRMWARE_OFFSET256
  #define AMDGPU_VCN_MAX_ENC_RINGS  3
  
+#define VCN_DEC_KMD_CMD			0x8000

  #define VCN_DEC_CMD_FENCE 0x
  #define VCN_DEC_CMD_TRAP  0x0001
  #define VCN_DEC_CMD_WRITE_REG 0x0004
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index bc9726787c97..7091aef95ff0 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -1488,7 +1488,7 @@ static void vcn_v2_0_dec_ring_insert_start(struct 
amdgpu_ring *ring)
amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_DATA0_INTERNAL_OFFSET, 
0));
amdgpu_ring_write(ring, 0);
amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 
0));
-   amdgpu_ring_write(ring, VCN_DEC_CMD_PACKET_START << 1);
+   amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_PACKET_START << 
1));
  }
  
  /**

@@ -1501,7 +1501,7 @@ static void vcn_v2_0_dec_ring_insert_start(struct 
amdgpu_ring *ring)
  static void vcn_v2_0_dec_ring_insert_end(struct amdgpu_ring *ring)
  {
amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 
0));
-   amdgpu_ring_write(ring, VCN_DEC_CMD_PACKET_END << 1);
+   amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_PACKET_END << 
1));
  }
  
  /**

@@ -1546,7 +1546,7 @@ static void vcn_v2_0_dec_ring_emit_fence(struct 
amdgpu_ring *ring, u64 addr, u64
amdgpu_ring_write(ring, upper_32_bits(addr) & 0xff);
  
  	amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));

-   amdgpu_ring_write(ring, VCN_DEC_CMD_FENCE << 1);
+   amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_FENCE << 1));
  
  	amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_DATA0_INTERNAL_OFFSET, 0));

amdgpu_ring_write(ring, 0);
@@ -1556,7 +1556,7 @@ static void vcn_v2_0_dec_ring_emit_fence(struct 
amdgpu_ring *ring, u64 addr, u64
  
  	amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
  
-	amdgpu_ring_write(ring, VCN_DEC_CMD_TRAP << 1);

+   amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_TRAP << 1));
  }
  
  /**

@@ -1600,7 +1600,7 @@ static void vcn_v2_0_dec_ring_emit_reg_wait(struct 
amdgpu_ring *ring,
  
  	amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
  
-	amdgpu_ring_write(ring, VCN_DEC_CMD_REG_READ_COND_WAIT << 1);

+   amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_REG_READ_COND_WAIT 
<< 1));
  }
  
  static void vcn_v2_0_dec_ring_emit_vm_flush(struct amdgpu_ring *ring,

@@ -1629,7 +1629,7 @@ static void vcn_v2_0_dec_ring_emit_wreg(struct 
amdgpu_ring *ring,
  
  	amdgpu_ring_write(ring, PACKET0(mmUVD_GPCOM_VCPU_CMD_INTERNAL_OFFSET, 0));
  
-	amdgpu_ring_write(ring, VCN_DEC_CMD_WRITE_REG << 1);

+   amdgpu_ring_write(ring, VCN_DEC_KMD_CMD | (VCN_DEC_CMD_WRITE_REG << 1));
  }
  
  /**


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v5 4/4] drm/amd/display: enable S/G for RAVEN chip

[Christian König]

Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:

From: Shirish S 

enables gpu_vm_support in dm and adds
AMDGPU_GEM_DOMAIN_GTT as supported domain

v2:
Move BO placement logic into amdgpu_display_supported_domains

v3:
Use amdgpu_bo_validate_uswc in amdgpu_display_supported_domains.

v4:
amdgpu_bo_validate_uswc moved to sepperate patch.

Change-Id: If34300beaa60be2d36170b7b5b096ec644502b20
Signed-off-by: Shirish S 
Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 2 +-
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index cac9975..73045a3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -505,7 +505,7 @@ uint32_t amdgpu_display_supported_domains(struct 
amdgpu_device *adev)
 * APUs. So force the BO placement to VRAM in case this architecture
 * will not allow USWC mappings.
 */
-   if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type < CHIP_RAVEN &&
+   if (adev->asic_type >= CHIP_CARRIZO && adev->asic_type <= CHIP_RAVEN &&


This whole approach is incorrect since we don't check the flags of the 
actually BO used for scanout any more.


As I wrote before it is still perfectly possible and valid that 
userspace never sets this flag.


Regards,
Christian.


adev->flags & AMD_IS_APU && amdgpu_bo_support_uswc(0) &&
amdgpu_device_asic_has_dc_support(adev->asic_type))
domain |= AMDGPU_GEM_DOMAIN_GTT;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 4922589..f0387ce1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -686,7 +686,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 */
if (adev->flags & AMD_IS_APU &&
adev->asic_type >= CHIP_CARRIZO &&
-   adev->asic_type < CHIP_RAVEN)
+   adev->asic_type <= CHIP_RAVEN)
init_data.flags.gpu_vm_support = true;
  
  	if (amdgpu_dc_feature_mask & DC_FBC_MASK)


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v5 2/4] drm/amdgpu: Create helper to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC

[Christian König]

Am 25.07.19 um 16:24 schrieb Andrey Grodzovsky:

Move the logic to clear AMDGPU_GEM_CREATE_CPU_GTT_USWC in
amdgpu_bo_do_create into standalone helper so it can be reused
in other functions.

v4:
Switch to return bool.

v5: Fix typos.

Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 61 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +
  2 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 989b7b5..8702062 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -413,6 +413,40 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device 
*adev,
return false;
  }
  
+bool amdgpu_bo_support_uswc(u64 bo_flags)

+{
+
+#ifdef CONFIG_X86_32
+   /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
+* See https://bugs.freedesktop.org/show_bug.cgi?id=84627
+*/
+   return false;
+#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
+   /* Don't try to enable write-combining when it can't work, or things
+* may be slow
+* See https://bugs.freedesktop.org/show_bug.cgi?id=88758
+*/
+
+#ifndef CONFIG_COMPILE_TEST
+#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance \
+thanks to write-combining
+#endif
+
+   if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
+   DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT for 
"
+ "better performance thanks to write-combining\n");


I don't think this message belongs here.


+   return false;
+#else
+   /* For architectures that don't support WC memory,
+* mask out the WC flag from the BO
+*/
+   if (!drm_arch_can_wc_memory())
+   return false;
+
+   return true;
+#endif
+}
+
  static int amdgpu_bo_do_create(struct amdgpu_device *adev,
   struct amdgpu_bo_param *bp,
   struct amdgpu_bo **bo_ptr)
@@ -466,33 +500,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
  
  	bo->flags = bp->flags;
  
-#ifdef CONFIG_X86_32

-   /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
-* See https://bugs.freedesktop.org/show_bug.cgi?id=84627
-*/
-   bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
-#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
-   /* Don't try to enable write-combining when it can't work, or things
-* may be slow
-* See https://bugs.freedesktop.org/show_bug.cgi?id=88758
-*/
-
-#ifndef CONFIG_COMPILE_TEST
-#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance \
-thanks to write-combining
-#endif
-
-   if (bo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
-   DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT for 
"
- "better performance thanks to write-combining\n");
-   bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
-#else
-   /* For architectures that don't support WC memory,
-* mask out the WC flag from the BO
-*/
-   if (!drm_arch_can_wc_memory())
+   if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;


Rather here we should do "if (bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC 
&& !amdgpu_bo_support_uswc())" and then clear the flag and also print 
the warning.


Apart from that the series looks good to me,
Christian.


-#endif
  
  	bo->tbo.bdev = &adev->mman.bdev;

if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index d60593c..dc44cf3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -308,5 +308,7 @@ void amdgpu_sa_bo_dump_debug_info(struct amdgpu_sa_manager 
*sa_manager,
 struct seq_file *m);
  #endif
  
+bool amdgpu_bo_support_uswc(u64 bo_flags);

+
  
  #endif


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/4] drm/radeon: Fill out gem_object->resv

[Koenig, Christian]

Am 25.07.19 um 15:26 schrieb Daniel Vetter:
> That way we can ditch our gem_prime_res_obj implementation. Since ttm
> absolutely needs the right reservation object all the boilerplate is
> already there and we just have to wire it up correctly.
>
> Note that gem/prime doesn't care when we do this, as long as we do it
> before the bo is registered and someone can call the handle2fd ioctl
> on it.
>
> Aside: ttm_buffer_object.ttm_resv could probably be ditched in favour
> of always passing a non-NULL resv to ttm_bo_init(). At least for gem
> drivers that would avoid having two of these, on in ttm_buffer_object
> and the other in drm_gem_object, one just there for confusion.
>
> Acked-by: Gerd Hoffmann 
> Cc: Gerd Hoffmann 
> Reviewed-by: Emil Velikov 
> Signed-off-by: Daniel Vetter 
> Cc: Alex Deucher 
> Cc: "Christian König" 
> Cc: "David (ChunMing) Zhou" 
> Cc: amd-gfx@lists.freedesktop.org

Reviewed-by: Christian König 

> ---
>   drivers/gpu/drm/radeon/radeon_drv.c| 2 --
>   drivers/gpu/drm/radeon/radeon_object.c | 1 +
>   drivers/gpu/drm/radeon/radeon_prime.c  | 7 ---
>   3 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 4403e76e1ae0..a4a78dfdef37 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -152,7 +152,6 @@ struct drm_gem_object 
> *radeon_gem_prime_import_sg_table(struct drm_device *dev,
>   struct sg_table *sg);
>   int radeon_gem_prime_pin(struct drm_gem_object *obj);
>   void radeon_gem_prime_unpin(struct drm_gem_object *obj);
> -struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object *);
>   void *radeon_gem_prime_vmap(struct drm_gem_object *obj);
>   void radeon_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
>   
> @@ -566,7 +565,6 @@ static struct drm_driver kms_driver = {
>   .gem_prime_export = radeon_gem_prime_export,
>   .gem_prime_pin = radeon_gem_prime_pin,
>   .gem_prime_unpin = radeon_gem_prime_unpin,
> - .gem_prime_res_obj = radeon_gem_prime_res_obj,
>   .gem_prime_get_sg_table = radeon_gem_prime_get_sg_table,
>   .gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
>   .gem_prime_vmap = radeon_gem_prime_vmap,
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
> b/drivers/gpu/drm/radeon/radeon_object.c
> index 21f73fc86f38..7a2bad843f8a 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -262,6 +262,7 @@ int radeon_bo_create(struct radeon_device *rdev,
>   r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
>   &bo->placement, page_align, !kernel, acc_size,
>   sg, resv, &radeon_ttm_bo_destroy);
> + bo->gem_base.resv = bo->tbo.resv;
>   up_read(&rdev->pm.mclk_lock);
>   if (unlikely(r != 0)) {
>   return r;
> diff --git a/drivers/gpu/drm/radeon/radeon_prime.c 
> b/drivers/gpu/drm/radeon/radeon_prime.c
> index deaffce50a2e..8ce3e8045d42 100644
> --- a/drivers/gpu/drm/radeon/radeon_prime.c
> +++ b/drivers/gpu/drm/radeon/radeon_prime.c
> @@ -117,13 +117,6 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj)
>   }
>   
>   
> -struct reservation_object *radeon_gem_prime_res_obj(struct drm_gem_object 
> *obj)
> -{
> - struct radeon_bo *bo = gem_to_radeon_bo(obj);
> -
> - return bo->tbo.resv;
> -}
> -
>   struct dma_buf *radeon_gem_prime_export(struct drm_gem_object *gobj,
>   int flags)
>   {

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx