Re: [PATCH v4] drm/amdgpu: fix multiple memory leaks in acp_hw_init
> --- Why did you omit the patch change log at this place? > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 34 - > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > @@ -189,7 +189,7 @@ static int acp_hw_init(void *handle) … > + struct i2s_platform_data *i2s_pdata = NULL; … I propose to reconsider this update suggestion once more. > @@ -393,6 +396,13 @@ static int acp_hw_init(void *handle) > val &= ~ACP_SOFT_RESET__SoftResetAud_MASK; > cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val); > return 0; > + > +failure: > + kfree(i2s_pdata); > + kfree(adev->acp.acp_res); > + kfree(adev->acp.acp_cell); > + kfree(adev->acp.acp_genpd); > + return r; > } > > /** Are you going to follow a known programming guideline? https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12-C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution%28copy_process%28%29fromLinuxkernel%29 Regards, Markus
[PATCH] drm/amdgpu: Drop unused variable and statement
Even though 'smu8_smu' is declared, it is not used after below statement.
smu8_smu = hwmgr->smu_backend;
So 'unused variable' could be safely removed
to stop warning message as below:
drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/smu8_smumgr.c:180:22:
warning: variable ‘smu8_smu’ set but not used
[-Wunused-but-set-variable]
struct smu8_smumgr *smu8_smu;
^
Signed-off-by: Austin Kim
---
drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
index 4728aa2..7dca04a 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
@@ -177,12 +177,10 @@ static int smu8_load_mec_firmware(struct pp_hwmgr *hwmgr)
uint32_t tmp;
int ret = 0;
struct cgs_firmware_info info = {0};
- struct smu8_smumgr *smu8_smu;
if (hwmgr == NULL || hwmgr->device == NULL)
return -EINVAL;
- smu8_smu = hwmgr->smu_backend;
ret = cgs_get_firmware_info(hwmgr->device,
CGS_UCODE_ID_CP_MEC, );
--
2.6.2
Re: [PATCH v3] drm/amdgpu: fix multiple memory leaks in acp_hw_init
On Tue, Oct 1, 2019 at 7:19 AM Koenig, Christian
wrote:
>
> Am 30.09.19 um 23:26 schrieb Navid Emamdoost:
> > In acp_hw_init there are some allocations that needs to be released in
> > case of failure:
> >
> > 1- adev->acp.acp_genpd should be released if any allocation attemp for
> > adev->acp.acp_cell, adev->acp.acp_res or i2s_pdata fails.
> > 2- all of those allocations should be released if
> > mfd_add_hotplug_devices or pm_genpd_add_device fail.
> > 3- Release is needed in case of time out values expire.
> >
> > Signed-off-by: Navid Emamdoost
> > ---
> > Changes in v2:
> > -- moved the releases under goto
> >
> > Changes in v3:
> > -- fixed multiple goto issue
> > -- added goto for 3 other failure cases: one when
> > mfd_add_hotplug_devices fails, and two when time out values expires.
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 41 -
> > 1 file changed, 27 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > index eba42c752bca..7809745ec0f1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > @@ -184,12 +184,12 @@ static struct device *get_mfd_cell_dev(const char
> > *device_name, int r)
> >*/
> > static int acp_hw_init(void *handle)
> > {
> > - int r, i;
> > + int r, i, ret;
>
> Please don't add another "ret" variable, instead always use "r" here.
>
Done!
> Apart from that looks good to me,
> Christian.
>
> > uint64_t acp_base;
> > u32 val = 0;
> > u32 count = 0;
> > struct device *dev;
> > - struct i2s_platform_data *i2s_pdata;
> > + struct i2s_platform_data *i2s_pdata = NULL;
> >
> > struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> >
> > @@ -231,20 +231,21 @@ static int acp_hw_init(void *handle)
> > adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
> > GFP_KERNEL);
> >
> > - if (adev->acp.acp_cell == NULL)
> > - return -ENOMEM;
> > + if (adev->acp.acp_cell == NULL) {
> > + ret = -ENOMEM;
> > + goto failure;
> > + }
> >
> > adev->acp.acp_res = kcalloc(5, sizeof(struct resource), GFP_KERNEL);
> > if (adev->acp.acp_res == NULL) {
> > - kfree(adev->acp.acp_cell);
> > - return -ENOMEM;
> > + ret = -ENOMEM;
> > + goto failure;
> > }
> >
> > i2s_pdata = kcalloc(3, sizeof(struct i2s_platform_data), GFP_KERNEL);
> > if (i2s_pdata == NULL) {
> > - kfree(adev->acp.acp_res);
> > - kfree(adev->acp.acp_cell);
> > - return -ENOMEM;
> > + ret = -ENOMEM;
> > + goto failure;
> > }
> >
> > switch (adev->asic_type) {
> > @@ -340,15 +341,18 @@ static int acp_hw_init(void *handle)
> >
> > r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell,
> > ACP_DEVS);
> > - if (r)
> > - return r;
> > + if (r) {
> > + ret = r;
> > + goto failure;
> > + }
> >
> > for (i = 0; i < ACP_DEVS ; i++) {
> > dev = get_mfd_cell_dev(adev->acp.acp_cell[i].name, i);
> > r = pm_genpd_add_device(>acp.acp_genpd->gpd, dev);
> > if (r) {
> > dev_err(dev, "Failed to add dev to genpd\n");
> > - return r;
> > + ret = r;
> > + goto failure;
> > }
> > }
> >
> > @@ -367,7 +371,8 @@ static int acp_hw_init(void *handle)
> > break;
> > if (--count == 0) {
> > dev_err(>pdev->dev, "Failed to reset ACP\n");
> > - return -ETIMEDOUT;
> > + ret = -ETIMEDOUT;
> > + goto failure;
> > }
> > udelay(100);
> > }
> > @@ -384,7 +389,8 @@ static int acp_hw_init(void *handle)
> > break;
> > if (--count == 0) {
> > dev_err(>pdev->dev, "Failed to reset ACP\n");
> > - return -ETIMEDOUT;
> > + ret = -ETIMEDOUT;
> > + goto failure;
> > }
> > udelay(100);
> > }
> > @@ -393,6 +399,13 @@ static int acp_hw_init(void *handle)
> > val &= ~ACP_SOFT_RESET__SoftResetAud_MASK;
> > cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val);
> > return 0;
> > +
> > +failure:
> > + kfree(i2s_pdata);
> > + kfree(adev->acp.acp_res);
> > + kfree(adev->acp.acp_cell);
> > + kfree(adev->acp.acp_genpd);
> > + return ret;
> > }
> >
> > /**
>
--
Navid.
[PATCH v4] drm/amdgpu: fix multiple memory leaks in acp_hw_init
In acp_hw_init there are some allocations that needs to be released in
case of failure:
1- adev->acp.acp_genpd should be released if any allocation attemp for
adev->acp.acp_cell, adev->acp.acp_res or i2s_pdata fails.
2- all of those allocations should be released if
mfd_add_hotplug_devices or pm_genpd_add_device fail.
3- Release is needed in case of time out values expire.
Signed-off-by: Navid Emamdoost
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 34 -
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
index eba42c752bca..82155ac3288a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
@@ -189,7 +189,7 @@ static int acp_hw_init(void *handle)
u32 val = 0;
u32 count = 0;
struct device *dev;
- struct i2s_platform_data *i2s_pdata;
+ struct i2s_platform_data *i2s_pdata = NULL;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -231,20 +231,21 @@ static int acp_hw_init(void *handle)
adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
GFP_KERNEL);
- if (adev->acp.acp_cell == NULL)
- return -ENOMEM;
+ if (adev->acp.acp_cell == NULL) {
+ r = -ENOMEM;
+ goto failure;
+ }
adev->acp.acp_res = kcalloc(5, sizeof(struct resource), GFP_KERNEL);
if (adev->acp.acp_res == NULL) {
- kfree(adev->acp.acp_cell);
- return -ENOMEM;
+ r = -ENOMEM;
+ goto failure;
}
i2s_pdata = kcalloc(3, sizeof(struct i2s_platform_data), GFP_KERNEL);
if (i2s_pdata == NULL) {
- kfree(adev->acp.acp_res);
- kfree(adev->acp.acp_cell);
- return -ENOMEM;
+ r = -ENOMEM;
+ goto failure;
}
switch (adev->asic_type) {
@@ -341,14 +342,14 @@ static int acp_hw_init(void *handle)
r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell,
ACP_DEVS);
if (r)
- return r;
+ goto failure;
for (i = 0; i < ACP_DEVS ; i++) {
dev = get_mfd_cell_dev(adev->acp.acp_cell[i].name, i);
r = pm_genpd_add_device(>acp.acp_genpd->gpd, dev);
if (r) {
dev_err(dev, "Failed to add dev to genpd\n");
- return r;
+ goto failure;
}
}
@@ -367,7 +368,8 @@ static int acp_hw_init(void *handle)
break;
if (--count == 0) {
dev_err(>pdev->dev, "Failed to reset ACP\n");
- return -ETIMEDOUT;
+ r = -ETIMEDOUT;
+ goto failure;
}
udelay(100);
}
@@ -384,7 +386,8 @@ static int acp_hw_init(void *handle)
break;
if (--count == 0) {
dev_err(>pdev->dev, "Failed to reset ACP\n");
- return -ETIMEDOUT;
+ r = -ETIMEDOUT;
+ goto failure;
}
udelay(100);
}
@@ -393,6 +396,13 @@ static int acp_hw_init(void *handle)
val &= ~ACP_SOFT_RESET__SoftResetAud_MASK;
cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val);
return 0;
+
+failure:
+ kfree(i2s_pdata);
+ kfree(adev->acp.acp_res);
+ kfree(adev->acp.acp_cell);
+ kfree(adev->acp.acp_genpd);
+ return r;
}
/**
--
2.17.1
[PATCH] drm/amd/display: Use pixel encoding 444 for dongle usb-c to hdmi
Fix pinkish color issue around grey areas. This also happens
when not using any dongle so directly with a usb-c to Display
Port cable. Meaning there is something wrong when using pixel
encoding RGB with amd driver in the general case. In the meantime
just use the same pixel encoding as when using HDMI without dongle.
This way users will see the same thing on 2 identical screens when
one is connected with hdmi-to-hdmi and the other is connected with
usb-c-to-hdmi.
Signed-off-by: Julien Isorce
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
1 file changed, 5 insertions(+)
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 d3f404f097eb..8139dcc0bfba 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3313,6 +3313,7 @@ static void fill_stream_properties_from_drm_display_mode(
{
struct dc_crtc_timing *timing_out = >timing;
const struct drm_display_info *info = >display_info;
+ const struct dc_link *link = stream->sink->link;
memset(timing_out, 0, sizeof(struct dc_crtc_timing));
@@ -3327,6 +3328,10 @@ static void fill_stream_properties_from_drm_display_mode(
else if ((connector->display_info.color_formats &
DRM_COLOR_FORMAT_YCRCB444)
&& stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR444;
+ else if ((connector->display_info.color_formats &
DRM_COLOR_FORMAT_YCRCB444)
+ && stream->sink->sink_signal == SIGNAL_TYPE_DISPLAY_PORT
+ && link->dpcd_caps.dongle_type ==
DISPLAY_DONGLE_DP_HDMI_CONVERTER)
+ timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR444;
else
timing_out->pixel_encoding = PIXEL_ENCODING_RGB;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init
On 2019-10-01 5:57 p.m., Gustavo A. R. Silva wrote:
>
> On 10/1/19 16:46, Liu, Leo wrote:
>
> + ring->sched.ready = true;
This is redundant. all the sched->ready is initialized as true, please
refer to drm_sched_init()
>>> I see... so in the following commit 1b61de45dfaf ("drm/amdgpu: add initial
>>> VCN2.0 support (v2)")
>>> that line is also redundant?
>> Yes.
>>
> OK.
>
> r = amdgpu_ring_test_ring(ring);
> if (r) {
> ring->sched.ready = false;
> @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle)
>
> for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
> ring = >vcn.inst[j].ring_enc[i];
> - ring->sched.ready = false;
> - continue;
> + ring->sched.ready = true;
Because the VCN 2.5 FW still has issue for encode, so we have it
disabled here.
>>> OK. So, maybe we can add a comment pointing that out?
>> That could be better.
>>
> Great. I'm glad it's not a bug. I'll write a patch for that so other
> people don't waste time taking a look.
Thanks, just sent two patches to add comment, and long with the patch to
make VCN ring ready properly.
Leo
>
> I appreciate your feedback.
> Thanks
> --
> Gustavo
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init
On 10/1/19 16:46, Liu, Leo wrote:
+ ring->sched.ready = true;
>>> This is redundant. all the sched->ready is initialized as true, please
>>> refer to drm_sched_init()
>>>
>> I see... so in the following commit 1b61de45dfaf ("drm/amdgpu: add initial
>> VCN2.0 support (v2)")
>> that line is also redundant?
>
> Yes.
>
OK.
>
r = amdgpu_ring_test_ring(ring);
if (r) {
ring->sched.ready = false;
@@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle)
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
ring = >vcn.inst[j].ring_enc[i];
- ring->sched.ready = false;
- continue;
+ ring->sched.ready = true;
>>> Because the VCN 2.5 FW still has issue for encode, so we have it
>>> disabled here.
>>>
>> OK. So, maybe we can add a comment pointing that out?
>
> That could be better.
>
Great. I'm glad it's not a bug. I'll write a patch for that so other
people don't waste time taking a look.
I appreciate your feedback.
Thanks
--
Gustavo
[PATCH 1/2] drm/amdgpu/vcn: use amdgpu_ring_test_helper instead of
amdgpu_ring_test_ring, so it will determine whether the ring is ready
Signed-off-by: Leo Liu
Cc: Gustavo A. R. Silva
---
drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 1 -
drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 21 ++---
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 18 ++
3 files changed, 12 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 93b3500e522b..b4f84a820a44 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -202,7 +202,6 @@ static int vcn_v1_0_hw_init(void *handle)
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
ring = >vcn.inst->ring_enc[i];
- ring->sched.ready = true;
r = amdgpu_ring_test_helper(ring);
if (r)
goto done;
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index 4628fd10a9ec..38f787a560cb 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -247,30 +247,21 @@ static int vcn_v2_0_hw_init(void *handle)
adev->nbio.funcs->vcn_doorbell_range(adev, ring->use_doorbell,
ring->doorbell_index, 0);
- ring->sched.ready = true;
- r = amdgpu_ring_test_ring(ring);
- if (r) {
- ring->sched.ready = false;
+ r = amdgpu_ring_test_helper(ring);
+ if (r)
goto done;
- }
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
ring = >vcn.inst->ring_enc[i];
- ring->sched.ready = true;
- r = amdgpu_ring_test_ring(ring);
- if (r) {
- ring->sched.ready = false;
+ r = amdgpu_ring_test_helper(ring);
+ if (r)
goto done;
- }
}
ring = >vcn.inst->ring_jpeg;
- ring->sched.ready = true;
- r = amdgpu_ring_test_ring(ring);
- if (r) {
- ring->sched.ready = false;
+ r = amdgpu_ring_test_helper(ring);
+ if (r)
goto done;
- }
done:
if (!r)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index bf8626e15b09..cc19363f 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -258,29 +258,23 @@ static int vcn_v2_5_hw_init(void *handle)
adev->nbio.funcs->vcn_doorbell_range(adev, ring->use_doorbell,
ring->doorbell_index, j);
- r = amdgpu_ring_test_ring(ring);
- if (r) {
- ring->sched.ready = false;
+ r = amdgpu_ring_test_helper(ring);
+ if (r)
goto done;
- }
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
ring = >vcn.inst[j].ring_enc[i];
ring->sched.ready = false;
continue;
- r = amdgpu_ring_test_ring(ring);
- if (r) {
- ring->sched.ready = false;
+ r = amdgpu_ring_test_helper(ring);
+ if (r)
goto done;
- }
}
ring = >vcn.inst[j].ring_jpeg;
- r = amdgpu_ring_test_ring(ring);
- if (r) {
- ring->sched.ready = false;
+ r = amdgpu_ring_test_helper(ring);
+ if (r)
goto done;
- }
}
done:
if (!r)
--
2.20.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/2] drm/amdgpu: add a comment to VCN 2.5 encode ring
Signed-off-by: Leo Liu
Suggested-by: Gustavo A. R. Silva
Cc: Gustavo A. R. Silva
---
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index cc19363f..0d5c95d73b63 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -264,6 +264,7 @@ static int vcn_v2_5_hw_init(void *handle)
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
ring = >vcn.inst[j].ring_enc[i];
+ /* disable encode rings till the robustness of the FW */
ring->sched.ready = false;
continue;
r = amdgpu_ring_test_helper(ring);
--
2.20.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: don't increment vram lost if we are in hibernation
We reset the GPU as part of our hibernation sequence so we need
to make sure we don't mark vram as lost in that case.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=111879
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/nv.c| 6 --
drivers/gpu/drm/amd/amdgpu/soc15.c | 6 --
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index b3e7756fcc4b..17dc054a4b6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -319,10 +319,12 @@ static int nv_asic_reset(struct amdgpu_device *adev)
struct smu_context *smu = >smu;
if (nv_asic_reset_method(adev) == AMD_RESET_METHOD_BACO) {
- amdgpu_inc_vram_lost(adev);
+ if (!adev->in_suspend)
+ amdgpu_inc_vram_lost(adev);
ret = smu_baco_reset(smu);
} else {
- amdgpu_inc_vram_lost(adev);
+ if (!adev->in_suspend)
+ amdgpu_inc_vram_lost(adev);
ret = nv_asic_mode1_reset(adev);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index b5240b5dbf7b..88a49a68c67f 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -564,12 +564,14 @@ static int soc15_asic_reset(struct amdgpu_device *adev)
{
switch (soc15_asic_reset_method(adev)) {
case AMD_RESET_METHOD_BACO:
- amdgpu_inc_vram_lost(adev);
+ if (!adev->in_suspend)
+ amdgpu_inc_vram_lost(adev);
return soc15_asic_baco_reset(adev);
case AMD_RESET_METHOD_MODE2:
return soc15_mode2_reset(adev);
default:
- amdgpu_inc_vram_lost(adev);
+ if (!adev->in_suspend)
+ amdgpu_inc_vram_lost(adev);
return soc15_asic_mode1_reset(adev);
}
}
--
2.20.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init
On 2019-10-01 5:43 p.m., Gustavo A. R. Silva wrote:
> Hi,
>
> On 10/1/19 16:29, Liu, Leo wrote:
>> On 2019-10-01 1:16 p.m., Gustavo A. R. Silva wrote:
>>> Notice that there is a *continue* statement in the middle of the
>>> for loop and that prevents the code below from ever being reached:
>>>
>>> r = amdgpu_ring_test_ring(ring);
>>> if (r) {
>>> ring->sched.ready = false;
>>> goto done;
>>> }
>>>
>>> Fix this by removing the continue statement and updating ring->sched.ready
>>> to true before calling amdgpu_ring_test_ring(ring).
>>>
>>> Notice that this fix is based on
>>> commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)")
>>>
>>> Addresses-Coverity-ID 1485608 ("Structurally dead code")
>>> Fixes: 28c17d72072b ("drm/amdgpu: add VCN2.5 basic supports")
>>> Signed-off-by: Gustavo A. R. Silva
>>> ---
>>>
>>> Any feedback is greatly appreciated.
>>>
>>>drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 5 +++--
>>>1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>>> index 395c2259f979..47b0dcd59e13 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>>> @@ -258,6 +258,7 @@ static int vcn_v2_5_hw_init(void *handle)
>>> adev->nbio_funcs->vcn_doorbell_range(adev, ring->use_doorbell,
>>> ring->doorbell_index, j);
>>>
>>> + ring->sched.ready = true;
>> This is redundant. all the sched->ready is initialized as true, please
>> refer to drm_sched_init()
>>
> I see... so in the following commit 1b61de45dfaf ("drm/amdgpu: add initial
> VCN2.0 support (v2)")
> that line is also redundant?
Yes.
>>> r = amdgpu_ring_test_ring(ring);
>>> if (r) {
>>> ring->sched.ready = false;
>>> @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle)
>>>
>>> for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
>>> ring = >vcn.inst[j].ring_enc[i];
>>> - ring->sched.ready = false;
>>> - continue;
>>> + ring->sched.ready = true;
>> Because the VCN 2.5 FW still has issue for encode, so we have it
>> disabled here.
>>
> OK. So, maybe we can add a comment pointing that out?
That could be better.
Thanks,
Leo
>
> Thanks
> --
> Gustavo
>
>>> r = amdgpu_ring_test_ring(ring);
>>> if (r) {
>>> ring->sched.ready = false;
>>> @@ -276,6 +276,7 @@ static int vcn_v2_5_hw_init(void *handle)
>>> }
>>>
>>> ring = >vcn.inst[j].ring_jpeg;
>>> + ring->sched.ready = true;
>>> r = amdgpu_ring_test_ring(ring);
>>> if (r) {
>>> ring->sched.ready = false;
Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init
Hi,
On 10/1/19 16:29, Liu, Leo wrote:
>
> On 2019-10-01 1:16 p.m., Gustavo A. R. Silva wrote:
>> Notice that there is a *continue* statement in the middle of the
>> for loop and that prevents the code below from ever being reached:
>>
>> r = amdgpu_ring_test_ring(ring);
>> if (r) {
>> ring->sched.ready = false;
>> goto done;
>> }
>>
>> Fix this by removing the continue statement and updating ring->sched.ready
>> to true before calling amdgpu_ring_test_ring(ring).
>>
>> Notice that this fix is based on
>> commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)")
>>
>> Addresses-Coverity-ID 1485608 ("Structurally dead code")
>> Fixes: 28c17d72072b ("drm/amdgpu: add VCN2.5 basic supports")
>> Signed-off-by: Gustavo A. R. Silva
>> ---
>>
>> Any feedback is greatly appreciated.
>>
>> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>> index 395c2259f979..47b0dcd59e13 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
>> @@ -258,6 +258,7 @@ static int vcn_v2_5_hw_init(void *handle)
>> adev->nbio_funcs->vcn_doorbell_range(adev, ring->use_doorbell,
>> ring->doorbell_index, j);
>>
>> +ring->sched.ready = true;
>
> This is redundant. all the sched->ready is initialized as true, please
> refer to drm_sched_init()
>
I see... so in the following commit 1b61de45dfaf ("drm/amdgpu: add initial
VCN2.0 support (v2)")
that line is also redundant?
>
>> r = amdgpu_ring_test_ring(ring);
>> if (r) {
>> ring->sched.ready = false;
>> @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle)
>>
>> for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
>> ring = >vcn.inst[j].ring_enc[i];
>> -ring->sched.ready = false;
>> -continue;
>> +ring->sched.ready = true;
>
> Because the VCN 2.5 FW still has issue for encode, so we have it
> disabled here.
>
OK. So, maybe we can add a comment pointing that out?
Thanks
--
Gustavo
>
>> r = amdgpu_ring_test_ring(ring);
>> if (r) {
>> ring->sched.ready = false;
>> @@ -276,6 +276,7 @@ static int vcn_v2_5_hw_init(void *handle)
>> }
>>
>> ring = >vcn.inst[j].ring_jpeg;
>> +ring->sched.ready = true;
>> r = amdgpu_ring_test_ring(ring);
>> if (r) {
>> ring->sched.ready = false;
Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init
On 2019-10-01 1:16 p.m., Gustavo A. R. Silva wrote:
> Notice that there is a *continue* statement in the middle of the
> for loop and that prevents the code below from ever being reached:
>
> r = amdgpu_ring_test_ring(ring);
> if (r) {
> ring->sched.ready = false;
> goto done;
> }
>
> Fix this by removing the continue statement and updating ring->sched.ready
> to true before calling amdgpu_ring_test_ring(ring).
>
> Notice that this fix is based on
> commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)")
>
> Addresses-Coverity-ID 1485608 ("Structurally dead code")
> Fixes: 28c17d72072b ("drm/amdgpu: add VCN2.5 basic supports")
> Signed-off-by: Gustavo A. R. Silva
> ---
>
> Any feedback is greatly appreciated.
>
> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> index 395c2259f979..47b0dcd59e13 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> @@ -258,6 +258,7 @@ static int vcn_v2_5_hw_init(void *handle)
> adev->nbio_funcs->vcn_doorbell_range(adev, ring->use_doorbell,
>ring->doorbell_index, j);
>
> + ring->sched.ready = true;
This is redundant. all the sched->ready is initialized as true, please
refer to drm_sched_init()
> r = amdgpu_ring_test_ring(ring);
> if (r) {
> ring->sched.ready = false;
> @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle)
>
> for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
> ring = >vcn.inst[j].ring_enc[i];
> - ring->sched.ready = false;
> - continue;
> + ring->sched.ready = true;
Because the VCN 2.5 FW still has issue for encode, so we have it
disabled here.
Regards,
Leo
> r = amdgpu_ring_test_ring(ring);
> if (r) {
> ring->sched.ready = false;
> @@ -276,6 +276,7 @@ static int vcn_v2_5_hw_init(void *handle)
> }
>
> ring = >vcn.inst[j].ring_jpeg;
> + ring->sched.ready = true;
> r = amdgpu_ring_test_ring(ring);
> if (r) {
> ring->sched.ready = false;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdkfd: Improve KFD IOCTL printing
On 2019-09-30 17:55, Zhao, Yong wrote:
> The code use hex define, so should the printing.
>
> Change-Id: Ia7cc7690553bb043915b3d8c0157216c64421a60
> Signed-off-by: Yong Zhao
Reviewed-by: Felix Kuehling
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index c28ba0c1d7ac..d9e36dbf13d5 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1840,7 +1840,7 @@ static long kfd_ioctl(struct file *filep, unsigned int
> cmd, unsigned long arg)
> } else
> goto err_i1;
>
> - dev_dbg(kfd_device, "ioctl cmd 0x%x (#%d), arg 0x%lx\n", cmd, nr, arg);
> + dev_dbg(kfd_device, "ioctl cmd 0x%x (#0x%x), arg 0x%lx\n", cmd, nr,
> arg);
>
> process = kfd_get_process(current);
> if (IS_ERR(process)) {
> @@ -1895,7 +1895,8 @@ static long kfd_ioctl(struct file *filep, unsigned int
> cmd, unsigned long arg)
> kfree(kdata);
>
> if (retcode)
> - dev_dbg(kfd_device, "ret = %d\n", retcode);
> + dev_dbg(kfd_device, "ioctl cmd (#0x%x), arg 0x%lx, ret = %d\n",
> + nr, arg, retcode);
>
> return retcode;
> }
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu : enable msix for amdgpu driver
Reviewed-by: Alex Deucher
From: amd-gfx on behalf of Liu, Shaoyun
Sent: Tuesday, October 1, 2019 4:03 PM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun
Subject: [PATCH] drm/amdgpu : enable msix for amdgpu driver
We might used out of the msi resources in some cloud project
which have a lot gpu devices(including PF and VF), msix can
provide enough resources from system level view
Change-Id: I9f03762074ac416c07f27b8f00c052ca93c7d6cb
Signed-off-by: shaoyunl
---
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index d1d5e7f..1bd27ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -245,8 +245,9 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
adev->irq.msi_enabled = false;
if (amdgpu_msi_ok(adev)) {
- int ret = pci_enable_msi(adev->pdev);
- if (!ret) {
+ int nvec = pci_alloc_irq_vectors(adev->pdev, 1,
pci_msix_vec_count(adev->pdev),
+ PCI_IRQ_MSI | PCI_IRQ_MSIX)
+ if (nvec > 0) {
adev->irq.msi_enabled = true;
dev_dbg(adev->dev, "amdgpu: using MSI.\n");
}
--
2.7.4
___
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/amdgpu : enable msix for amdgpu driver
We might used out of the msi resources in some cloud project
which have a lot gpu devices(including PF and VF), msix can
provide enough resources from system level view
Change-Id: I9f03762074ac416c07f27b8f00c052ca93c7d6cb
Signed-off-by: shaoyunl
---
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index d1d5e7f..1bd27ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -245,8 +245,9 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
adev->irq.msi_enabled = false;
if (amdgpu_msi_ok(adev)) {
- int ret = pci_enable_msi(adev->pdev);
- if (!ret) {
+ int nvec = pci_alloc_irq_vectors(adev->pdev, 1,
pci_msix_vec_count(adev->pdev),
+ PCI_IRQ_MSI | PCI_IRQ_MSIX)
+ if (nvec > 0) {
adev->irq.msi_enabled = true;
dev_dbg(adev->dev, "amdgpu: using MSI.\n");
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Export setup_vm_pt_regs() logic for mmhub 2.0
Reviewed-by: Alex Deucher
From: amd-gfx on behalf of Zhao, Yong
Sent: Friday, September 27, 2019 11:40 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhao, Yong
Subject: [PATCH] drm/amdgpu: Export setup_vm_pt_regs() logic for mmhub 2.0
The KFD code will call this function later.
Change-Id: I5993323603799963e9eb473852b6c72de2172ed6
Signed-off-by: Yong Zhao
---
drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c | 19 ---
drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.h | 2 ++
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
index 86ed8cb915a8..2eea702de8ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
@@ -31,20 +31,25 @@
#include "soc15_common.h"
-static void mmhub_v2_0_init_gart_pt_regs(struct amdgpu_device *adev)
+void mmhub_v2_0_setup_vm_pt_regs(struct amdgpu_device *adev, uint32_t vmid,
+ uint64_t page_table_base)
{
- uint64_t value = amdgpu_gmc_pd_addr(adev->gart.bo);
+ /* two registers distance between mmMMVM_CONTEXT0_* to
mmMMVM_CONTEXT1_* */
+ int offset = mmMMVM_CONTEXT1_PAGE_TABLE_BASE_ADDR_LO32
+ - mmMMVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32;
- WREG32_SOC15(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32,
-lower_32_bits(value));
+ WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32,
+ offset * vmid, lower_32_bits(page_table_base));
- WREG32_SOC15(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32,
-upper_32_bits(value));
+ WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32,
+ offset * vmid, upper_32_bits(page_table_base));
}
static void mmhub_v2_0_init_gart_aperture_regs(struct amdgpu_device *adev)
{
- mmhub_v2_0_init_gart_pt_regs(adev);
+ uint64_t pt_base = amdgpu_gmc_pd_addr(adev->gart.bo);
+
+ mmhub_v2_0_setup_vm_pt_regs(adev, 0, pt_base);
WREG32_SOC15(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_START_ADDR_LO32,
(u32)(adev->gmc.gart_start >> 12));
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.h
b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.h
index db16f3ece218..3ea4344f0315 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.h
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.h
@@ -31,5 +31,7 @@ void mmhub_v2_0_init(struct amdgpu_device *adev);
int mmhub_v2_0_set_clockgating(struct amdgpu_device *adev,
enum amd_clockgating_state state);
void mmhub_v2_0_get_clockgating(struct amdgpu_device *adev, u32 *flags);
+void mmhub_v2_0_setup_vm_pt_regs(struct amdgpu_device *adev, uint32_t vmid,
+ uint64_t page_table_base);
#endif
--
2.17.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] drm/amd/display: memory leak
Would you please review this patch? Thanks, Navid. On Mon, Sep 16, 2019 at 10:21 PM Navid Emamdoost wrote: > > In dcn*_clock_source_create when dcn20_clk_src_construct fails allocated > clk_src needs release. > > Signed-off-by: Navid Emamdoost > --- > drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 3 ++- > drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 1 + > drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 1 + > drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c | 1 + > drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c | 1 + > drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 1 + > drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 1 + > 7 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c > b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c > index 6248c8455314..21de230b303a 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c > @@ -667,7 +667,8 @@ struct clock_source *dce100_clock_source_create( > clk_src->base.dp_clk_src = dp_clk_src; > return _src->base; > } > - > + > + kfree(clk_src); > BREAK_TO_DEBUGGER(); > return NULL; > } > diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c > b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c > index 764329264c3b..0cb83b0e0e1e 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c > @@ -714,6 +714,7 @@ struct clock_source *dce110_clock_source_create( > return _src->base; > } > > + kfree(clk_src); > BREAK_TO_DEBUGGER(); > return NULL; > } > diff --git a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c > b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c > index c6136e0ed1a4..147d77173e2b 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c > @@ -687,6 +687,7 @@ struct clock_source *dce112_clock_source_create( > return _src->base; > } > > + kfree(clk_src); > BREAK_TO_DEBUGGER(); > return NULL; > } > diff --git a/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c > b/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c > index 4a6ba3173a5a..0b5eeff17d00 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c > @@ -500,6 +500,7 @@ static struct clock_source *dce120_clock_source_create( > return _src->base; > } > > + kfree(clk_src); > BREAK_TO_DEBUGGER(); > return NULL; > } > diff --git a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c > b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c > index 860a524ebcfa..952440893fbb 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c > @@ -701,6 +701,7 @@ struct clock_source *dce80_clock_source_create( > return _src->base; > } > > + kfree(clk_src); > BREAK_TO_DEBUGGER(); > return NULL; > } > diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c > b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c > index a12530a3ab9c..3f25e8da5396 100644 > --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c > @@ -786,6 +786,7 @@ struct clock_source *dcn10_clock_source_create( > return _src->base; > } > > + kfree(clk_src); > BREAK_TO_DEBUGGER(); > return NULL; > } > diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c > index b949e202d6cb..418fdcf1f492 100644 > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c > @@ -955,6 +955,7 @@ struct clock_source *dcn20_clock_source_create( > return _src->base; > } > > + kfree(clk_src) > BREAK_TO_DEBUGGER(); > return NULL; > } > -- > 2.17.1 > -- Navid.
[PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init
Notice that there is a *continue* statement in the middle of the
for loop and that prevents the code below from ever being reached:
r = amdgpu_ring_test_ring(ring);
if (r) {
ring->sched.ready = false;
goto done;
}
Fix this by removing the continue statement and updating ring->sched.ready
to true before calling amdgpu_ring_test_ring(ring).
Notice that this fix is based on
commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)")
Addresses-Coverity-ID 1485608 ("Structurally dead code")
Fixes: 28c17d72072b ("drm/amdgpu: add VCN2.5 basic supports")
Signed-off-by: Gustavo A. R. Silva
---
Any feedback is greatly appreciated.
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index 395c2259f979..47b0dcd59e13 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -258,6 +258,7 @@ static int vcn_v2_5_hw_init(void *handle)
adev->nbio_funcs->vcn_doorbell_range(adev, ring->use_doorbell,
ring->doorbell_index, j);
+ ring->sched.ready = true;
r = amdgpu_ring_test_ring(ring);
if (r) {
ring->sched.ready = false;
@@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle)
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
ring = >vcn.inst[j].ring_enc[i];
- ring->sched.ready = false;
- continue;
+ ring->sched.ready = true;
r = amdgpu_ring_test_ring(ring);
if (r) {
ring->sched.ready = false;
@@ -276,6 +276,7 @@ static int vcn_v2_5_hw_init(void *handle)
}
ring = >vcn.inst[j].ring_jpeg;
+ ring->sched.ready = true;
r = amdgpu_ring_test_ring(ring);
if (r) {
ring->sched.ready = false;
--
2.23.0
[PATCH AUTOSEL 4.4 06/15] drm/amdgpu: Check for valid number of registers to read
From: Trek [ Upstream commit 73d8e6c7b841d9bf298c8928f228fb433676635c ] Do not try to allocate any amount of memory requested by the user. Instead limit it to 128 registers. Actually the longest series of consecutive allowed registers are 48, mmGB_TILE_MODE0-31 and mmGB_MACROTILE_MODE0-15 (0x2644-0x2673). Bug: https://bugs.freedesktop.org/show_bug.cgi?id=111273 Signed-off-by: Trek Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index a5c8240784726..e35e603710b4d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -406,6 +406,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK) sh_num = 0x; + if (info->read_mmr_reg.count > 128) + return -EINVAL; + regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs), GFP_KERNEL); if (!regs) return -ENOMEM; -- 2.20.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH AUTOSEL 4.9 07/19] drm/amdgpu: Check for valid number of registers to read
From: Trek [ Upstream commit 73d8e6c7b841d9bf298c8928f228fb433676635c ] Do not try to allocate any amount of memory requested by the user. Instead limit it to 128 registers. Actually the longest series of consecutive allowed registers are 48, mmGB_TILE_MODE0-31 and mmGB_MACROTILE_MODE0-15 (0x2644-0x2673). Bug: https://bugs.freedesktop.org/show_bug.cgi?id=111273 Signed-off-by: Trek Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 3938fca1ea8e5..24941a7b659f4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -430,6 +430,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK) sh_num = 0x; + if (info->read_mmr_reg.count > 128) + return -EINVAL; + regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs), GFP_KERNEL); if (!regs) return -ENOMEM; -- 2.20.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH AUTOSEL 4.14 10/29] drm/amdgpu: Check for valid number of registers to read
From: Trek [ Upstream commit 73d8e6c7b841d9bf298c8928f228fb433676635c ] Do not try to allocate any amount of memory requested by the user. Instead limit it to 128 registers. Actually the longest series of consecutive allowed registers are 48, mmGB_TILE_MODE0-31 and mmGB_MACROTILE_MODE0-15 (0x2644-0x2673). Bug: https://bugs.freedesktop.org/show_bug.cgi?id=111273 Signed-off-by: Trek Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index e16229000a983..884ed359f2493 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -540,6 +540,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK) sh_num = 0x; + if (info->read_mmr_reg.count > 128) + return -EINVAL; + regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs), GFP_KERNEL); if (!regs) return -ENOMEM; -- 2.20.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH AUTOSEL 4.19 41/43] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
From: Hans de Goede
[ Upstream commit 9dbc88d013b79c62bd845cb9e7c0256e660967c5 ]
Bail from the pci_driver probe function instead of from the drm_driver
load function.
This avoid /dev/dri/card0 temporarily getting registered and then
unregistered again, sending unwanted add / remove udev events to
userspace.
Specifically this avoids triggering the (userspace) bug fixed by this
plymouth merge-request:
https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59
Note that despite that being an userspace bug, not sending unnecessary
udev events is a good idea in general.
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490
Reviewed-by: Michel Dänzer
Signed-off-by: Hans de Goede
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/radeon/radeon_drv.c | 31 +
drivers/gpu/drm/radeon/radeon_kms.c | 25 ---
2 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index 2a7977a23b31c..c26f09b47ecb2 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -340,8 +340,39 @@ static int radeon_kick_out_firmware_fb(struct pci_dev
*pdev)
static int radeon_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
+ unsigned long flags = 0;
int ret;
+ if (!ent)
+ return -ENODEV; /* Avoid NULL-ptr deref in drm_get_pci_dev */
+
+ flags = ent->driver_data;
+
+ if (!radeon_si_support) {
+ switch (flags & RADEON_FAMILY_MASK) {
+ case CHIP_TAHITI:
+ case CHIP_PITCAIRN:
+ case CHIP_VERDE:
+ case CHIP_OLAND:
+ case CHIP_HAINAN:
+ dev_info(>dev,
+"SI support disabled by module param\n");
+ return -ENODEV;
+ }
+ }
+ if (!radeon_cik_support) {
+ switch (flags & RADEON_FAMILY_MASK) {
+ case CHIP_KAVERI:
+ case CHIP_BONAIRE:
+ case CHIP_HAWAII:
+ case CHIP_KABINI:
+ case CHIP_MULLINS:
+ dev_info(>dev,
+"CIK support disabled by module param\n");
+ return -ENODEV;
+ }
+ }
+
if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c
b/drivers/gpu/drm/radeon/radeon_kms.c
index 6a8fb6fd183c3..3ff835767ac58 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -95,31 +95,6 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned
long flags)
struct radeon_device *rdev;
int r, acpi_status;
- if (!radeon_si_support) {
- switch (flags & RADEON_FAMILY_MASK) {
- case CHIP_TAHITI:
- case CHIP_PITCAIRN:
- case CHIP_VERDE:
- case CHIP_OLAND:
- case CHIP_HAINAN:
- dev_info(dev->dev,
-"SI support disabled by module param\n");
- return -ENODEV;
- }
- }
- if (!radeon_cik_support) {
- switch (flags & RADEON_FAMILY_MASK) {
- case CHIP_KAVERI:
- case CHIP_BONAIRE:
- case CHIP_HAWAII:
- case CHIP_KABINI:
- case CHIP_MULLINS:
- dev_info(dev->dev,
-"CIK support disabled by module param\n");
- return -ENODEV;
- }
- }
-
rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
if (rdev == NULL) {
return -ENOMEM;
--
2.20.1
[PATCH AUTOSEL 5.2 60/63] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
From: Hans de Goede
[ Upstream commit 9dbc88d013b79c62bd845cb9e7c0256e660967c5 ]
Bail from the pci_driver probe function instead of from the drm_driver
load function.
This avoid /dev/dri/card0 temporarily getting registered and then
unregistered again, sending unwanted add / remove udev events to
userspace.
Specifically this avoids triggering the (userspace) bug fixed by this
plymouth merge-request:
https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59
Note that despite that being an userspace bug, not sending unnecessary
udev events is a good idea in general.
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490
Reviewed-by: Michel Dänzer
Signed-off-by: Hans de Goede
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/radeon/radeon_drv.c | 31 +
drivers/gpu/drm/radeon/radeon_kms.c | 25 ---
2 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index 2e96c886392bd..5502e34288685 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -320,8 +320,39 @@ bool radeon_device_is_virtual(void);
static int radeon_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
+ unsigned long flags = 0;
int ret;
+ if (!ent)
+ return -ENODEV; /* Avoid NULL-ptr deref in drm_get_pci_dev */
+
+ flags = ent->driver_data;
+
+ if (!radeon_si_support) {
+ switch (flags & RADEON_FAMILY_MASK) {
+ case CHIP_TAHITI:
+ case CHIP_PITCAIRN:
+ case CHIP_VERDE:
+ case CHIP_OLAND:
+ case CHIP_HAINAN:
+ dev_info(>dev,
+"SI support disabled by module param\n");
+ return -ENODEV;
+ }
+ }
+ if (!radeon_cik_support) {
+ switch (flags & RADEON_FAMILY_MASK) {
+ case CHIP_KAVERI:
+ case CHIP_BONAIRE:
+ case CHIP_HAWAII:
+ case CHIP_KABINI:
+ case CHIP_MULLINS:
+ dev_info(>dev,
+"CIK support disabled by module param\n");
+ return -ENODEV;
+ }
+ }
+
if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c
b/drivers/gpu/drm/radeon/radeon_kms.c
index 6a8fb6fd183c3..3ff835767ac58 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -95,31 +95,6 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned
long flags)
struct radeon_device *rdev;
int r, acpi_status;
- if (!radeon_si_support) {
- switch (flags & RADEON_FAMILY_MASK) {
- case CHIP_TAHITI:
- case CHIP_PITCAIRN:
- case CHIP_VERDE:
- case CHIP_OLAND:
- case CHIP_HAINAN:
- dev_info(dev->dev,
-"SI support disabled by module param\n");
- return -ENODEV;
- }
- }
- if (!radeon_cik_support) {
- switch (flags & RADEON_FAMILY_MASK) {
- case CHIP_KAVERI:
- case CHIP_BONAIRE:
- case CHIP_HAWAII:
- case CHIP_KABINI:
- case CHIP_MULLINS:
- dev_info(dev->dev,
-"CIK support disabled by module param\n");
- return -ENODEV;
- }
- }
-
rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
if (rdev == NULL) {
return -ENOMEM;
--
2.20.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH AUTOSEL 4.19 13/43] drm/amdgpu: Fix KFD-related kernel oops on Hawaii
From: Felix Kuehling
[ Upstream commit dcafbd50f2e4d5cc964aae409fb5691b743fba23 ]
Hawaii needs to flush caches explicitly, submitting an IB in a user
VMID from kernel mode. There is no s_fence in this case.
Fixes: eb3961a57424 ("drm/amdgpu: remove fence context from the job")
Signed-off-by: Felix Kuehling
Reviewed-by: Christian König
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 51b5e977ca885..f4e9d1b10e3ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -139,7 +139,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
num_ibs,
/* ring tests don't use a job */
if (job) {
vm = job->vm;
- fence_ctx = job->base.s_fence->scheduled.context;
+ fence_ctx = job->base.s_fence ?
+ job->base.s_fence->scheduled.context : 0;
} else {
vm = NULL;
fence_ctx = 0;
--
2.20.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH AUTOSEL 4.19 14/43] drm/amdgpu: Check for valid number of registers to read
From: Trek [ Upstream commit 73d8e6c7b841d9bf298c8928f228fb433676635c ] Do not try to allocate any amount of memory requested by the user. Instead limit it to 128 registers. Actually the longest series of consecutive allowed registers are 48, mmGB_TILE_MODE0-31 and mmGB_MACROTILE_MODE0-15 (0x2644-0x2673). Bug: https://bugs.freedesktop.org/show_bug.cgi?id=111273 Signed-off-by: Trek Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index c0396e83f3526..fc93b103f7778 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -562,6 +562,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK) sh_num = 0x; + if (info->read_mmr_reg.count > 128) + return -EINVAL; + regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs), GFP_KERNEL); if (!regs) return -ENOMEM; -- 2.20.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH AUTOSEL 5.2 19/63] drm/amdgpu: Check for valid number of registers to read
From: Trek [ Upstream commit 73d8e6c7b841d9bf298c8928f228fb433676635c ] Do not try to allocate any amount of memory requested by the user. Instead limit it to 128 registers. Actually the longest series of consecutive allowed registers are 48, mmGB_TILE_MODE0-31 and mmGB_MACROTILE_MODE0-15 (0x2644-0x2673). Bug: https://bugs.freedesktop.org/show_bug.cgi?id=111273 Signed-off-by: Trek Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index b17d0545728ee..a103998f72128 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -662,6 +662,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK) sh_num = 0x; + if (info->read_mmr_reg.count > 128) + return -EINVAL; + regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs), GFP_KERNEL); if (!regs) return -ENOMEM; -- 2.20.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH AUTOSEL 5.2 18/63] drm/amdgpu: Fix KFD-related kernel oops on Hawaii
From: Felix Kuehling
[ Upstream commit dcafbd50f2e4d5cc964aae409fb5691b743fba23 ]
Hawaii needs to flush caches explicitly, submitting an IB in a user
VMID from kernel mode. There is no s_fence in this case.
Fixes: eb3961a57424 ("drm/amdgpu: remove fence context from the job")
Signed-off-by: Felix Kuehling
Reviewed-by: Christian König
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index fe393a46f8811..5eed2423dbb5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -141,7 +141,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
num_ibs,
/* ring tests don't use a job */
if (job) {
vm = job->vm;
- fence_ctx = job->base.s_fence->scheduled.context;
+ fence_ctx = job->base.s_fence ?
+ job->base.s_fence->scheduled.context : 0;
} else {
vm = NULL;
fence_ctx = 0;
--
2.20.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH AUTOSEL 5.3 68/71] drm/radeon: Bail earlier when radeon.cik_/si_support=0 is passed
From: Hans de Goede
[ Upstream commit 9dbc88d013b79c62bd845cb9e7c0256e660967c5 ]
Bail from the pci_driver probe function instead of from the drm_driver
load function.
This avoid /dev/dri/card0 temporarily getting registered and then
unregistered again, sending unwanted add / remove udev events to
userspace.
Specifically this avoids triggering the (userspace) bug fixed by this
plymouth merge-request:
https://gitlab.freedesktop.org/plymouth/plymouth/merge_requests/59
Note that despite that being an userspace bug, not sending unnecessary
udev events is a good idea in general.
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490
Reviewed-by: Michel Dänzer
Signed-off-by: Hans de Goede
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/radeon/radeon_drv.c | 31 +
drivers/gpu/drm/radeon/radeon_kms.c | 25 ---
2 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index a6cbe11f79c61..7033f3a38c878 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -325,8 +325,39 @@ bool radeon_device_is_virtual(void);
static int radeon_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
+ unsigned long flags = 0;
int ret;
+ if (!ent)
+ return -ENODEV; /* Avoid NULL-ptr deref in drm_get_pci_dev */
+
+ flags = ent->driver_data;
+
+ if (!radeon_si_support) {
+ switch (flags & RADEON_FAMILY_MASK) {
+ case CHIP_TAHITI:
+ case CHIP_PITCAIRN:
+ case CHIP_VERDE:
+ case CHIP_OLAND:
+ case CHIP_HAINAN:
+ dev_info(>dev,
+"SI support disabled by module param\n");
+ return -ENODEV;
+ }
+ }
+ if (!radeon_cik_support) {
+ switch (flags & RADEON_FAMILY_MASK) {
+ case CHIP_KAVERI:
+ case CHIP_BONAIRE:
+ case CHIP_HAWAII:
+ case CHIP_KABINI:
+ case CHIP_MULLINS:
+ dev_info(>dev,
+"CIK support disabled by module param\n");
+ return -ENODEV;
+ }
+ }
+
if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c
b/drivers/gpu/drm/radeon/radeon_kms.c
index 07f7ace42c4ba..e85c554eeaa94 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -100,31 +100,6 @@ int radeon_driver_load_kms(struct drm_device *dev,
unsigned long flags)
struct radeon_device *rdev;
int r, acpi_status;
- if (!radeon_si_support) {
- switch (flags & RADEON_FAMILY_MASK) {
- case CHIP_TAHITI:
- case CHIP_PITCAIRN:
- case CHIP_VERDE:
- case CHIP_OLAND:
- case CHIP_HAINAN:
- dev_info(dev->dev,
-"SI support disabled by module param\n");
- return -ENODEV;
- }
- }
- if (!radeon_cik_support) {
- switch (flags & RADEON_FAMILY_MASK) {
- case CHIP_KAVERI:
- case CHIP_BONAIRE:
- case CHIP_HAWAII:
- case CHIP_KABINI:
- case CHIP_MULLINS:
- dev_info(dev->dev,
-"CIK support disabled by module param\n");
- return -ENODEV;
- }
- }
-
rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
if (rdev == NULL) {
return -ENOMEM;
--
2.20.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH AUTOSEL 5.3 19/71] drm/amdgpu: Fix KFD-related kernel oops on Hawaii
From: Felix Kuehling
[ Upstream commit dcafbd50f2e4d5cc964aae409fb5691b743fba23 ]
Hawaii needs to flush caches explicitly, submitting an IB in a user
VMID from kernel mode. There is no s_fence in this case.
Fixes: eb3961a57424 ("drm/amdgpu: remove fence context from the job")
Signed-off-by: Felix Kuehling
Reviewed-by: Christian König
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 7850084a05e3a..60655834d6498 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -143,7 +143,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
num_ibs,
/* ring tests don't use a job */
if (job) {
vm = job->vm;
- fence_ctx = job->base.s_fence->scheduled.context;
+ fence_ctx = job->base.s_fence ?
+ job->base.s_fence->scheduled.context : 0;
} else {
vm = NULL;
fence_ctx = 0;
--
2.20.1
[PATCH AUTOSEL 5.3 20/71] drm/amdgpu: Check for valid number of registers to read
From: Trek [ Upstream commit 73d8e6c7b841d9bf298c8928f228fb433676635c ] Do not try to allocate any amount of memory requested by the user. Instead limit it to 128 registers. Actually the longest series of consecutive allowed registers are 48, mmGB_TILE_MODE0-31 and mmGB_MACROTILE_MODE0-15 (0x2644-0x2673). Bug: https://bugs.freedesktop.org/show_bug.cgi?id=111273 Signed-off-by: Trek Signed-off-by: Alex Deucher Signed-off-by: Sasha Levin --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 0cf7e8606fd3d..00beba533582c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -662,6 +662,9 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK) sh_num = 0x; + if (info->read_mmr_reg.count > 128) + return -EINVAL; + regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs), GFP_KERNEL); if (!regs) return -ENOMEM; -- 2.20.1
[PATCH 11/14] drm/amd/display: Write DSC enable to MST DPCD
From: David Francis
Rework the dm_helpers_write_dsc_enable callback to
handle the MST case.
Use the cached dsc_aux field.
Reviewed-by: Wenjing Liu
Signed-off-by: David Francis
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index fc537ae25eeb..bd694499e3de 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -37,6 +37,7 @@
#include "dc.h"
#include "amdgpu_dm.h"
#include "amdgpu_dm_irq.h"
+#include "amdgpu_dm_mst_types.h"
#include "dm_helpers.h"
@@ -518,8 +519,24 @@ bool dm_helpers_dp_write_dsc_enable(
)
{
uint8_t enable_dsc = enable ? 1 : 0;
+ struct amdgpu_dm_connector *aconnector;
+
+ if (!stream)
+ return false;
+
+ if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST) {
+ aconnector = (struct amdgpu_dm_connector
*)stream->dm_stream_context;
+
+ if (!aconnector->dsc_aux)
+ return false;
+
+ return (drm_dp_dpcd_write(aconnector->dsc_aux, DP_DSC_ENABLE,
_dsc, 1) >= 0);
+ }
+
+ if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT)
+ return dm_helpers_dp_write_dpcd(ctx, stream->link,
DP_DSC_ENABLE, _dsc, 1);
- return dm_helpers_dp_write_dpcd(ctx, stream->sink->link, DP_DSC_ENABLE,
_dsc, 1);
+ return false;
}
#endif
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 13/14] drm/amd/display: Recalculate VCPI slots for new DSC connectors
From: Mikita Lipski
Since for DSC MST connector's PBN is claculated differently
due to compression, we have to recalculate both PBN and
VCPI slots for that connector.
This patch proposes to use similair logic as in
dm_encoder_helper_atomic_chek, but since we do not know which
connectors will have DSC enabled we have to recalculate PBN only
after that's determined, which is done in
compute_mst_dsc_configs_for_state.
Cc: Jerry Zuo
Cc: Harry Wentland
Cc: Lyude Paul
Signed-off-by: Mikita Lipski
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 64 +--
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 --
2 files changed, 59 insertions(+), 11 deletions(-)
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 81e30918f9ec..7501ce2233ed 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4569,6 +4569,27 @@ static void dm_encoder_helper_disable(struct drm_encoder
*encoder)
}
+static int convert_dc_color_depth_into_bpc (enum dc_color_depth
display_color_depth)
+{
+ switch (display_color_depth) {
+ case COLOR_DEPTH_666:
+ return 6;
+ case COLOR_DEPTH_888:
+ return 8;
+ case COLOR_DEPTH_101010:
+ return 10;
+ case COLOR_DEPTH_121212:
+ return 12;
+ case COLOR_DEPTH_141414:
+ return 14;
+ case COLOR_DEPTH_161616:
+ return 16;
+ default:
+ break;
+ }
+ return 0;
+}
+
static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
struct drm_connector_state
*conn_state)
@@ -4616,6 +4637,36 @@ const struct drm_encoder_helper_funcs
amdgpu_dm_encoder_helper_funcs = {
.atomic_check = dm_encoder_helper_atomic_check
};
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+static int dm_update_mst_vcpi_slots_for_dsc(struct drm_atomic_state *state,
+ struct dc_state *dc_state)
+{
+ struct dc_stream_state *stream;
+ struct amdgpu_dm_connector *aconnector;
+ int i, clock = 0, bpp = 0;
+
+ for (i = 0; i < dc_state->stream_count; i++) {
+ stream = dc_state->streams[i];
+ aconnector = (struct amdgpu_dm_connector
*)stream->dm_stream_context;
+
+ if (stream && stream->timing.flags.DSC == 1) {
+ bpp =
convert_dc_color_depth_into_bpc(stream->timing.display_color_depth)* 3;
+ clock = stream->timing.pix_clk_100hz / 10;
+
+ aconnector->pbn = drm_dp_calc_pbn_mode(clock, bpp,
true);
+
+ aconnector->vcpi_slots =
drm_dp_atomic_find_vcpi_slots(state,
+
>mst_port->mst_mgr,
+ aconnector->port,
+ aconnector->pbn);
+ if (aconnector->vcpi_slots < 0)
+ return aconnector->vcpi_slots;
+ }
+ }
+ return 0;
+}
+#endif
+
static void dm_drm_plane_reset(struct drm_plane *plane)
{
struct dm_plane_state *amdgpu_state = NULL;
@@ -7629,11 +7680,6 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
- /* Perform validation of MST topology in the state*/
- ret = drm_dp_mst_atomic_check(state);
- if (ret)
- goto fail;
-
if (state->legacy_cursor_update) {
/*
* This is a fast cursor update coming from the plane update
@@ -7705,6 +7751,10 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
if (!compute_mst_dsc_configs_for_state(dm_state->context))
goto fail;
+
+ ret = dm_update_mst_vcpi_slots_for_dsc(state,
dm_state->context);
+ if (ret)
+ goto fail;
#endif
if (dc_validate_global_state(dc, dm_state->context, false) !=
DC_OK) {
ret = -EINVAL;
@@ -7734,6 +7784,10 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
dc_retain_state(old_dm_state->context);
}
}
+ /* Perform validation of MST topology in the state*/
+ ret = drm_dp_mst_atomic_check(state);
+ if (ret)
+ goto fail;
/* Store the overall update type for use later in atomic check. */
for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
diff --git
[PATCH 14/14] drm/amd/display: Trigger modesets on MST DSC connectors
From: David Francis
Whenever a connector on an MST network is attached, detached, or
undergoes a modeset, the DSC configs for each stream on that
topology will be recalculated. This can change their required
bandwidth, requiring a full reprogramming, as though a modeset
was performed, even if that stream did not change timing.
Therefore, whenever a crtc has drm_atomic_crtc_needs_modeset,
for each crtc that shares a MST topology with that stream and
supports DSC, add that crtc (and all affected connectors and
planes) to the atomic state and set mode_changed on its state
v2: Do this check only on Navi and before adding connectors
and planes on modesetting crtcs
Cc: Leo Li
Cc: Nicholas Kazlauskas
Cc: Lyude Paul
Signed-off-by: David Francis
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 +++
1 file changed, 79 insertions(+)
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 7501ce2233ed..371086a67c68 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6888,6 +6888,74 @@ static int do_aquire_global_lock(struct drm_device *dev,
return ret < 0 ? ret : 0;
}
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+/*
+ * TODO: This logic should at some point be moved into DRM
+ */
+static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct
drm_crtc *crtc)
+{
+ struct drm_connector *connector;
+ struct drm_connector_state *conn_state;
+ struct drm_connector_list_iter conn_iter;
+ struct drm_crtc_state *new_crtc_state;
+ struct amdgpu_dm_connector *aconnector = NULL, *aconnector_to_add;
+ int i, j;
+ struct drm_crtc *crtcs_affected[AMDGPU_MAX_CRTCS] = { 0 };
+
+ for_each_new_connector_in_state(state, connector, conn_state, i) {
+ if (conn_state->crtc != crtc)
+ continue;
+
+ aconnector = to_amdgpu_dm_connector(connector);
+ if (!aconnector->port)
+ aconnector = NULL;
+ else
+ break;
+ }
+
+ if (!aconnector)
+ return 0;
+
+ i = 0;
+ drm_connector_list_iter_begin(state->dev, _iter);
+ drm_for_each_connector_iter(connector, _iter) {
+ if (!connector->state || !connector->state->crtc)
+ continue;
+
+ aconnector_to_add = to_amdgpu_dm_connector(connector);
+ if (!aconnector_to_add->port)
+ continue;
+
+ if (aconnector_to_add->port->mgr != aconnector->port->mgr)
+ continue;
+
+ if (!aconnector_to_add->dc_sink)
+ continue;
+
+ if
(!aconnector_to_add->dc_sink->sink_dsc_caps.dsc_dec_caps.is_dsc_supported)
+ continue;
+
+ if (i >= AMDGPU_MAX_CRTCS)
+ continue;
+
+ crtcs_affected[i] = connector->state->crtc;
+ i++;
+ }
+ drm_connector_list_iter_end(_iter);
+
+ for (j = 0; j < i; j++) {
+ new_crtc_state = drm_atomic_get_crtc_state(state,
crtcs_affected[j]);
+ if (IS_ERR(new_crtc_state))
+ return PTR_ERR(new_crtc_state);
+
+ new_crtc_state->mode_changed = true;
+ }
+
+ return 0;
+
+}
+#endif
+
static void get_freesync_config_for_crtc(
struct dm_crtc_state *new_crtc_state,
struct dm_connector_state *new_con_state)
@@ -7577,6 +7645,17 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+ if (adev->asic_type >= CHIP_NAVI10) {
+ for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
+ if (drm_atomic_crtc_needs_modeset(new_crtc_state)) {
+ ret = add_affected_mst_dsc_crtcs(state, crtc);
+ if (ret)
+ goto fail;
+ }
+ }
+ }
+#endif
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
if (!drm_atomic_crtc_needs_modeset(new_crtc_state) &&
!new_crtc_state->color_mgmt_changed &&
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 04/14] drm/dp_mst: Add MST support to DP DPCD R/W functions
From: David Francis
Instead of having drm_dp_dpcd_read/write and
drm_dp_mst_dpcd_read/write as entry points into the
aux code, have drm_dp_dpcd_read/write handle both.
This means that DRM drivers can make MST DPCD read/writes.
v2: Fix spacing
v3: Dump dpcd access on MST read/writes
Reviewed-by: Lyude Paul
Reviewed-by: Harry Wentland
Signed-off-by: David Francis
---
drivers/gpu/drm/drm_dp_aux_dev.c | 12 ++--
drivers/gpu/drm/drm_dp_helper.c | 31 +--
2 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c
index 0cfb386754c3..418cad4f649a 100644
--- a/drivers/gpu/drm/drm_dp_aux_dev.c
+++ b/drivers/gpu/drm/drm_dp_aux_dev.c
@@ -163,11 +163,7 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb, struct
iov_iter *to)
break;
}
- if (aux_dev->aux->is_remote)
- res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf,
- todo);
- else
- res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
+ res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
if (res <= 0)
break;
@@ -215,11 +211,7 @@ static ssize_t auxdev_write_iter(struct kiocb *iocb,
struct iov_iter *from)
break;
}
- if (aux_dev->aux->is_remote)
- res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf,
- todo);
- else
- res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
+ res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf, todo);
if (res <= 0)
break;
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ffc68d305afe..af1cd968adfd 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -32,6 +32,8 @@
#include
#include
#include
+#include
+#include
#include "drm_crtc_helper_internal.h"
@@ -251,7 +253,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8
request,
/**
* drm_dp_dpcd_read() - read a series of bytes from the DPCD
- * @aux: DisplayPort AUX channel
+ * @aux: DisplayPort AUX channel (SST or MST)
* @offset: address of the (first) register to read
* @buffer: buffer to store the register values
* @size: number of bytes in @buffer
@@ -280,13 +282,18 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned
int offset,
* We just have to do it before any DPCD access and hope that the
* monitor doesn't power down exactly after the throw away read.
*/
- ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV, buffer,
-1);
- if (ret != 1)
- goto out;
+ if (!aux->is_remote) {
+ ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV,
+buffer, 1);
+ if (ret != 1)
+ goto out;
+ }
- ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, buffer,
-size);
+ if (aux->is_remote)
+ ret = drm_dp_mst_dpcd_read(aux, offset, buffer, size);
+ else
+ ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset,
+buffer, size);
out:
drm_dp_dump_access(aux, DP_AUX_NATIVE_READ, offset, buffer, ret);
@@ -296,7 +303,7 @@ EXPORT_SYMBOL(drm_dp_dpcd_read);
/**
* drm_dp_dpcd_write() - write a series of bytes to the DPCD
- * @aux: DisplayPort AUX channel
+ * @aux: DisplayPort AUX channel (SST or MST)
* @offset: address of the (first) register to write
* @buffer: buffer containing the values to write
* @size: number of bytes in @buffer
@@ -313,8 +320,12 @@ ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned
int offset,
{
int ret;
- ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer,
-size);
+ if (aux->is_remote)
+ ret = drm_dp_mst_dpcd_write(aux, offset, buffer, size);
+ else
+ ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_WRITE, offset,
+buffer, size);
+
drm_dp_dump_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer, ret);
return ret;
}
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 09/14] drm/amd/display: Initialize DSC PPS variables to 0
From: David Francis
For DSC MST, sometimes monitors would break out
in full-screen static. The issue traced back to the
PPS generation code, where these variables were being used
uninitialized and were picking up garbage.
memset to 0 to avoid this
Reviewed-by: Nicholas Kazlauskas
Signed-off-by: David Francis
---
drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c | 3 +++
drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
index a519dbc5ecb6..5d6cbaebebc0 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
@@ -496,6 +496,9 @@ bool dp_set_dsc_pps_sdp(struct pipe_ctx *pipe_ctx, bool
enable)
struct dsc_config dsc_cfg;
uint8_t dsc_packed_pps[128];
+ memset(_cfg, 0, sizeof(dsc_cfg));
+ memset(dsc_packed_pps, 0, 128);
+
/* Enable DSC hw block */
dsc_cfg.pic_width = stream->timing.h_addressable +
stream->timing.h_border_left + stream->timing.h_border_right;
dsc_cfg.pic_height = stream->timing.v_addressable +
stream->timing.v_border_top + stream->timing.v_border_bottom;
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c
index 63eb377ed9c0..296eeff00296 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c
@@ -207,6 +207,9 @@ static bool dsc2_get_packed_pps(struct
display_stream_compressor *dsc, const str
struct dsc_reg_values dsc_reg_vals;
struct dsc_optc_config dsc_optc_cfg;
+ memset(_reg_vals, 0, sizeof(dsc_reg_vals));
+ memset(_optc_cfg, 0, sizeof(dsc_optc_cfg));
+
DC_LOG_DSC("Getting packed DSC PPS for DSC Config:");
dsc_config_log(dsc, dsc_cfg);
DC_LOG_DSC("DSC Picture Parameter Set (PPS):");
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 02/14] drm/dp_mst: Add PBN calculation for DSC modes
From: David Francis
With DSC, bpp can be fractional in multiples of 1/16.
Change drm_dp_calc_pbn_mode to reflect this, adding a new
parameter bool dsc. When this parameter is true, treat the
bpp parameter as having units not of bits per pixel, but
1/16 of a bit per pixel
v2: Don't add separate function for this
Reviewed-by: Manasi Navare
Reviewed-by: Lyude Paul
Reviewed-by: Harry Wentland
Signed-off-by: David Francis
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 2 +-
drivers/gpu/drm/drm_dp_mst_topology.c| 16
drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 ++-
drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +-
include/drm/drm_dp_mst_helper.h | 3 +--
6 files changed, 19 insertions(+), 10 deletions(-)
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 3fc1afccbb33..59114b52090d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4597,7 +4597,7 @@ static int dm_encoder_helper_atomic_check(struct
drm_encoder *encoder,
if(!state->duplicated) {
bpp = (uint8_t)connector->display_info.bpc * 3;
clock = adjusted_mode->clock;
- aconnector->pbn = drm_dp_calc_pbn_mode(clock, bpp);
+ aconnector->pbn = drm_dp_calc_pbn_mode(clock, bpp, false);
}
aconnector->vcpi_slots = drm_dp_atomic_find_vcpi_slots(state,
mst_mgr,
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 82add736e17d..3e7b7553cf4d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3534,10 +3534,11 @@ EXPORT_SYMBOL(drm_dp_check_act_status);
* drm_dp_calc_pbn_mode() - Calculate the PBN for a mode.
* @clock: dot clock for the mode
* @bpp: bpp for the mode.
+ * @dsc: DSC mode. If true, bpp has units of 1/16 of a bit per pixel
*
* This uses the formula in the spec to calculate the PBN value for a mode.
*/
-int drm_dp_calc_pbn_mode(int clock, int bpp)
+int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
{
u64 kbps;
s64 peak_kbps;
@@ -3555,11 +3556,18 @@ int drm_dp_calc_pbn_mode(int clock, int bpp)
* peak_kbps *= (1006/1000)
* peak_kbps *= (64/54)
* peak_kbps *= 8convert to bytes
+*
+* If the bpp is in units of 1/16, further divide by 16. Put this
+* factor in the numerator rather than the denominator to avoid
+* integer overflow
*/
numerator = 64 * 1006;
denominator = 54 * 8 * 1000 * 1000;
+ if (dsc)
+ numerator /= 16;
+
kbps *= numerator;
peak_kbps = drm_fixp_from_fraction(kbps, denominator);
@@ -3570,19 +3578,19 @@ EXPORT_SYMBOL(drm_dp_calc_pbn_mode);
static int test_calc_pbn_mode(void)
{
int ret;
- ret = drm_dp_calc_pbn_mode(154000, 30);
+ ret = drm_dp_calc_pbn_mode(154000, 30, false);
if (ret != 689) {
DRM_ERROR("PBN calculation test failed - clock %d, bpp %d,
expected PBN %d, actual PBN %d.\n",
154000, 30, 689, ret);
return -EINVAL;
}
- ret = drm_dp_calc_pbn_mode(234000, 30);
+ ret = drm_dp_calc_pbn_mode(234000, 30, false);
if (ret != 1047) {
DRM_ERROR("PBN calculation test failed - clock %d, bpp %d,
expected PBN %d, actual PBN %d.\n",
234000, 30, 1047, ret);
return -EINVAL;
}
- ret = drm_dp_calc_pbn_mode(297000, 24);
+ ret = drm_dp_calc_pbn_mode(297000, 24, false);
if (ret != 1063) {
DRM_ERROR("PBN calculation test failed - clock %d, bpp %d,
expected PBN %d, actual PBN %d.\n",
297000, 24, 1063, ret);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 2c5ac3dd647f..dfac450841df 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -61,7 +61,8 @@ static int intel_dp_mst_compute_link_config(struct
intel_encoder *encoder,
crtc_state->pipe_bpp = bpp;
crtc_state->pbn =
drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock,
- crtc_state->pipe_bpp);
+ crtc_state->pipe_bpp,
+ false);
slots = drm_dp_atomic_find_vcpi_slots(state, _dp->mst_mgr,
port, crtc_state->pbn);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index
[PATCH 07/14] drm/dp_mst: Add new quirk for Synaptics MST hubs
From: Mikita Lipski
Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not
support virtual DPCD registers, but do support DSC.
The DSC caps can be read from the physical aux,
like in SST DSC. These hubs have many different
DEVICE_IDs. Add a new quirk to detect this case.
Reviewed-by: Wenjing Liu
Reviewed-by: Lyude Paul
Signed-off-by: David Francis
---
drivers/gpu/drm/drm_dp_helper.c | 2 ++
drivers/gpu/drm/drm_dp_mst_topology.c | 27 +++
include/drm/drm_dp_helper.h | 7 +++
3 files changed, 36 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index af1cd968adfd..02fa8c3d9a82 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1271,6 +1271,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
{ OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false,
BIT(DP_DPCD_QUIRK_NO_PSR) },
/* CH7511 seems to leave SINK_COUNT zeroed */
{ OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'),
false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
+ /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */
+ { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true,
BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) },
};
#undef OUI
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index d8f9ba27b559..d5df02315e14 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4222,6 +4222,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct
drm_dp_mst_port *port)
{
struct drm_dp_mst_port *immediate_upstream_port;
struct drm_dp_mst_port *fec_port;
+ struct drm_dp_desc desc = { 0 };
if (!port)
return NULL;
@@ -4274,6 +4275,32 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct
drm_dp_mst_port *port)
if (drm_dp_mst_is_virtual_dpcd(port))
return >aux;
+ /*
+* Synaptics quirk
+* Applies to ports for which:
+* - Physical aux has Synaptics OUI
+* - DPv1.4 or higher
+* - Port is on primary branch device
+* - Not a VGA adapter (DP_DWN_STRM_PORT_TYPE_ANALOG)
+*/
+ if (!drm_dp_read_desc(port->mgr->aux, , true))
+ return NULL;
+
+ if (drm_dp_has_quirk(, DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) &&
+ port->mgr->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14 &&
+ port->parent == port->mgr->mst_primary) {
+ u8 downstreamport;
+
+ if (drm_dp_dpcd_read(>aux, DP_DOWNSTREAMPORT_PRESENT,
+, 1) < 0)
+ return NULL;
+
+ if ((downstreamport & DP_DWN_STRM_PORT_PRESENT) &&
+ ((downstreamport & DP_DWN_STRM_PORT_TYPE_MASK)
+!= DP_DWN_STRM_PORT_TYPE_ANALOG))
+ return port->mgr->aux;
+ }
+
return NULL;
}
EXPORT_SYMBOL(drm_dp_mst_dsc_aux_for_port);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 8364502f92cf..a1331b08705f 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1434,6 +1434,13 @@ enum drm_dp_quirk {
* The driver should ignore SINK_COUNT during detection.
*/
DP_DPCD_QUIRK_NO_SINK_COUNT,
+ /**
+* @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD:
+*
+* The device supports MST DSC despite not supporting Virtual DPCD.
+* The DSC caps can be read from the physical aux instead.
+*/
+ DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD,
};
/**
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 12/14] drm/amd/display: MST DSC compute fair share
From: David Francis
If there is limited link bandwidth on a MST network,
it must be divided fairly between the streams on that network
Implement an algorithm to determine the correct DSC config
for each stream
The algorithm:
This
[ ] ( )
represents the range of bandwidths possible for a given stream.
The [] area represents the range of DSC configs, and the ()
represents no DSC. The bandwidth used increases from left to right.
First, try disabling DSC on all streams
[ ] (|)
[ ](|)
Check this against the bandwidth limits of the link and each branch
(including each endpoint). If it passes, the job is done
Second, try maximum DSC compression on all streams
that support DSC
[| ]( )
[|] ( )
If this does not pass, then enabling this combination of streams
is impossible
Otherwise, divide the remaining bandwidth evenly amongst the streams
[| ] ( )
[| ]( )
If one or more of the streams reach minimum compression, evenly
divide the reamining bandwidth amongst the remaining streams
[|] ( )
[ |] ( )
[ | ] ( )
[ | ] ( )
If all streams can reach minimum compression, disable compression
greedily
[ |] ( )
[|]( )
[ ](|)
Perform this algorithm on each full update, on each MST link
with at least one DSC stream on it
After the configs are computed, call
dcn20_add_dsc_to_stream_resource on each stream with DSC enabled.
It is only after all streams are created that we can know which
of them will need DSC.
Do all of this at the end of amdgpu atomic check. If it fails,
fail check; This combination of timings cannot be supported.
Reviewed-by: Wenjing Liu
Signed-off-by: David Francis
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 386 ++
.../display/amdgpu_dm/amdgpu_dm_mst_types.h | 4 +
.../drm/amd/display/dc/dcn20/dcn20_resource.c | 7 +-
.../drm/amd/display/dc/dcn20/dcn20_resource.h | 1 +
5 files changed, 400 insertions(+), 2 deletions(-)
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 59114b52090d..81e30918f9ec 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7702,6 +7702,10 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+ if (!compute_mst_dsc_configs_for_state(dm_state->context))
+ goto fail;
+#endif
if (dc_validate_global_state(dc, dm_state->context, false) !=
DC_OK) {
ret = -EINVAL;
goto fail;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 270d972c9c3c..6b9696889668 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -38,6 +38,8 @@
#include "i2caux_interface.h"
+#include "dc/dcn20/dcn20_resource.h"
+
/* #define TRACE_DPCD */
#ifdef TRACE_DPCD
@@ -490,3 +492,387 @@ void amdgpu_dm_initialize_dp_connector(struct
amdgpu_display_manager *dm,
aconnector->connector_id);
}
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+struct dsc_mst_fairness_params {
+ struct dc_crtc_timing *timing;
+ struct dc_sink *sink;
+ struct dc_dsc_bw_range bw_range;
+ bool compression_possible;
+ struct drm_dp_mst_port *port;
+};
+
+struct dsc_mst_fairness_vars {
+ int pbn;
+ bool dsc_enabled;
+ int bpp_x16;
+};
+
+static bool port_downstream_of_branch(struct drm_dp_mst_port *port,
+ struct drm_dp_mst_branch *branch)
+{
+ while (port->parent) {
+ if (port->parent == branch)
+ return true;
+
+ if (port->parent->port_parent)
+ port = port->parent->port_parent;
+ else
+ break;
+ }
+ return false;
+}
+
+static bool check_pbn_limit_on_branch(struct drm_dp_mst_branch *branch,
+ struct dsc_mst_fairness_params *params,
+ struct dsc_mst_fairness_vars *vars, int count)
+{
+ struct drm_dp_mst_port *port;
+ int i;
+ int pbn_limit = 0;
+ int pbn_used = 0;
+
+ list_for_each_entry(port, >ports, next) {
+ if (port->mstb)
+ if (!check_pbn_limit_on_branch(port->mstb, params,
vars, count))
+ return false;
+
+ if
[PATCH 01/14] drm/amd/display: Add MST atomic routines
From: Mikita Lipski
- Adding encoder atomic check to find vcpi slots for a connector
- Using DRM helper functions to calculate PBN
- Adding connector atomic check to release vcpi slots if connector
loses CRTC
- Calculate PBN and VCPI slots only once during atomic
check and store them on amdgpu connector to eliminate
redundant calculation
- Call drm_dp_mst_atomic_check to verify validity of MST topology
during state atomic check
v2: squashed previous 3 separate patches, removed DSC PBN calculation,
and added PBN and VCPI slots properties to amdgpu connector
Cc: Jerry Zuo
Cc: Harry Wentland
Cc: Nicholas Kazlauskas
Cc: Lyude Paul
Signed-off-by: Mikita Lipski
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 +++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 ++
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 42 ++-
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 32 ++
4 files changed, 81 insertions(+), 39 deletions(-)
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 239b1ae86007..3fc1afccbb33 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4573,6 +4573,41 @@ static int dm_encoder_helper_atomic_check(struct
drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
struct drm_connector_state
*conn_state)
{
+ struct drm_atomic_state *state = crtc_state->state;
+ struct drm_connector *connector = conn_state->connector;
+ struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
+ struct dm_crtc_state *dm_new_crtc_state = to_dm_crtc_state(crtc_state);
+ const struct drm_display_mode *adjusted_mode =
_state->adjusted_mode;
+ struct drm_dp_mst_topology_mgr *mst_mgr;
+ struct drm_dp_mst_port *mst_port;
+ int clock, bpp = 0;
+
+ if (!dm_new_crtc_state)
+ return 0;
+
+ if (!aconnector->port || !aconnector->dc_sink)
+ return 0;
+
+ mst_port = aconnector->port;
+ mst_mgr = >mst_port->mst_mgr;
+
+ if (!crtc_state->connectors_changed && !crtc_state->mode_changed)
+ return 0;
+
+ if(!state->duplicated) {
+ bpp = (uint8_t)connector->display_info.bpc * 3;
+ clock = adjusted_mode->clock;
+ aconnector->pbn = drm_dp_calc_pbn_mode(clock, bpp);
+ }
+ aconnector->vcpi_slots = drm_dp_atomic_find_vcpi_slots(state,
+ mst_mgr,
+ mst_port,
+ aconnector->pbn);
+
+ if (aconnector->vcpi_slots < 0) {
+ DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n",
aconnector->vcpi_slots);
+ return aconnector->vcpi_slots;
+ }
return 0;
}
@@ -5197,6 +5232,8 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
aconnector->base.dpms = DRM_MODE_DPMS_OFF;
aconnector->hpd.hpd = AMDGPU_HPD_NONE; /* not used */
aconnector->audio_inst = -1;
+ aconnector->vcpi_slots = 0;
+ aconnector->pbn = 0;
mutex_init(>hpd_lock);
/*
@@ -7592,6 +7629,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
+ /* Perform validation of MST topology in the state*/
+ ret = drm_dp_mst_atomic_check(state);
+ if (ret)
+ goto fail;
+
if (state->legacy_cursor_update) {
/*
* This is a fast cursor update coming from the plane update
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index c6fdebee7189..3ce104324096 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -280,6 +280,10 @@ struct amdgpu_dm_connector {
struct amdgpu_dm_connector *mst_port;
struct amdgpu_encoder *mst_encoder;
+ /* MST specific */
+ uint32_t vcpi_slots;
+ uint32_t pbn;
+
/* TODO see if we can merge with ddc_bus or make a dm_connector */
struct amdgpu_i2c_adapter *i2c;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 11e5784aa62a..5256abe32e92 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -184,11 +184,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_mgr *mst_mgr;
struct drm_dp_mst_port *mst_port;
- int slots = 0;
[PATCH 06/14] drm/dp_mst: Add helpers for MST DSC and virtual DPCD aux
From: David Francis
Add drm_dp_mst_dsc_aux_for_port. To enable DSC, the DSC_ENABLED
register might have to be written on the leaf port's DPCD,
its parent's DPCD, or the MST manager's DPCD. This function
finds the correct aux for the job.
As part of this, add drm_dp_mst_is_virtual_dpcd. Virtual DPCD
is a DP feature new in DP v1.4, which exposes certain DPCD
registers on virtual ports.
v2: Remember to unlock mutex on all paths
v3: Refactor to match coding style and increase brevity
Reviewed-by: Lyude Paul
Reviewed-by: Wenjing Liu
Signed-off-by: David Francis
---
drivers/gpu/drm/drm_dp_mst_topology.c | 127 ++
include/drm/drm_dp_mst_helper.h | 2 +
2 files changed, 129 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 502923c24450..d8f9ba27b559 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4150,3 +4150,130 @@ static void drm_dp_mst_unregister_i2c_bus(struct
drm_dp_aux *aux)
{
i2c_del_adapter(>ddc);
}
+
+/**
+ * drm_dp_mst_is_virtual_dpcd() - Is the given port a virtual DP Peer Device
+ * @port: The port to check
+ *
+ * A single physical MST hub object can be represented in the topology
+ * by multiple branches, with virtual ports between those branches.
+ *
+ * As of DP1.4, An MST hub with internal (virtual) ports must expose
+ * certain DPCD registers over those ports. See sections 2.6.1.1.1
+ * and 2.6.1.1.2 of Display Port specification v1.4 for details.
+ *
+ * May acquire mgr->lock
+ *
+ * Returns:
+ * true if the port is a virtual DP peer device, false otherwise
+ */
+static bool drm_dp_mst_is_virtual_dpcd(struct drm_dp_mst_port *port)
+{
+ struct drm_dp_mst_port *downstream_port;
+
+ if (!port || port->dpcd_rev < DP_DPCD_REV_14)
+ return false;
+
+ /* Virtual DP Sink (Internal Display Panel) */
+ if (port->port_num >= 8)
+ return true;
+
+ /* DP-to-HDMI Protocol Converter */
+ if (port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV &&
+ !port->mcs &&
+ port->ldps)
+ return true;
+
+ /* DP-to-DP */
+ mutex_lock(>mgr->lock);
+ if (port->pdt == DP_PEER_DEVICE_MST_BRANCHING &&
+ port->mstb &&
+ port->mstb->num_ports == 2) {
+ list_for_each_entry(downstream_port, >mstb->ports, next) {
+ if (downstream_port->pdt == DP_PEER_DEVICE_SST_SINK &&
+ !downstream_port->input) {
+ mutex_unlock(>mgr->lock);
+ return true;
+ }
+ }
+ }
+ mutex_unlock(>mgr->lock);
+
+ return false;
+}
+
+/**
+ * drm_dp_mst_dsc_aux_for_port() - Find the correct aux for DSC
+ * @port: The port to check. A leaf of the MST tree with an attached display.
+ *
+ * Depending on the situation, DSC may be enabled via the endpoint aux,
+ * the immediately upstream aux, or the connector's physical aux.
+ *
+ * This is both the correct aux to read DSC_CAPABILITY and the
+ * correct aux to write DSC_ENABLED.
+ *
+ * This operation can be expensive (up to four aux reads), so
+ * the caller should cache the return.
+ *
+ * Returns:
+ * NULL if DSC cannot be enabled on this port, otherwise the aux device
+ */
+struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
+{
+ struct drm_dp_mst_port *immediate_upstream_port;
+ struct drm_dp_mst_port *fec_port;
+
+ if (!port)
+ return NULL;
+
+ if (port->parent)
+ immediate_upstream_port = port->parent->port_parent;
+ else
+ immediate_upstream_port = NULL;
+
+ fec_port = immediate_upstream_port;
+ while (fec_port) {
+ /*
+* Each physical link (i.e. not a virtual port) between the
+* output and the primary device must support FEC
+*/
+ if (!drm_dp_mst_is_virtual_dpcd(fec_port) &&
+ !fec_port->fec_capable)
+ return NULL;
+
+ fec_port = fec_port->parent->port_parent;
+ }
+
+ /* DP-to-DP peer device */
+ if (drm_dp_mst_is_virtual_dpcd(immediate_upstream_port)) {
+ u8 upstream_dsc;
+ u8 endpoint_dsc;
+ u8 endpoint_fec;
+
+ if (drm_dp_dpcd_read(>aux,
+DP_DSC_SUPPORT, _dsc, 1) < 0)
+ return NULL;
+ if (drm_dp_dpcd_read(>aux,
+DP_FEC_CAPABILITY, _fec, 1) < 0)
+ return NULL;
+ if (drm_dp_dpcd_read(_upstream_port->aux,
+DP_DSC_SUPPORT, _dsc, 1) < 0)
+ return NULL;
+
+ /* Enpoint decompression with DP-to-DP peer device */
+ if
[PATCH v2 00/14] DSC MST support for AMDGPU
From: Mikita Lipski This set of patches is a continuation of DSC enablement patches for AMDGPU. This set enables DSC on MST. It also contains implementation of both encoder and connector atomic check routines. First 12 patches have been introduced in multiple iterations to the mailing list before. These patches were developed by David Francis as part of his work on DSC. Other 2 patches add atomic check functionality to encoder and connector to allocate and release VCPI slots on each state atomic check. These changes utilize newly added drm_mst_helper functions for better tracking of VCPI slots. v2: squashed previously 3 separate atomic check patches, separate atomic check for dsc connectors, track vcpi and pbn on connectors. David Francis (12): drm/dp_mst: Add PBN calculation for DSC modes drm/dp_mst: Parse FEC capability on MST ports drm/dp_mst: Add MST support to DP DPCD R/W functions drm/dp_mst: Fill branch->num_ports drm/dp_mst: Add helpers for MST DSC and virtual DPCD aux drm/dp_mst: Add new quirk for Synaptics MST hubs drm/amd/display: Use correct helpers to compute timeslots drm/amd/display: Initialize DSC PPS variables to 0 drm/amd/display: Validate DSC caps on MST endpoints drm/amd/display: Write DSC enable to MST DPCD drm/amd/display: MST DSC compute fair share drm/amd/display: Trigger modesets on MST DSC connectors Mikita Lipski (2): drm/amd/display: Add MST atomic routines drm/amd/display: Recalculate VCPI slots for new DSC connectors .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 179 +++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 7 + .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 63 +-- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 449 +- .../display/amdgpu_dm/amdgpu_dm_mst_types.h | 4 + .../drm/amd/display/dc/core/dc_link_hwss.c| 3 + .../gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c | 3 + .../drm/amd/display/dc/dcn20/dcn20_resource.c | 7 +- .../drm/amd/display/dc/dcn20/dcn20_resource.h | 1 + drivers/gpu/drm/drm_dp_aux_dev.c | 12 +- drivers/gpu/drm/drm_dp_helper.c | 33 +- drivers/gpu/drm/drm_dp_mst_topology.c | 174 ++- drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 +- drivers/gpu/drm/radeon/radeon_dp_mst.c| 2 +- include/drm/drm_dp_helper.h | 7 + include/drm/drm_dp_mst_helper.h | 8 +- 17 files changed, 885 insertions(+), 73 deletions(-) -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 10/14] drm/amd/display: Validate DSC caps on MST endpoints
From: David Francis
During MST mode enumeration, if a new dc_sink is created,
populate it with dsc caps as appropriate.
Use drm_dp_mst_dsc_aux_for_port to get the raw caps,
then parse them onto dc_sink with dc_dsc_parse_dsc_dpcd.
Reviewed-by: Wenjing Liu
Signed-off-by: David Francis
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 ++
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 31 ++-
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 3ce104324096..8bae6f264ef1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -279,6 +279,9 @@ struct amdgpu_dm_connector {
struct drm_dp_mst_port *port;
struct amdgpu_dm_connector *mst_port;
struct amdgpu_encoder *mst_encoder;
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+ struct drm_dp_aux *dsc_aux;
+#endif
/* MST specific */
uint32_t vcpi_slots;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 7f3ce29bd14c..270d972c9c3c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -25,6 +25,7 @@
#include
#include
+#include
#include "dm_services.h"
#include "amdgpu.h"
#include "amdgpu_dm.h"
@@ -187,6 +188,28 @@ static const struct drm_connector_funcs
dm_dp_mst_connector_funcs = {
.early_unregister = amdgpu_dm_mst_connector_early_unregister,
};
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+static bool validate_dsc_caps_on_connector(struct amdgpu_dm_connector
*aconnector)
+{
+ struct dc_sink *dc_sink = aconnector->dc_sink;
+ struct drm_dp_mst_port *port = aconnector->port;
+ u8 dsc_caps[16] = { 0 };
+
+ aconnector->dsc_aux = drm_dp_mst_dsc_aux_for_port(port);
+
+ if (!aconnector->dsc_aux)
+ return false;
+
+ if (drm_dp_dpcd_read(aconnector->dsc_aux, DP_DSC_SUPPORT, dsc_caps, 16)
< 0)
+ return false;
+
+ if (!dc_dsc_parse_dsc_dpcd(dsc_caps, NULL,
_sink->sink_dsc_caps.dsc_dec_caps))
+ return false;
+
+ return true;
+}
+#endif
+
static int dm_dp_mst_get_modes(struct drm_connector *connector)
{
struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
@@ -229,10 +252,16 @@ static int dm_dp_mst_get_modes(struct drm_connector
*connector)
/* dc_link_add_remote_sink returns a new reference */
aconnector->dc_sink = dc_sink;
- if (aconnector->dc_sink)
+ if (aconnector->dc_sink) {
amdgpu_dm_update_freesync_caps(
connector, aconnector->edid);
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+ if (!validate_dsc_caps_on_connector(aconnector))
+ memset(>dc_sink->sink_dsc_caps,
+ 0,
sizeof(aconnector->dc_sink->sink_dsc_caps));
+#endif
+ }
}
drm_connector_update_edid_property(
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 08/14] drm/amd/display: Use correct helpers to compute timeslots
From: David Francis
We were using drm helpers to convert a timing into its
bandwidth, its bandwidth into pbn, and its pbn into timeslots
These helpers
-Did not take DSC timings into account
-Used the link rate and lane count of the link's aux device,
which are not the same as the link's current cap
-Did not take FEC into account (FEC reduces the PBN per timeslot)
For converting timing into PBN, use the new function
drm_dp_calc_pbn_mode_dsc that handles the DSC case
For converting PBN into time slots, amdgpu doesn't use the
'correct' atomic method (drm_dp_atomic_find_vcpi_slots), so
don't add a new helper to cover our approach. Use the same
means of calculating pbn per time slot as the DSC code.
Cc: Jerry Zuo
Cc: Nicholas Kazlauskas
Signed-off-by: David Francis
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 5256abe32e92..fc537ae25eeb 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -185,6 +185,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
struct drm_dp_mst_topology_mgr *mst_mgr;
struct drm_dp_mst_port *mst_port;
bool ret;
+ int pbn_per_timeslot = 0;
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
@@ -200,6 +201,11 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
if (enable) {
+ /* Convert kilobits per second / 64 (for 64 timeslots) to pbn
(54/64 megabytes per second) */
+ pbn_per_timeslot = dc_link_bandwidth_kbps(
+ stream->link,
dc_link_get_link_cap(stream->link)) / (8 * 1000 * 54);
+ aconnector->vcpi_slots = DIV_ROUND_UP(aconnector->pbn,
pbn_per_timeslot);
+
ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port,
aconnector->pbn,
aconnector->vcpi_slots);
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 05/14] drm/dp_mst: Fill branch->num_ports
From: David Francis This field on drm_dp_mst_branch was never filled It is initialized to zero when the port is kzallocced. When a port is added to the list, increment num_ports, and when a port is removed from the list, decrement num_ports. v2: remember to decrement on port removal v3: don't explicitly init to 0 Reviewed-by: Lyude Paul Reviewed-by: Harry Wentland Signed-off-by: David Francis --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 9f3604355705..502923c24450 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1669,6 +1669,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, mutex_lock(>mgr->lock); drm_dp_mst_topology_get_port(port); list_add(>next, >ports); + mstb->num_ports++; mutex_unlock(>mgr->lock); } @@ -1703,6 +1704,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, /* remove it from the port list */ mutex_lock(>mgr->lock); list_del(>next); + mstb->num_ports--; mutex_unlock(>mgr->lock); /* drop port list reference */ drm_dp_mst_topology_put_port(port); -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 03/14] drm/dp_mst: Parse FEC capability on MST ports
From: David Francis
As of DP1.4, ENUM_PATH_RESOURCES returns a bit indicating
if FEC can be supported up to that point in the MST network.
The bit is the first byte of the ENUM_PATH_RESOURCES ack reply,
bottom-most bit (refer to section 2.11.9.4 of DP standard,
v1.4)
That value is needed for FEC and DSC support
Store it on drm_dp_mst_port
Reviewed-by: Lyude Paul
Reviewed-by: Harry Wentland
Signed-off-by: David Francis
---
drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++
include/drm/drm_dp_mst_helper.h | 3 +++
2 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 3e7b7553cf4d..9f3604355705 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -553,6 +553,7 @@ static bool
drm_dp_sideband_parse_enum_path_resources_ack(struct drm_dp_sideband
{
int idx = 1;
repmsg->u.path_resources.port_number = (raw->msg[idx] >> 4) & 0xf;
+ repmsg->u.path_resources.fec_capable = raw->msg[idx] & 0x1;
idx++;
if (idx > raw->curlen)
goto fail_len;
@@ -2183,6 +2184,7 @@ static int drm_dp_send_enum_path_resources(struct
drm_dp_mst_topology_mgr *mgr,
DRM_DEBUG_KMS("enum path resources %d: %d %d\n",
txmsg->reply.u.path_resources.port_number,
txmsg->reply.u.path_resources.full_payload_bw_number,
txmsg->reply.u.path_resources.avail_payload_bw_number);
port->available_pbn =
txmsg->reply.u.path_resources.avail_payload_bw_number;
+ port->fec_capable =
txmsg->reply.u.path_resources.fec_capable;
}
}
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 9116b2c95239..f113ae04fa88 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -108,6 +108,8 @@ struct drm_dp_mst_port {
* audio-capable.
*/
bool has_audio;
+
+ bool fec_capable;
};
/**
@@ -312,6 +314,7 @@ struct drm_dp_port_number_req {
struct drm_dp_enum_path_resources_ack_reply {
u8 port_number;
+ bool fec_capable;
u16 full_payload_bw_number;
u16 avail_payload_bw_number;
};
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer
On Mon, Sep 30, 2019 at 8:57 AM Ayan Halder wrote: > > On Mon, Sep 30, 2019 at 09:51:35AM +, Brian Starkey wrote: > > Hi, > > > > On Tue, Sep 17, 2019 at 07:36:45PM +0200, Daniel Vetter wrote: > > > On Tue, Sep 17, 2019 at 6:15 PM Neil Armstrong > > > wrote: > > > > > > > > Hi, > > > > > > > > On 17/09/2019 18:07, Liviu Dudau wrote: > > > > > On Tue, Sep 17, 2019 at 02:53:01PM +0200, Daniel Vetter wrote: > > > > >> On Mon, Sep 09, 2019 at 01:42:53PM +, Ayan Halder wrote: > > > > >>> Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that > > > > >>> the framebuffer > > > > >>> is allocated in a protected system memory. > > > > >>> Essentially, we want to support EGL_EXT_protected_content in our > > > > >>> komeda driver. > > > > >>> > > > > >>> Signed-off-by: Ayan Kumar Halder > > > > >>> > > > > >>> /-- Note to reviewer > > > > >>> Komeda driver is capable of rendering DRM (Digital Rights > > > > >>> Management) protected > > > > >>> content. The DRM content is stored in a framebuffer allocated in > > > > >>> system memory > > > > >>> (which needs some special hardware signals for access). > > > > >>> > > > > >>> Let us ignore how the protected system memory is allocated and for > > > > >>> the scope of > > > > >>> this discussion, we want to figure out the best way possible for > > > > >>> the userspace > > > > >>> to communicate to the drm driver to turn the protected mode on (for > > > > >>> accessing the > > > > >>> framebuffer with the DRM content) or off. > > > > >>> > > > > >>> The possible ways by which the userspace could achieve this is via:- > > > > >>> > > > > >>> 1. Modifiers :- This looks to me the best way by which the > > > > >>> userspace can > > > > >>> communicate to the kernel to turn the protected mode on for the > > > > >>> komeda driver > > > > >>> as it is going to access one of the protected framebuffers. The > > > > >>> only problem is > > > > >>> that the current modifiers describe the tiling/compression format. > > > > >>> However, it > > > > >>> does not hurt to extend the meaning of modifiers to denote other > > > > >>> attributes of > > > > >>> the framebuffer as well. > > > > >>> > > > > >>> The other reason is that on Android, we get an info from Gralloc > > > > >>> (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is > > > > >>> protected. This can > > > > >>> be used to set up the modifier/s (AddFB2) during framebuffer > > > > >>> creation. > > > > >> > > > > >> How does this mesh with other modifiers, like AFBC? That's where I > > > > >> see the > > > > >> issue here. > > > > > > > > > > AFBC modifiers are currently under Arm's namespace, the thought > > > > > behind the DRM > > > > > modifiers would be to have it as a "generic" modifier. > > > > > > But if it's a generic flag, how do you combine that with other > > > modifiers? Like if you have a tiled buffer, but also encrypted? Or > > > afbc compressed, or whatever else. I'd expect for your hw encryption > > > is orthogonal to the buffer/tiling/compression format used? > > > > This bit doesn't overlap with any of the other AFBC modifiers, so as > > you say it'd be orthogonal, and could be set on AFBC buffers (if we > > went that route). > > > > > > > > > >>> 2. Framebuffer flags :- As of today, this can be one of the two > > > > >>> values > > > > >>> ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike > > > > >>> modifiers, the drm > > > > >>> framebuffer flags are generic to the drm subsystem and ideally we > > > > >>> should not > > > > >>> introduce any driver specific constraint/feature. > > > > >>> > > > > >>> 3. Connector property:- I could see the following properties used > > > > >>> for DRM > > > > >>> protected content:- > > > > >>> DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is > > > > >>> used by > > > > >>> userspace to request the kernel protect future content communicated > > > > >>> over > > > > >>> the link". Clearly, we are not concerned with the protection > > > > >>> attributes of the > > > > >>> transmitter. So, we cannot use this property for our case. > > > > >>> > > > > >>> 4. DRM plane property:- Again, we want to communicate that the > > > > >>> framebuffer(which > > > > >>> can be attached to any plane) is protected. So introducing a new > > > > >>> plane property > > > > >>> does not help. > > > > >>> > > > > >>> 5. DRM crtc property:- For the same reason as above, introducing a > > > > >>> new crtc > > > > >>> property does not help. > > > > >> > > > > >> 6. Just track this as part of buffer allocation, i.e. I think it does > > > > >> matter how you allocate these protected buffers. We could add a "is > > > > >> protected buffer" flag at the dma_buf level for this. > > > > I also like this approach. The protected-ness is a property of the > > allocation, so makes sense to store it with the allocation IMO. > > > > > > >> > > > > >> So yeah for this stuff here I think we do want the full userspace > > > > >>
HMM related? memory leakage
In the development version of ubuntu, using a 5.3 kernel, running the dolphin emulator appears to leak memory (there may be other apps that trigger the same issue, but haven't run into them). The "used" memory reported by top grows until the app exits, and does not get freed at that time. This is on a dell xps 2-in-1 with hybrid intel/amd-vega-m graphics, and DRI_PRIME=1. Some more details at downstream bug report: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1844962 Running a kernel bisection to find the offending commit shows this: 899fbde1464639e3d12eaffdad8481a59b367fcb is the first bad commit commit 899fbde1464639e3d12eaffdad8481a59b367fcb Author: Philip Yang Date: Thu Dec 13 15:35:28 2018 -0500 drm/amdgpu: replace get_user_pages with HMM mirror helpers Use HMM helper function hmm_vma_fault() to get physical pages backing userptr and start CPU page table update track of those pages. Then use hmm_vma_range_done() to check if those pages are updated before amdgpu_cs_submit for gfx or before user queues are resumed for kfd. If userptr pages are updated, for gfx, amdgpu_cs_ioctl will restart from scratch, for kfd, restore worker is rescheduled to retry. HMM simplify the CPU page table concurrent update check, so remove guptasklock, mmu_invalidations, last_set_pages fields from amdgpu_ttm_tt struct. HMM does not pin the page (increase page ref count), so remove related operations like release_pages(), put_page(), mark_page_dirty(). Signed-off-by: Philip Yang Reviewed-by: Felix Kuehling Reviewed-by: Christian König Signed-off-by: Alex Deucher :04 04 0c9f0e2e82e5e4d2d3a4c0daea22eb911244b771 fdcdc7c80f5383486962edf4561e205b55bd8c21 M drivers $ git bisect log # bad: [f74c2bb98776e2de508f4d607cd519873065118e] Linux 5.3-rc8 # good: [1c163f4c7b3f621efff9b28a47abb36f7378d783] Linux 5.0 git bisect start 'v5.3-rc8' 'v5.0' # good: [a2d635decbfa9c1e4ae15cb05b68b2559f7f827c] Merge tag 'drm-next-2019-05-09' of git://anongit.freedesktop.org/drm/drm git bisect good a2d635decbfa9c1e4ae15cb05b68b2559f7f827c # good: [a2d635decbfa9c1e4ae15cb05b68b2559f7f827c] Merge tag 'drm-next-2019-05-09' of git://anongit.freedesktop.org/drm/drm git bisect good a2d635decbfa9c1e4ae15cb05b68b2559f7f827c # good: [8f6ccf6159aed1f04c6d179f61f6fb2691261e84] Merge tag 'clone3-v5.3' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux git bisect good 8f6ccf6159aed1f04c6d179f61f6fb2691261e84 # good: [8f6ccf6159aed1f04c6d179f61f6fb2691261e84] Merge tag 'clone3-v5.3' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux git bisect good 8f6ccf6159aed1f04c6d179f61f6fb2691261e84 # bad: [be8454afc50f43016ca8b6130d9673bdd0bd56ec] Merge tag 'drm-next-2019-07-16' of git://anongit.freedesktop.org/drm/drm git bisect bad be8454afc50f43016ca8b6130d9673bdd0bd56ec # bad: [be8454afc50f43016ca8b6130d9673bdd0bd56ec] Merge tag 'drm-next-2019-07-16' of git://anongit.freedesktop.org/drm/drm git bisect bad be8454afc50f43016ca8b6130d9673bdd0bd56ec # good: [d72619706abc4aa7e540ea882dae883cee7cc3b3] Merge tag 'tty-5.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty git bisect good d72619706abc4aa7e540ea882dae883cee7cc3b3 # bad: [83145f110eb2ada9d54fcbcf416c02de126381c1] drm/amdgpu: don't invalidate caches in RELEASE_MEM, only do the writeback git bisect bad 83145f110eb2ada9d54fcbcf416c02de126381c1 # bad: [b239c01727459ba08c44b79e6225d3c58723f282] drm/amdgpu: add mcbp driver parameter git bisect bad b239c01727459ba08c44b79e6225d3c58723f282 # good: [e1dc68a4b149d47536cd001d0d0abadbb62d37bd] drm: atmel-hlcdc: avoid initializing cfg with zero git bisect good e1dc68a4b149d47536cd001d0d0abadbb62d37bd # bad: [c53e4db71276bf257b09010935a04bdafddd458e] drm/amdgpu: cancel late_init_work before gpu reset git bisect bad c53e4db71276bf257b09010935a04bdafddd458e # good: [2da4605dce38b84cd2e5b86686f43adae1b2cacb] drm/amd/display: Use DCN functions instead of DCE git bisect good 2da4605dce38b84cd2e5b86686f43adae1b2cacb # bad: [1c1e53f7f2ce191e6787d3d0648fe8ce7088ceaa] drm/amd/doc: Add XGMI sysfs documentation git bisect bad 1c1e53f7f2ce191e6787d3d0648fe8ce7088ceaa # good: [89cd9d23e9a74d94f0db5bbbaf2ef1f6ede36ae5] drm/amdkfd: avoid HMM change cause circular lock git bisect good 89cd9d23e9a74d94f0db5bbbaf2ef1f6ede36ae5 # bad: [0803e7a9e850f9d6397c594d6c6deac9b2b6d696] drm/amdkfd: Allocate hiq and sdma mqd from mqd trunk git bisect bad 0803e7a9e850f9d6397c594d6c6deac9b2b6d696 # bad: [972fcdb52fe865a2f639e3200b97e648f34a0f41] drm/amdkfd: Introduce asic-specific mqd_manager_init function git bisect bad 972fcdb52fe865a2f639e3200b97e648f34a0f41 # bad: [6c55d6e90e68a4789cbd72a0287026d4dfb4a9f9] drm/amdkfd: support concurrent userptr update for HMM git bisect bad 6c55d6e90e68a4789cbd72a0287026d4dfb4a9f9 # bad: [ad595b8634f36f04bf69bef4eff854091d94f8b3] drm/amdgpu: fix HMM config dependency issue git bisect bad ad595b8634f36f04bf69bef4eff854091d94f8b3 # bad:
Re: [PATCH RFC v4 11/16] drm, cgroup: Add per cgroup bw measure and control
Hi.
On Thu, Aug 29, 2019 at 02:05:28AM -0400, Kenny Ho wrote:
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1256,6 +1257,12 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
> [...]
> + move_delay /= 2000; /* check every half period in ms*/
> [...]
> diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
> [...]
> @@ -382,6 +548,25 @@ static ssize_t drmcg_limit_write(struct kernfs_open_file
> *of, char *buf,
> [...]
> + if (rc || val < 2000) {
This just caught my eye and it may be simply caused by RFC-ness of the
series but I'd suggest turning this into a constant with descriptive
name.
My 2 cents,
Michal
signature.asc
Description: Digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC v4 02/16] cgroup: Introduce cgroup for drm subsystem
Hi.
On Thu, Aug 29, 2019 at 02:05:19AM -0400, Kenny Ho wrote:
> +struct cgroup_subsys drm_cgrp_subsys = {
> + .css_alloc = drmcg_css_alloc,
> + .css_free = drmcg_css_free,
> + .early_init = false,
> + .legacy_cftypes = files,
Do you really want to expose the DRM controller on v1 hierarchies (where
threads of one process can be in different cgroups, or children cgroups
compete with their parents)?
> + .dfl_cftypes= files,
> +};
Just asking,
Michal
signature.asc
Description: Digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 0/9] drm/print: add and use drm_debug_enabled()
On Tue, 01 Oct 2019, Eric Engestrom wrote:
> On Tuesday, 2019-10-01 14:03:55 +0300, Jani Nikula wrote:
>> On Thu, 26 Sep 2019, Eric Engestrom wrote:
>> > On Tuesday, 2019-09-24 15:58:56 +0300, Jani Nikula wrote:
>> >> Hi all, v2 of [1], a little refactoring around drm_debug access to
>> >> abstract it better. There shouldn't be any functional changes.
>> >>
>> >> I'd appreciate acks for merging the lot via drm-misc. If there are any
>> >> objections to that, we'll need to postpone the last patch until
>> >> everything has been merged and converted in drm-next.
>> >>
>> >> BR,
>> >> Jani.
>> >>
>> >> Cc: Eric Engestrom
>> >> Cc: Alex Deucher
>> >> Cc: Christian König
>> >> Cc: David (ChunMing) Zhou
>> >> Cc: amd-gfx@lists.freedesktop.org
>> >> Cc: Ben Skeggs
>> >> Cc: nouv...@lists.freedesktop.org
>> >> Cc: Rob Clark
>> >> Cc: Sean Paul
>> >> Cc: linux-arm-...@vger.kernel.org
>> >> Cc: freedr...@lists.freedesktop.org
>> >> Cc: Francisco Jerez
>> >> Cc: Lucas Stach
>> >> Cc: Russell King
>> >> Cc: Christian Gmeiner
>> >> Cc: etna...@lists.freedesktop.org
>> >>
>> >>
>> >> [1] cover.1568375189.git.jani.nikula@intel.com">http://mid.mail-archive.com/cover.1568375189.git.jani.nikula@intel.com
>> >>
>> >> Jani Nikula (9):
>> >> drm/print: move drm_debug variable to drm_print.[ch]
>> >> drm/print: add drm_debug_enabled()
>> >> drm/i915: use drm_debug_enabled() to check for debug categories
>> >> drm/print: rename drm_debug to __drm_debug to discourage use
>> >
>> > The above four patches are:
>> > Reviewed-by: Eric Engestrom
>> >
>> > Did you check to make sure the `unlikely()` is propagated correctly
>> > outside the `drm_debug_enabled()` call?
>>
>> I did now.
>>
>> Having drm_debug_enabled() as a macro vs. as an inline function does not
>> seem to make a difference, so I think the inline is clearly preferrable.
>
> Agreed :)
>
>>
>> However, for example
>>
>> unlikely(foo && drm_debug & DRM_UT_DP)
>>
>> does produce code different from
>>
>> (foo && drm_debug_enabled(DRM_UT_DP))
>>
>> indicating that the unlikely() within drm_debug_enabled() does not
>> propagate to the whole condition. It's possible to retain the same
>> assembly output with
>>
>> (unlikely(foo) && drm_debug_enabled(DRM_UT_DP))
>>
>> but it's unclear to me whether this is really worth it, either
>> readability or performance wise.
>>
>> Thoughts?
>
> That kind of code only happens 2 times, both in
> drivers/gpu/drm/drm_dp_mst_topology.c (in patch 2/9), right?
>
> I think your suggestion is the right thing to do here:
>
> - if (unlikely(ret && drm_debug & DRM_UT_DP)) {
> + if (unlikely(ret) && drm_debug_enabled(DRM_UT_DP)) {
>
> It doesn't really cost much in readability (especially compared to what
> it was before), and whether it's important performance wise I couldn't
> tell, but I think it's best to keep the code optimised as it was before
> unless there's a reason to drop it.
>
> Lyude might know more since she wrote 2f015ec6eab69301fdcf5, if you want
> to ping her?
Just ended up sending the updated version with what you suggest and I
agree with; pedantically the change should be a separate patch anyway.
Thanks for your inputs.
BR,
Jani.
>
>>
>> BR,
>> Jani.
>>
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open Source Graphics Center
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC v4 07/16] drm, cgroup: Add total GEM buffer allocation limit
Hello.
On Thu, Aug 29, 2019 at 02:05:24AM -0400, Kenny Ho wrote:
> drm.buffer.default
> A read-only flat-keyed file which exists on the root cgroup.
> Each entry is keyed by the drm device's major:minor.
>
> Default limits on the total GEM buffer allocation in bytes.
What is the purpose of this attribute (and alikes for other resources)?
I can't see it being set differently but S64_MAX in
drmcg_device_early_init.
> +static ssize_t drmcg_limit_write(struct kernfs_open_file *of, char *buf,
> [...]
> + switch (type) {
> + case DRMCG_TYPE_BO_TOTAL:
> + p_max = parent == NULL ? S64_MAX :
> + parent->dev_resources[minor]->
> + bo_limits_total_allocated;
> +
> + rc = drmcg_process_limit_s64_val(sattr, true,
> + props->bo_limits_total_allocated_default,
> + p_max,
> + );
IIUC, this allows initiating the particular limit value based either on
parent or the default per-device value. This is alas rather an
antipattern. The most stringent limit on the path from a cgroup to the
root should be applied at the charging time. However, the child should
not inherit the verbatim value from the parent (may race with parent and
it won't be updated upon parent change).
You already do the appropriate hierarchical check in
drmcg_try_chb_bo_alloc, so the parent propagation could be simply
dropped if I'm not mistaken.
Also, I can't find how the read of
parent->dev_resources[minor]->bo_limits_total_allocated and its
concurrent update are synchronized (i.e. someone writing
buffer.total.max for parent and child in parallel). (It may just my
oversight.)
I'm posting this to the buffer knobs patch but similar applies to lgpu
resource controls as well.
HTH,
Michal
signature.asc
Description: Digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 15/15] drm/amd/display: Trigger modesets on MST DSC connectors
On 19.09.2019 20:08, Lyude Paul wrote:
> This still needs to be moved into an atomic helper so it can be reused by
> other drivers
It would be a future to move the implementation to a contained atomic
helper.
> ...
> ...
>
> however, I've had this patch series on my mind for a while and something
> occurred to me that would be a lot easier. Why exactly are we not just
> enabling DSC wherever it's supported, regardless of whether or not it's
> actually needed to fit into bandwidth constraints? The reason I mention this
> is while only enabling DSC when needed makes perfect sense for SST, since I'd
> assume non-DSC consumes less power, it doesn't quite make sense for MST.
>
> See: currently with MST (at least this is the case with i915, but I plan on
> making it the case with nouveau as well), we always default to link training
> at the maximum link rate/lane count. i915 does the opposite with SST for
> optimization, but we maximize for MST because maximizing the amount of
> bandwidth we have to work with means that we can enable new displays and
> disable displays without having to potentially update the rest of the topology
> with new PBN/VCPI allocations (unless the bandwidth restrictions get lowered
> later, of course).
>
> An abstract example: Let's say a user has a hub setup with two displays,
> without DSC enabled. The user plugs in a third display, but this display won't
> fit into the available bandwidth without enabling DSC. But assuming enabling
> DSC causes us to need to recalculate the bandwidth for the other two displays,
> we end up having to do a modeset on all three displays.
>
> Alternatively: the user has two displays again. This time though we started
> off by using DSC from the start, so adding a new display can be done without
> performing a modeset on the other two displays.
>
> Taking this into consideration, wouldn't we be able to just get rid of this
> whole function by enabling DSC by default instead of as-needed? And get a
> nicer result?
The idea of enabling DSC from the beginning does sound appealing, but
DSC is a lossy compression and we think it is better to enable it only
when we need it, not to degrade picture quality. The greedy algorithm
developed by David Francis in his patch "MST DSC compute fair share"
tries to minimize compression and enable maximum number of streams
without it, so they won't lose the picture quality.
That being said, I agree, the implementation of DSC code would be much
simpler if we would just enable DSC from the beginning, but again we do
not want to compromise the picture quality.
>
> On Wed, 2019-09-18 at 16:26 -0400, mikita.lip...@amd.com wrote:
>> From: David Francis
>>
>> Whenever a connector on an MST network is attached, detached, or
>> undergoes a modeset, the DSC configs for each stream on that
>> topology will be recalculated. This can change their required
>> bandwidth, requiring a full reprogramming, as though a modeset
>> was performed, even if that stream did not change timing.
>>
>> Therefore, whenever a crtc has drm_atomic_crtc_needs_modeset,
>> for each crtc that shares a MST topology with that stream and
>> supports DSC, add that crtc (and all affected connectors and
>> planes) to the atomic state and set mode_changed on its state
>>
>> v2: Do this check only on Navi and before adding connectors
>> and planes on modesetting crtcs
>>
>> Cc: Leo Li
>> Cc: Nicholas Kazlauskas
>> Cc: Lyude Paul
>> Signed-off-by: David Francis
>> ---
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 +++
>> 1 file changed, 79 insertions(+)
>>
>> 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 ba017e6bf0b4..f65326e85b86 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -6704,6 +6704,74 @@ static int do_aquire_global_lock(struct drm_device
>> *dev,
>> return ret < 0 ? ret : 0;
>> }
>>
>> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
>> +/*
>> + * TODO: This logic should at some point be moved into DRM
>> + */
>> +static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state,
>> struct drm_crtc *crtc)
>> +{
>> +struct drm_connector *connector;
>> +struct drm_connector_state *conn_state;
>> +struct drm_connector_list_iter conn_iter;
>> +struct drm_crtc_state *new_crtc_state;
>> +struct amdgpu_dm_connector *aconnector = NULL, *aconnector_to_add;
>> +int i, j;
>> +struct drm_crtc *crtcs_affected[AMDGPU_MAX_CRTCS] = { 0 };
>> +
>> +for_each_new_connector_in_state(state, connector, conn_state, i) {
>> +if (conn_state->crtc != crtc)
>> +continue;
>> +
>> +aconnector = to_amdgpu_dm_connector(connector);
>> +if (!aconnector->port)
>> +aconnector = NULL;
>> +else
>> +break;
>> +}
>> +
>> +if (!aconnector)
Re: [PATCH v2 0/9] drm/print: add and use drm_debug_enabled()
On Thu, 26 Sep 2019, Eric Engestrom wrote: > On Tuesday, 2019-09-24 15:58:56 +0300, Jani Nikula wrote: >> Hi all, v2 of [1], a little refactoring around drm_debug access to >> abstract it better. There shouldn't be any functional changes. >> >> I'd appreciate acks for merging the lot via drm-misc. If there are any >> objections to that, we'll need to postpone the last patch until >> everything has been merged and converted in drm-next. >> >> BR, >> Jani. >> >> Cc: Eric Engestrom >> Cc: Alex Deucher >> Cc: Christian König >> Cc: David (ChunMing) Zhou >> Cc: amd-gfx@lists.freedesktop.org >> Cc: Ben Skeggs >> Cc: nouv...@lists.freedesktop.org >> Cc: Rob Clark >> Cc: Sean Paul >> Cc: linux-arm-...@vger.kernel.org >> Cc: freedr...@lists.freedesktop.org >> Cc: Francisco Jerez >> Cc: Lucas Stach >> Cc: Russell King >> Cc: Christian Gmeiner >> Cc: etna...@lists.freedesktop.org >> >> >> [1] cover.1568375189.git.jani.nikula@intel.com">http://mid.mail-archive.com/cover.1568375189.git.jani.nikula@intel.com >> >> Jani Nikula (9): >> drm/print: move drm_debug variable to drm_print.[ch] >> drm/print: add drm_debug_enabled() >> drm/i915: use drm_debug_enabled() to check for debug categories >> drm/print: rename drm_debug to __drm_debug to discourage use > > The above four patches are: > Reviewed-by: Eric Engestrom > > Did you check to make sure the `unlikely()` is propagated correctly > outside the `drm_debug_enabled()` call? I did now. Having drm_debug_enabled() as a macro vs. as an inline function does not seem to make a difference, so I think the inline is clearly preferrable. However, for example unlikely(foo && drm_debug & DRM_UT_DP) does produce code different from (foo && drm_debug_enabled(DRM_UT_DP)) indicating that the unlikely() within drm_debug_enabled() does not propagate to the whole condition. It's possible to retain the same assembly output with (unlikely(foo) && drm_debug_enabled(DRM_UT_DP)) but it's unclear to me whether this is really worth it, either readability or performance wise. Thoughts? BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3] drm/amdgpu: fix multiple memory leaks in acp_hw_init
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > @@ -184,12 +184,12 @@ static struct device *get_mfd_cell_dev(const char > *device_name, int r) … > + struct i2s_platform_data *i2s_pdata = NULL; … I propose to reconsider this update suggestion. > @@ -231,20 +231,21 @@ static int acp_hw_init(void *handle) > adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell), > GFP_KERNEL); > > - if (adev->acp.acp_cell == NULL) > - return -ENOMEM; … I suggest to keep this source code place unchanged (at the moment). https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=54ecb8f7028c5eb3d740bb82b0f1d90f2df63c5c#n456 > @@ -393,6 +399,13 @@ static int acp_hw_init(void *handle) > val &= ~ACP_SOFT_RESET__SoftResetAud_MASK; > cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val); > return 0; > + > +failure: > + kfree(i2s_pdata); > + kfree(adev->acp.acp_res); > + kfree(adev->acp.acp_cell); > + kfree(adev->acp.acp_genpd); > + return ret; > } > > /** I would prefer separate jump targets for efficient exception handling. Please choose more appropriate labels for this function implementation. > --- I suggest to replace this second delimiter by a blank line. Regards, Markus ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Quick question for the mobile Raven Ridge auto-rotate function
On Tue, Oct 1, 2019 at 3:41 AM Sjoerd Bergmans wrote: > > Hi, > > Quick question regarding an earlier announcement; > https://lists.freedesktop.org/archives/amd-gfx/2019-May/034431.html > > I feel a lot of Linux users are still waiting for the specified driver to > surface. Since you gave the original statement, might it be you are able to > offer any new insights or update? https://lists.freedesktop.org/archives/amd-gfx/2019-September/040553.html Alex > > Thnx! > ___ > 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] drm/amd/amdgpu/sriov ip block setting of Arcturus
Actually, we can probably drop that hunk because adev->mode_info.num_crtc
should be 0 for arcturus right?
Alex
From: amd-gfx on behalf of Deucher,
Alexander
Sent: Tuesday, October 1, 2019 9:28 AM
To: Zhang, Jack (Jian) ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu/sriov ip block setting of Arcturus
From: amd-gfx on behalf of Jack Zhang
Sent: Monday, September 30, 2019 1:00 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Jack (Jian)
Subject: [PATCH] drm/amd/amdgpu/sriov ip block setting of Arcturus
Add ip block setting for Arcturus SRIOV
1.PSP need to be initialized before IH.
2.SMU doesn't need to be initialized at kmd driver.
3.Arcturus doesn't support DCE hardware,it needs to skip
register access to DCE.
Signed-off-by: Jack Zhang
---
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 10 ++
drivers/gpu/drm/amd/amdgpu/soc15.c| 19 +++
2 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 95a9a5f5..44023bd 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1330,11 +1330,13 @@ static int gmc_v9_0_hw_init(void *handle)
gmc_v9_0_init_golden_registers(adev);
if (adev->mode_info.num_crtc) {
- /* Lockout access through VGA aperture*/
- WREG32_FIELD15(DCE, 0, VGA_HDP_CONTROL, VGA_MEMORY_DISABLE, 1);
+ if (adev->asic_type != CHIP_ARCTURUS) {
+ /* Lockout access through VGA aperture*/
+ WREG32_FIELD15(DCE, 0, VGA_HDP_CONTROL,
VGA_MEMORY_DISABLE, 1);
- /* disable VGA render */
- WREG32_FIELD15(DCE, 0, VGA_RENDER_CONTROL, VGA_VSTATUS_CNTL, 0);
+ /* disable VGA render */
+ WREG32_FIELD15(DCE, 0, VGA_RENDER_CONTROL,
VGA_VSTATUS_CNTL, 0);
+ }
}
This is a general bug fix and should be split out into a separate patch.
Alex
r = gmc_v9_0_gart_enable(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index dbd790e..ac181e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -754,14 +754,25 @@ int soc15_set_ip_blocks(struct amdgpu_device *adev)
case CHIP_ARCTURUS:
amdgpu_device_ip_block_add(adev, _common_ip_block);
amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
- amdgpu_device_ip_block_add(adev, _ih_ip_block);
- if (likely(adev->firmware.load_type == AMDGPU_FW_LOAD_PSP))
- amdgpu_device_ip_block_add(adev, _v11_0_ip_block);
+
+ /* For MI100 SR-IOV, PSP need to be initialized before IH */
+ if (amdgpu_sriov_vf(adev)) {
+ if (likely(adev->firmware.load_type ==
AMDGPU_FW_LOAD_PSP))
+ amdgpu_device_ip_block_add(adev,
_v11_0_ip_block);
+ amdgpu_device_ip_block_add(adev, _ih_ip_block);
+ } else {
+ amdgpu_device_ip_block_add(adev, _ih_ip_block);
+ if (likely(adev->firmware.load_type ==
AMDGPU_FW_LOAD_PSP))
+ amdgpu_device_ip_block_add(adev,
_v11_0_ip_block);
+ }
+
if (adev->enable_virtual_display || amdgpu_sriov_vf(adev))
amdgpu_device_ip_block_add(adev,
_virtual_ip_block);
amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
amdgpu_device_ip_block_add(adev, _v4_0_ip_block);
- amdgpu_device_ip_block_add(adev, _v11_0_ip_block);
+ if (!amdgpu_sriov_vf(adev))
+ amdgpu_device_ip_block_add(adev, _v11_0_ip_block);
+
if (unlikely(adev->firmware.load_type ==
AMDGPU_FW_LOAD_DIRECT))
amdgpu_device_ip_block_add(adev, _v2_5_ip_block);
break;
--
2.7.4
___
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] drm/amd/amdgpu/sriov ip block setting of Arcturus
From: amd-gfx on behalf of Jack Zhang
Sent: Monday, September 30, 2019 1:00 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Jack (Jian)
Subject: [PATCH] drm/amd/amdgpu/sriov ip block setting of Arcturus
Add ip block setting for Arcturus SRIOV
1.PSP need to be initialized before IH.
2.SMU doesn't need to be initialized at kmd driver.
3.Arcturus doesn't support DCE hardware,it needs to skip
register access to DCE.
Signed-off-by: Jack Zhang
---
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 10 ++
drivers/gpu/drm/amd/amdgpu/soc15.c| 19 +++
2 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 95a9a5f5..44023bd 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1330,11 +1330,13 @@ static int gmc_v9_0_hw_init(void *handle)
gmc_v9_0_init_golden_registers(adev);
if (adev->mode_info.num_crtc) {
- /* Lockout access through VGA aperture*/
- WREG32_FIELD15(DCE, 0, VGA_HDP_CONTROL, VGA_MEMORY_DISABLE, 1);
+ if (adev->asic_type != CHIP_ARCTURUS) {
+ /* Lockout access through VGA aperture*/
+ WREG32_FIELD15(DCE, 0, VGA_HDP_CONTROL,
VGA_MEMORY_DISABLE, 1);
- /* disable VGA render */
- WREG32_FIELD15(DCE, 0, VGA_RENDER_CONTROL, VGA_VSTATUS_CNTL, 0);
+ /* disable VGA render */
+ WREG32_FIELD15(DCE, 0, VGA_RENDER_CONTROL,
VGA_VSTATUS_CNTL, 0);
+ }
}
This is a general bug fix and should be split out into a separate patch.
Alex
r = gmc_v9_0_gart_enable(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index dbd790e..ac181e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -754,14 +754,25 @@ int soc15_set_ip_blocks(struct amdgpu_device *adev)
case CHIP_ARCTURUS:
amdgpu_device_ip_block_add(adev, _common_ip_block);
amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
- amdgpu_device_ip_block_add(adev, _ih_ip_block);
- if (likely(adev->firmware.load_type == AMDGPU_FW_LOAD_PSP))
- amdgpu_device_ip_block_add(adev, _v11_0_ip_block);
+
+ /* For MI100 SR-IOV, PSP need to be initialized before IH */
+ if (amdgpu_sriov_vf(adev)) {
+ if (likely(adev->firmware.load_type ==
AMDGPU_FW_LOAD_PSP))
+ amdgpu_device_ip_block_add(adev,
_v11_0_ip_block);
+ amdgpu_device_ip_block_add(adev, _ih_ip_block);
+ } else {
+ amdgpu_device_ip_block_add(adev, _ih_ip_block);
+ if (likely(adev->firmware.load_type ==
AMDGPU_FW_LOAD_PSP))
+ amdgpu_device_ip_block_add(adev,
_v11_0_ip_block);
+ }
+
if (adev->enable_virtual_display || amdgpu_sriov_vf(adev))
amdgpu_device_ip_block_add(adev,
_virtual_ip_block);
amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
amdgpu_device_ip_block_add(adev, _v4_0_ip_block);
- amdgpu_device_ip_block_add(adev, _v11_0_ip_block);
+ if (!amdgpu_sriov_vf(adev))
+ amdgpu_device_ip_block_add(adev, _v11_0_ip_block);
+
if (unlikely(adev->firmware.load_type ==
AMDGPU_FW_LOAD_DIRECT))
amdgpu_device_ip_block_add(adev, _v2_5_ip_block);
break;
--
2.7.4
___
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] drm/amdgpu: Do not implement power-on for SDMA after do mode2 reset on Renoir
On Sun, Sep 29, 2019 at 2:19 AM chen gong wrote: > > Find that ring sdma0 test failed if turn on SDMA powergating after do > mode2 reset. > > Perhaps the mode2 reset does not reset the SDMA PG state, SDMA is > already powered up so there is no need to ask the SMU to power it up > again. So I skip this function for a moment. > > Signed-off-by: chen gong > --- > drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > index 96581b5..e0eb2450 100644 > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c > @@ -1792,7 +1792,7 @@ static int sdma_v4_0_hw_init(void *handle) > > if ((adev->asic_type == CHIP_RAVEN && adev->powerplay.pp_funcs && > adev->powerplay.pp_funcs->set_powergating_by_smu) || > - adev->asic_type == CHIP_RENOIR) > + (adev->asic_type == CHIP_RENOIR && adev->in_gpu_reset > != 1)) How about !adev->in_gpu_reset rather than explicitly checking for 1? With that fixed: Reviewed-by: Alex Deucher Alex > amdgpu_dpm_set_powergating_by_smu(adev, > AMD_IP_BLOCK_TYPE_SDMA, false); > > if (!amdgpu_sriov_vf(adev)) > -- > 2.7.4 > > ___ > 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 v2 0/9] drm/print: add and use drm_debug_enabled()
On Tuesday, 2019-10-01 14:03:55 +0300, Jani Nikula wrote:
> On Thu, 26 Sep 2019, Eric Engestrom wrote:
> > On Tuesday, 2019-09-24 15:58:56 +0300, Jani Nikula wrote:
> >> Hi all, v2 of [1], a little refactoring around drm_debug access to
> >> abstract it better. There shouldn't be any functional changes.
> >>
> >> I'd appreciate acks for merging the lot via drm-misc. If there are any
> >> objections to that, we'll need to postpone the last patch until
> >> everything has been merged and converted in drm-next.
> >>
> >> BR,
> >> Jani.
> >>
> >> Cc: Eric Engestrom
> >> Cc: Alex Deucher
> >> Cc: Christian König
> >> Cc: David (ChunMing) Zhou
> >> Cc: amd-gfx@lists.freedesktop.org
> >> Cc: Ben Skeggs
> >> Cc: nouv...@lists.freedesktop.org
> >> Cc: Rob Clark
> >> Cc: Sean Paul
> >> Cc: linux-arm-...@vger.kernel.org
> >> Cc: freedr...@lists.freedesktop.org
> >> Cc: Francisco Jerez
> >> Cc: Lucas Stach
> >> Cc: Russell King
> >> Cc: Christian Gmeiner
> >> Cc: etna...@lists.freedesktop.org
> >>
> >>
> >> [1] cover.1568375189.git.jani.nikula@intel.com">http://mid.mail-archive.com/cover.1568375189.git.jani.nikula@intel.com
> >>
> >> Jani Nikula (9):
> >> drm/print: move drm_debug variable to drm_print.[ch]
> >> drm/print: add drm_debug_enabled()
> >> drm/i915: use drm_debug_enabled() to check for debug categories
> >> drm/print: rename drm_debug to __drm_debug to discourage use
> >
> > The above four patches are:
> > Reviewed-by: Eric Engestrom
> >
> > Did you check to make sure the `unlikely()` is propagated correctly
> > outside the `drm_debug_enabled()` call?
>
> I did now.
>
> Having drm_debug_enabled() as a macro vs. as an inline function does not
> seem to make a difference, so I think the inline is clearly preferrable.
Agreed :)
>
> However, for example
>
> unlikely(foo && drm_debug & DRM_UT_DP)
>
> does produce code different from
>
> (foo && drm_debug_enabled(DRM_UT_DP))
>
> indicating that the unlikely() within drm_debug_enabled() does not
> propagate to the whole condition. It's possible to retain the same
> assembly output with
>
> (unlikely(foo) && drm_debug_enabled(DRM_UT_DP))
>
> but it's unclear to me whether this is really worth it, either
> readability or performance wise.
>
> Thoughts?
That kind of code only happens 2 times, both in
drivers/gpu/drm/drm_dp_mst_topology.c (in patch 2/9), right?
I think your suggestion is the right thing to do here:
- if (unlikely(ret && drm_debug & DRM_UT_DP)) {
+ if (unlikely(ret) && drm_debug_enabled(DRM_UT_DP)) {
It doesn't really cost much in readability (especially compared to what
it was before), and whether it's important performance wise I couldn't
tell, but I think it's best to keep the code optimised as it was before
unless there's a reason to drop it.
Lyude might know more since she wrote 2f015ec6eab69301fdcf5, if you want
to ping her?
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3] drm/amdgpu: fix multiple memory leaks in acp_hw_init
Am 30.09.19 um 23:26 schrieb Navid Emamdoost:
> In acp_hw_init there are some allocations that needs to be released in
> case of failure:
>
> 1- adev->acp.acp_genpd should be released if any allocation attemp for
> adev->acp.acp_cell, adev->acp.acp_res or i2s_pdata fails.
> 2- all of those allocations should be released if
> mfd_add_hotplug_devices or pm_genpd_add_device fail.
> 3- Release is needed in case of time out values expire.
>
> Signed-off-by: Navid Emamdoost
> ---
> Changes in v2:
> -- moved the releases under goto
>
> Changes in v3:
> -- fixed multiple goto issue
> -- added goto for 3 other failure cases: one when
> mfd_add_hotplug_devices fails, and two when time out values expires.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 41 -
> 1 file changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> index eba42c752bca..7809745ec0f1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> @@ -184,12 +184,12 @@ static struct device *get_mfd_cell_dev(const char
> *device_name, int r)
>*/
> static int acp_hw_init(void *handle)
> {
> - int r, i;
> + int r, i, ret;
Please don't add another "ret" variable, instead always use "r" here.
Apart from that looks good to me,
Christian.
> uint64_t acp_base;
> u32 val = 0;
> u32 count = 0;
> struct device *dev;
> - struct i2s_platform_data *i2s_pdata;
> + struct i2s_platform_data *i2s_pdata = NULL;
>
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> @@ -231,20 +231,21 @@ static int acp_hw_init(void *handle)
> adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
> GFP_KERNEL);
>
> - if (adev->acp.acp_cell == NULL)
> - return -ENOMEM;
> + if (adev->acp.acp_cell == NULL) {
> + ret = -ENOMEM;
> + goto failure;
> + }
>
> adev->acp.acp_res = kcalloc(5, sizeof(struct resource), GFP_KERNEL);
> if (adev->acp.acp_res == NULL) {
> - kfree(adev->acp.acp_cell);
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto failure;
> }
>
> i2s_pdata = kcalloc(3, sizeof(struct i2s_platform_data), GFP_KERNEL);
> if (i2s_pdata == NULL) {
> - kfree(adev->acp.acp_res);
> - kfree(adev->acp.acp_cell);
> - return -ENOMEM;
> + ret = -ENOMEM;
> + goto failure;
> }
>
> switch (adev->asic_type) {
> @@ -340,15 +341,18 @@ static int acp_hw_init(void *handle)
>
> r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell,
> ACP_DEVS);
> - if (r)
> - return r;
> + if (r) {
> + ret = r;
> + goto failure;
> + }
>
> for (i = 0; i < ACP_DEVS ; i++) {
> dev = get_mfd_cell_dev(adev->acp.acp_cell[i].name, i);
> r = pm_genpd_add_device(>acp.acp_genpd->gpd, dev);
> if (r) {
> dev_err(dev, "Failed to add dev to genpd\n");
> - return r;
> + ret = r;
> + goto failure;
> }
> }
>
> @@ -367,7 +371,8 @@ static int acp_hw_init(void *handle)
> break;
> if (--count == 0) {
> dev_err(>pdev->dev, "Failed to reset ACP\n");
> - return -ETIMEDOUT;
> + ret = -ETIMEDOUT;
> + goto failure;
> }
> udelay(100);
> }
> @@ -384,7 +389,8 @@ static int acp_hw_init(void *handle)
> break;
> if (--count == 0) {
> dev_err(>pdev->dev, "Failed to reset ACP\n");
> - return -ETIMEDOUT;
> + ret = -ETIMEDOUT;
> + goto failure;
> }
> udelay(100);
> }
> @@ -393,6 +399,13 @@ static int acp_hw_init(void *handle)
> val &= ~ACP_SOFT_RESET__SoftResetAud_MASK;
> cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val);
> return 0;
> +
> +failure:
> + kfree(i2s_pdata);
> + kfree(adev->acp.acp_res);
> + kfree(adev->acp.acp_cell);
> + kfree(adev->acp.acp_genpd);
> + return ret;
> }
>
> /**
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Quick question for the mobile Raven Ridge auto-rotate function
Hi, Quick question regarding an earlier announcement; https://lists.freedesktop.org/archives/amd-gfx/2019-May/034431.html I feel a lot of Linux users are still waiting for the specified driver to surface. Since you gave the original statement, might it be you are able to offer any new insights or update? Thnx! ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
