[PATCH AUTOSEL 4.19 009/167] drm/amdgpu/gfx9: Update gfx9 golden settings.

[PATCH AUTOSEL 4.19 009/167] drm/amdgpu/gfx9: Update gfx9 golden settings.

[Sasha Levin]

From: Feifei Xu <Feifei.Xu@amd.com>

[ Upstream commit 54d682d9a5b357eb711994fa94ef1bc44d7ce9d9 ]

Update the goldensettings for vega20.

Signed-off-by: Feifei Xu <Feifei.Xu@amd.com>
Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 46568497ef181..f040ec10eecf6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -82,7 +82,7 @@ MODULE_FIRMWARE("amdgpu/raven_rlc.bin");
 
 static const struct soc15_reg_golden golden_settings_gc_9_0[] =
 {
-	SOC15_REG_GOLDEN_VALUE(GC, 0, mmDB_DEBUG2, 0xf00fffff, 0x00000420),
+	SOC15_REG_GOLDEN_VALUE(GC, 0, mmDB_DEBUG2, 0xf00fffff, 0x00000400),
 	SOC15_REG_GOLDEN_VALUE(GC, 0, mmGB_GPU_ID, 0x0000000f, 0x00000000),
 	SOC15_REG_GOLDEN_VALUE(GC, 0, mmPA_SC_BINNER_EVENT_CNTL_3, 0x00000003, 0x82400024),
 	SOC15_REG_GOLDEN_VALUE(GC, 0, mmPA_SC_ENHANCE, 0x3fffffff, 0x00000001),
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 010/167] drm/amdgpu: Update gc_9_0 golden settings.

[Sasha Levin]

From: Feifei Xu <Feifei.Xu@amd.com>

[ Upstream commit c55045adf7210d246a016c961916f078ed31a951 ]

Add mmDB_DEBUG3 settings.

Signed-off-by: Feifei Xu <Feifei.Xu@amd.com>
Reviewed-by: Evan Quan <evan.quan@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index f040ec10eecf6..7824116498169 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -83,6 +83,7 @@ MODULE_FIRMWARE("amdgpu/raven_rlc.bin");
 static const struct soc15_reg_golden golden_settings_gc_9_0[] =
 {
 	SOC15_REG_GOLDEN_VALUE(GC, 0, mmDB_DEBUG2, 0xf00fffff, 0x00000400),
+	SOC15_REG_GOLDEN_VALUE(GC, 0, mmDB_DEBUG3, 0x80000000, 0x80000000),
 	SOC15_REG_GOLDEN_VALUE(GC, 0, mmGB_GPU_ID, 0x0000000f, 0x00000000),
 	SOC15_REG_GOLDEN_VALUE(GC, 0, mmPA_SC_BINNER_EVENT_CNTL_3, 0x00000003, 0x82400024),
 	SOC15_REG_GOLDEN_VALUE(GC, 0, mmPA_SC_ENHANCE, 0x3fffffff, 0x00000001),
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 022/167] powerplay: Respect units on max dcfclk watermark

[Sasha Levin]

From: David Francis <David.Francis@amd.com>

[ Upstream commit f191415b24a3ad3fa22088af7cd7fc328a2f469f ]

In a refactor, the watermark clock inputs to
powerplay from DC were changed from units of 10kHz to
kHz clocks.

One division by 100 was not converted into a division
by 1000.

Signed-off-by: David Francis <David.Francis@amd.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c
index 2aab1b4759459..a321c465b7dce 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c
@@ -674,7 +674,7 @@ int smu_set_watermarks_for_clocks_ranges(void *wt_table,
 		table->WatermarkRow[1][i].MaxClock =
 			cpu_to_le16((uint16_t)
 			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_dcfclk_clk_in_khz) /
-			100);
+			1000);
 		table->WatermarkRow[1][i].MinUclk =
 			cpu_to_le16((uint16_t)
 			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_mem_clk_in_khz) /
-- 
2.20.1

[PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Sasha Levin]

From: Yu Zhao <yuzhao@google.com>

[ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ]

Userspace may request pitch alignment that is not supported by GPU.
Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU
DMA will go out of bound.

Cc: stable@vger.kernel.org # v4.2+
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Yu Zhao <yuzhao@google.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 686a26de50f91..6e67814d33e29 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -527,6 +527,16 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
 	struct drm_gem_object *obj;
 	struct amdgpu_framebuffer *amdgpu_fb;
 	int ret;
+	struct amdgpu_device *adev = dev->dev_private;
+	int cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
+	int pitch = mode_cmd->pitches[0] / cpp;
+
+	pitch = amdgpu_align_pitch(adev, pitch, cpp, false);
+	if (mode_cmd->pitches[0] != pitch) {
+		DRM_DEBUG_KMS("Invalid pitch: expecting %d but got %d\n",
+			      pitch, mode_cmd->pitches[0]);
+		return ERR_PTR(-EINVAL);
+	}
 
 	obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[0]);
 	if (obj ==  NULL) {
-- 
2.20.1

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Michel Dänzer]

On 2019-09-03 6:23 p.m., Sasha Levin wrote:
> From: Yu Zhao <yuzhao@google.com>
> 
> [ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ]

Hold your horses!

This commit and c4a32b266da7bb702e60381ca0c35eaddbc89a6c had to be
reverted, as they caused regressions. See commits
25ec429e86bb790e40387a550f0501d0ac55a47c &
92b0730eaf2d549fdfb10ecc8b71f34b9f472c12 .


This isn't bolstering confidence in how these patches are selected...
I'm also a little nervous about others which change values by an order
of magnitude. There were cases before where such patches were backported
to branches which didn't have other corresponding changes, so they ended
up breaking stuff instead of fixing anything.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Greg KH]

On Tue, Sep 03, 2019 at 06:40:43PM +0200, Michel Dänzer wrote:
> On 2019-09-03 6:23 p.m., Sasha Levin wrote:
> > From: Yu Zhao <yuzhao@google.com>
> > 
> > [ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ]
> 
> Hold your horses!
> 
> This commit and c4a32b266da7bb702e60381ca0c35eaddbc89a6c had to be
> reverted, as they caused regressions. See commits
> 25ec429e86bb790e40387a550f0501d0ac55a47c &
> 92b0730eaf2d549fdfb10ecc8b71f34b9f472c12 .
> 
> 
> This isn't bolstering confidence in how these patches are selected...

The patch _itself_ said to be backported to the stable trees from 4.2
and newer.  Why wouldn't we be confident in doing this?

If the patch doesn't want to be backported, then do not add the cc:
stable line to it...

greg k-h

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Sasha Levin]

On Tue, Sep 03, 2019 at 07:03:47PM +0200, Greg KH wrote:
>On Tue, Sep 03, 2019 at 06:40:43PM +0200, Michel Dänzer wrote:
>> On 2019-09-03 6:23 p.m., Sasha Levin wrote:
>> > From: Yu Zhao <yuzhao@google.com>
>> >
>> > [ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ]
>>
>> Hold your horses!
>>
>> This commit and c4a32b266da7bb702e60381ca0c35eaddbc89a6c had to be
>> reverted, as they caused regressions. See commits
>> 25ec429e86bb790e40387a550f0501d0ac55a47c &
>> 92b0730eaf2d549fdfb10ecc8b71f34b9f472c12 .
>>
>>
>> This isn't bolstering confidence in how these patches are selected...
>
>The patch _itself_ said to be backported to the stable trees from 4.2
>and newer.  Why wouldn't we be confident in doing this?
>
>If the patch doesn't want to be backported, then do not add the cc:
>stable line to it...

This patch was picked because it has a stable tag, which you presumably
saw as your Reviewed-by tag is in the patch. This is why it was
backported; it doesn't take AI to backport patches tagged for stable...

The revert of this patch, however:

 1. Didn't have a stable tag.
 2. Didn't have a "Fixes:" tag.
 3. Didn't have the usual "the reverts commit ..." string added by git
 when one does a revert.

Which is why we still kick patches for review, even though they had a
stable tag, just so people could take a look and confirm we're not
missing anything - like we did here.

I'm not sure what you expected me to do differently here.

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Daniel Vetter]

On Tue, Sep 3, 2019 at 10:01 PM Sasha Levin <sashal@kernel.org> wrote:
>
> On Tue, Sep 03, 2019 at 07:03:47PM +0200, Greg KH wrote:
> >On Tue, Sep 03, 2019 at 06:40:43PM +0200, Michel Dänzer wrote:
> >> On 2019-09-03 6:23 p.m., Sasha Levin wrote:
> >> > From: Yu Zhao <yuzhao@google.com>
> >> >
> >> > [ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ]
> >>
> >> Hold your horses!
> >>
> >> This commit and c4a32b266da7bb702e60381ca0c35eaddbc89a6c had to be
> >> reverted, as they caused regressions. See commits
> >> 25ec429e86bb790e40387a550f0501d0ac55a47c &
> >> 92b0730eaf2d549fdfb10ecc8b71f34b9f472c12 .
> >>
> >>
> >> This isn't bolstering confidence in how these patches are selected...
> >
> >The patch _itself_ said to be backported to the stable trees from 4.2
> >and newer.  Why wouldn't we be confident in doing this?
> >
> >If the patch doesn't want to be backported, then do not add the cc:
> >stable line to it...
>
> This patch was picked because it has a stable tag, which you presumably
> saw as your Reviewed-by tag is in the patch. This is why it was
> backported; it doesn't take AI to backport patches tagged for stable...
>
> The revert of this patch, however:
>
>  1. Didn't have a stable tag.
>  2. Didn't have a "Fixes:" tag.
>  3. Didn't have the usual "the reverts commit ..." string added by git
>  when one does a revert.
>
> Which is why we still kick patches for review, even though they had a
> stable tag, just so people could take a look and confirm we're not
> missing anything - like we did here.
>
> I'm not sure what you expected me to do differently here.

Yeah this looks like fail on the revert side, they need to reference
the reverted commit somehow ...

Alex, why got this dropped? Is this more fallout from the back&forth
shuffling you're doing between your internal branches behind the
firewall, and the public history?

Also adding Dave Airlie.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Michel Dänzer]

On 2019-09-03 10:16 p.m., Daniel Vetter wrote:
> On Tue, Sep 3, 2019 at 10:01 PM Sasha Levin <sashal@kernel.org> wrote:
>> On Tue, Sep 03, 2019 at 07:03:47PM +0200, Greg KH wrote:
>>> On Tue, Sep 03, 2019 at 06:40:43PM +0200, Michel Dänzer wrote:
>>>> On 2019-09-03 6:23 p.m., Sasha Levin wrote:
>>>>> From: Yu Zhao <yuzhao@google.com>
>>>>>
>>>>> [ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ]
>>>>
>>>> Hold your horses!
>>>>
>>>> This commit and c4a32b266da7bb702e60381ca0c35eaddbc89a6c had to be
>>>> reverted, as they caused regressions. See commits
>>>> 25ec429e86bb790e40387a550f0501d0ac55a47c &
>>>> 92b0730eaf2d549fdfb10ecc8b71f34b9f472c12 .
>>>>
>>>>
>>>> This isn't bolstering confidence in how these patches are selected...
>>>
>>> The patch _itself_ said to be backported to the stable trees from 4.2
>>> and newer.  Why wouldn't we be confident in doing this?
>>>
>>> If the patch doesn't want to be backported, then do not add the cc:
>>> stable line to it...
>>
>> This patch was picked because it has a stable tag, which you presumably
>> saw as your Reviewed-by tag is in the patch. This is why it was
>> backported; it doesn't take AI to backport patches tagged for stable...

The patches did point to gaps in validation of ioctl parameters passed
in by userspace. Unfortunately, they turned out to be too strict,
causing valid parameters to spuriously fail. If that wasn't the case,
and the patches didn't have stable tags, maybe we'd be having a
discussion about why they didn't have the tags now...


>> The revert of this patch, however:
>>
>>  1. Didn't have a stable tag.

I guess it didn't occur to me that was necessary, as the patches got
reverted within days.


>>  2. Didn't have a "Fixes:" tag.
>>  3. Didn't have the usual "the reverts commit ..." string added by git
>>  when one does a revert.

I suspect that's because there were no stable commit hashes to
reference, see below.


>> Which is why we still kick patches for review, even though they had a
>> stable tag, just so people could take a look and confirm we're not
>> missing anything - like we did here.
>>
>> I'm not sure what you expected me to do differently here.

Yeah, sorry, I didn't realize it's tricky for scripts to recognize that
the patches have been reverted in this case.


> Yeah this looks like fail on the revert side, they need to reference
> the reverted commit somehow ...
> 
> Alex, why got this dropped? Is this more fallout from the back&forth
> shuffling you're doing between your internal branches behind the
> firewall, and the public history?

I do suspect that was at least a contributing factor.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Sasha Levin]

On Wed, Sep 04, 2019 at 10:55:10AM +0200, Michel Dänzer wrote:
>On 2019-09-03 10:16 p.m., Daniel Vetter wrote:
>> On Tue, Sep 3, 2019 at 10:01 PM Sasha Levin <sashal@kernel.org> wrote:
>>> On Tue, Sep 03, 2019 at 07:03:47PM +0200, Greg KH wrote:
>>>> On Tue, Sep 03, 2019 at 06:40:43PM +0200, Michel Dänzer wrote:
>>>>> On 2019-09-03 6:23 p.m., Sasha Levin wrote:
>>>>>> From: Yu Zhao <yuzhao@google.com>
>>>>>>
>>>>>> [ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ]
>>>>>
>>>>> Hold your horses!
>>>>>
>>>>> This commit and c4a32b266da7bb702e60381ca0c35eaddbc89a6c had to be
>>>>> reverted, as they caused regressions. See commits
>>>>> 25ec429e86bb790e40387a550f0501d0ac55a47c &
>>>>> 92b0730eaf2d549fdfb10ecc8b71f34b9f472c12 .
>>>>>
>>>>>
>>>>> This isn't bolstering confidence in how these patches are selected...
>>>>
>>>> The patch _itself_ said to be backported to the stable trees from 4.2
>>>> and newer.  Why wouldn't we be confident in doing this?
>>>>
>>>> If the patch doesn't want to be backported, then do not add the cc:
>>>> stable line to it...
>>>
>>> This patch was picked because it has a stable tag, which you presumably
>>> saw as your Reviewed-by tag is in the patch. This is why it was
>>> backported; it doesn't take AI to backport patches tagged for stable...
>
>The patches did point to gaps in validation of ioctl parameters passed
>in by userspace. Unfortunately, they turned out to be too strict,
>causing valid parameters to spuriously fail. If that wasn't the case,
>and the patches didn't have stable tags, maybe we'd be having a
>discussion about why they didn't have the tags now...

That's fair, and we're definitely not complaining that these patches had
a stable tag, my comment was directed more towards the "This isn't
bolstering confidence in how these patches are selected" comment you've
made - we basically did what we were told to do and for some reason you
got upset :)

>>> The revert of this patch, however:
>>>
>>>  1. Didn't have a stable tag.
>
>I guess it didn't occur to me that was necessary, as the patches got
>reverted within days.

Since the original stable tagged patch made it upstream, we're bound to
try and select it for stable branches even if there are more changes or
reverts later on. We'll try to detect further fixes and reverts, but
we're limited by the metadata in the commit message.

>>>  2. Didn't have a "Fixes:" tag.
>>>  3. Didn't have the usual "the reverts commit ..." string added by git
>>>  when one does a revert.
>
>I suspect that's because there were no stable commit hashes to
>reference, see below.
>
>
>>> Which is why we still kick patches for review, even though they had a
>>> stable tag, just so people could take a look and confirm we're not
>>> missing anything - like we did here.
>>>
>>> I'm not sure what you expected me to do differently here.
>
>Yeah, sorry, I didn't realize it's tricky for scripts to recognize that
>the patches have been reverted in this case.

FWIW, I've added another test to my scripts to try and catch these cases
(Revert "%s"). It'll slow down the scripts a bit but it's better to get
it right rather than to be done quickly :)

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Michel Dänzer]

On 2019-09-04 2:08 p.m., Sasha Levin wrote:
> 
> FWIW, I've added another test to my scripts to try and catch these cases
> (Revert "%s"). It'll slow down the scripts a bit but it's better to get
> it right rather than to be done quickly :)

Indeed, thanks! And again sorry for the brouhaha, I just honestly didn't
realize before how tricky this case was for the scripts.


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

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Joe Perches]

On Wed, 2019-09-04 at 08:08 -0400, Sasha Levin wrote:
> it's better to get
> it right rather than to be done quickly :)

That also applies to the initial selection of
patches for the stable trees.

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Alex Deucher]

On Tue, Sep 3, 2019 at 4:16 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Tue, Sep 3, 2019 at 10:01 PM Sasha Levin <sashal@kernel.org> wrote:
> >
> > On Tue, Sep 03, 2019 at 07:03:47PM +0200, Greg KH wrote:
> > >On Tue, Sep 03, 2019 at 06:40:43PM +0200, Michel Dänzer wrote:
> > >> On 2019-09-03 6:23 p.m., Sasha Levin wrote:
> > >> > From: Yu Zhao <yuzhao@google.com>
> > >> >
> > >> > [ Upstream commit 89f23b6efef554766177bf51aa754bce14c3e7da ]
> > >>
> > >> Hold your horses!
> > >>
> > >> This commit and c4a32b266da7bb702e60381ca0c35eaddbc89a6c had to be
> > >> reverted, as they caused regressions. See commits
> > >> 25ec429e86bb790e40387a550f0501d0ac55a47c &
> > >> 92b0730eaf2d549fdfb10ecc8b71f34b9f472c12 .
> > >>
> > >>
> > >> This isn't bolstering confidence in how these patches are selected...
> > >
> > >The patch _itself_ said to be backported to the stable trees from 4.2
> > >and newer.  Why wouldn't we be confident in doing this?
> > >
> > >If the patch doesn't want to be backported, then do not add the cc:
> > >stable line to it...
> >
> > This patch was picked because it has a stable tag, which you presumably
> > saw as your Reviewed-by tag is in the patch. This is why it was
> > backported; it doesn't take AI to backport patches tagged for stable...
> >
> > The revert of this patch, however:
> >
> >  1. Didn't have a stable tag.
> >  2. Didn't have a "Fixes:" tag.
> >  3. Didn't have the usual "the reverts commit ..." string added by git
> >  when one does a revert.
> >
> > Which is why we still kick patches for review, even though they had a
> > stable tag, just so people could take a look and confirm we're not
> > missing anything - like we did here.
> >
> > I'm not sure what you expected me to do differently here.
>
> Yeah this looks like fail on the revert side, they need to reference
> the reverted commit somehow ...
>
> Alex, why got this dropped? Is this more fallout from the back&forth
> shuffling you're doing between your internal branches behind the
> firewall, and the public history?

The behind the firewall comments are not really helpful.  There aren't
any "behind the firewall" trees.  Everything is mirrored in public.
Yes it is annoying that we don't have a direct committer tree, but the
only shuffling is between public trees.  The problem is 90% of our
customers want packaged out of tree drivers rather than in tree
drivers because they are using an old distro or a custom distro or
something else so we have to do this dance.  I realize there are other
dances we could do to solve this problem, but they all have their own
set of costs and this is what we have now.  The patch shuffling
doesn't help, but regardless, the same thing could happen even with a
direct committer tree if someone missed the tag when committing.

Alex

>
> Also adding Dave Airlie.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Michel Dänzer]

On 2019-09-07 4:58 p.m., Alex Deucher wrote:
>
> The patch shuffling doesn't help, but regardless, the same thing could
> happen even with a direct committer tree if someone missed the tag when
> committing.

True, but in the latter case it would at least be possible to add tags
referencing persistent commit hashes regardless of when fix-ups happen,
whereas with the former this isn't possible before the original change
makes it to Linus or at least Dave.


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

[PATCH AUTOSEL 4.19 045/167] drm/amdgpu: validate user GEM object size

[Sasha Levin]

From: Yu Zhao <yuzhao@google.com>

[ Upstream commit c4a32b266da7bb702e60381ca0c35eaddbc89a6c ]

When creating frame buffer, userspace may request to attach to a
previously allocated GEM object that is smaller than what GPU
requires. Validation must be done to prevent out-of-bound DMA,
otherwise it could be exploited to reveal sensitive data.

This fix is not done in a common code path because individual
driver might have different requirement.

Cc: stable@vger.kernel.org # v4.2+
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Yu Zhao <yuzhao@google.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 6e67814d33e29..1035a47f81c9c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -527,6 +527,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
 	struct drm_gem_object *obj;
 	struct amdgpu_framebuffer *amdgpu_fb;
 	int ret;
+	int height;
 	struct amdgpu_device *adev = dev->dev_private;
 	int cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
 	int pitch = mode_cmd->pitches[0] / cpp;
@@ -551,6 +552,13 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
 		return ERR_PTR(-EINVAL);
 	}
 
+	height = ALIGN(mode_cmd->height, 8);
+	if (obj->size < pitch * height) {
+		DRM_DEBUG_KMS("Invalid GEM size: expecting >= %d but got %zu\n",
+			      pitch * height, obj->size);
+		return ERR_PTR(-EINVAL);
+	}
+
 	amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL);
 	if (amdgpu_fb == NULL) {
 		drm_gem_object_put_unlocked(obj);
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 023/167] drm/amd/pp: Fix truncated clock value when set watermark

[Sasha Levin]

From: Rex Zhu <Rex.Zhu@amd.com>

[ Upstream commit 4d454e9ffdb1ef5a51ebc147b5389c96048db683 ]

the clk value should be tranferred to MHz first and
then transfer to uint16. otherwise, the clock value
will be truncated.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reported-by: Hersen Wu <hersenxs.wu@amd.com>
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 .../gpu/drm/amd/powerplay/hwmgr/smu_helper.c  | 32 +++++++++----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c
index a321c465b7dce..cede78cdf28db 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu_helper.c
@@ -669,20 +669,20 @@ int smu_set_watermarks_for_clocks_ranges(void *wt_table,
 	for (i = 0; i < wm_with_clock_ranges->num_wm_dmif_sets; i++) {
 		table->WatermarkRow[1][i].MinClock =
 			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_dcfclk_clk_in_khz) /
-			1000);
+			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_dcfclk_clk_in_khz /
+			1000));
 		table->WatermarkRow[1][i].MaxClock =
 			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_dcfclk_clk_in_khz) /
-			1000);
+			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_dcfclk_clk_in_khz /
+			1000));
 		table->WatermarkRow[1][i].MinUclk =
 			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_mem_clk_in_khz) /
-			1000);
+			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_min_mem_clk_in_khz /
+			1000));
 		table->WatermarkRow[1][i].MaxUclk =
 			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_mem_clk_in_khz) /
-			1000);
+			(wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_max_mem_clk_in_khz /
+			1000));
 		table->WatermarkRow[1][i].WmSetting = (uint8_t)
 				wm_with_clock_ranges->wm_dmif_clocks_ranges[i].wm_set_id;
 	}
@@ -690,20 +690,20 @@ int smu_set_watermarks_for_clocks_ranges(void *wt_table,
 	for (i = 0; i < wm_with_clock_ranges->num_wm_mcif_sets; i++) {
 		table->WatermarkRow[0][i].MinClock =
 			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_socclk_clk_in_khz) /
-			1000);
+			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_socclk_clk_in_khz /
+			1000));
 		table->WatermarkRow[0][i].MaxClock =
 			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_socclk_clk_in_khz) /
-			1000);
+			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_socclk_clk_in_khz /
+			1000));
 		table->WatermarkRow[0][i].MinUclk =
 			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_mem_clk_in_khz) /
-			1000);
+			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_min_mem_clk_in_khz /
+			1000));
 		table->WatermarkRow[0][i].MaxUclk =
 			cpu_to_le16((uint16_t)
-			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_mem_clk_in_khz) /
-			1000);
+			(wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_max_mem_clk_in_khz /
+			1000));
 		table->WatermarkRow[0][i].WmSetting = (uint8_t)
 				wm_with_clock_ranges->wm_mcif_clocks_ranges[i].wm_set_id;
 	}
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 024/167] drm/amd/dm: Understand why attaching path/tile properties are needed

[Sasha Levin]

From: Lyude Paul <lyude@redhat.com>

[ Upstream commit 04ac4b0ed412f65230b456fcd9aa07e13befff89 ]

Path property is used for userspace to know what MST connector goes to what actual DRM DisplayPort connector, the tiling property is for tiling configurations. Not sure what else there is to figure out.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Jerry (Fangzhi) Zuo <Jerry.Zuo@amd.com>
Cc: Stable <stable@vger.kernel.org>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 ---
 1 file changed, 3 deletions(-)

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 59445c83f0238..c85bea70d9652 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
@@ -377,9 +377,6 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
 	drm_connector_attach_encoder(&aconnector->base,
 				     &aconnector->mst_encoder->base);
 
-	/*
-	 * TODO: understand why this one is needed
-	 */
 	drm_object_attach_property(
 		&connector->base,
 		dev->mode_config.path_property,
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 114/167] drm/amdkfd: Add missing Polaris10 ID

[Sasha Levin]

From: Kent Russell <kent.russell@amd.com>

[ Upstream commit 0a5a9c276c335870a1cecc4f02b76d6d6f663c8b ]

This was added to amdgpu but was missed in amdkfd

Signed-off-by: Kent Russell <kent.russell@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.rg
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 5aba50f63ac6f..938d0053a8208 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -310,6 +310,7 @@ static const struct kfd_deviceid supported_devices[] = {
 	{ 0x67CF, &polaris10_device_info },	/* Polaris10 */
 	{ 0x67D0, &polaris10_vf_device_info },	/* Polaris10 vf*/
 	{ 0x67DF, &polaris10_device_info },	/* Polaris10 */
+	{ 0x6FDF, &polaris10_device_info },	/* Polaris10 */
 	{ 0x67E0, &polaris11_device_info },	/* Polaris11 */
 	{ 0x67E1, &polaris11_device_info },	/* Polaris11 */
 	{ 0x67E3, &polaris11_device_info },	/* Polaris11 */
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 116/167] drm/amdgpu: fix ring test failure issue during s3 in vce 3.0 (V2)

[Sasha Levin]

From: Louis Li <Ching-shih.Li@amd.com>

[ Upstream commit ce0e22f5d886d1b56c7ab4347c45b9ac5fcc058d ]

[What]
vce ring test fails consistently during resume in s3 cycle, due to
mismatch read & write pointers.
On debug/analysis its found that rptr to be compared is not being
correctly updated/read, which leads to this failure.
Below is the failure signature:
	[drm:amdgpu_vce_ring_test_ring] *ERROR* amdgpu: ring 12 test failed
	[drm:amdgpu_device_ip_resume_phase2] *ERROR* resume of IP block <vce_v3_0> failed -110
	[drm:amdgpu_device_resume] *ERROR* amdgpu_device_ip_resume failed (-110).

[How]
fetch rptr appropriately, meaning move its read location further down
in the code flow.
With this patch applied the s3 failure is no more seen for >5k s3 cycles,
which otherwise is pretty consistent.

V2: remove reduntant fetch of rptr

Signed-off-by: Louis Li <Ching-shih.Li@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 5f3f540738187..17862b9ecccd7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -1070,7 +1070,7 @@ void amdgpu_vce_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
 int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring)
 {
 	struct amdgpu_device *adev = ring->adev;
-	uint32_t rptr = amdgpu_ring_get_rptr(ring);
+	uint32_t rptr;
 	unsigned i;
 	int r, timeout = adev->usec_timeout;
 
@@ -1084,6 +1084,9 @@ int amdgpu_vce_ring_test_ring(struct amdgpu_ring *ring)
 			  ring->idx, r);
 		return r;
 	}
+
+	rptr = amdgpu_ring_get_rptr(ring);
+
 	amdgpu_ring_write(ring, VCE_CMD_END);
 	amdgpu_ring_commit(ring);
 
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 117/167] drm/amdgpu/{uvd, vcn}: fetch ring's read_ptr after alloc

[Sasha Levin]

From: Shirish S <shirish.s@amd.com>

[ Upstream commit 517b91f4cde3043d77b2178548473e8545ef07cb ]

[What]
readptr read always returns zero, since most likely
these blocks are either power or clock gated.

[How]
fetch rptr after amdgpu_ring_alloc() which informs
the power management code that the block is about to be
used and hence the gating is turned off.

Signed-off-by: Louis Li <Ching-shih.Li@amd.com>
Signed-off-by: Shirish S <shirish.s@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 5 ++++-
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c   | 5 ++++-
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c   | 5 ++++-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 400fc74bbae27..205e683fb9206 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -431,7 +431,7 @@ int amdgpu_vcn_dec_ring_test_ib(struct amdgpu_ring *ring, long timeout)
 int amdgpu_vcn_enc_ring_test_ring(struct amdgpu_ring *ring)
 {
 	struct amdgpu_device *adev = ring->adev;
-	uint32_t rptr = amdgpu_ring_get_rptr(ring);
+	uint32_t rptr;
 	unsigned i;
 	int r;
 
@@ -441,6 +441,9 @@ int amdgpu_vcn_enc_ring_test_ring(struct amdgpu_ring *ring)
 			  ring->idx, r);
 		return r;
 	}
+
+	rptr = amdgpu_ring_get_rptr(ring);
+
 	amdgpu_ring_write(ring, VCN_ENC_CMD_END);
 	amdgpu_ring_commit(ring);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index d4070839ac809..80613a74df420 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -170,7 +170,7 @@ static void uvd_v6_0_enc_ring_set_wptr(struct amdgpu_ring *ring)
 static int uvd_v6_0_enc_ring_test_ring(struct amdgpu_ring *ring)
 {
 	struct amdgpu_device *adev = ring->adev;
-	uint32_t rptr = amdgpu_ring_get_rptr(ring);
+	uint32_t rptr;
 	unsigned i;
 	int r;
 
@@ -180,6 +180,9 @@ static int uvd_v6_0_enc_ring_test_ring(struct amdgpu_ring *ring)
 			  ring->idx, r);
 		return r;
 	}
+
+	rptr = amdgpu_ring_get_rptr(ring);
+
 	amdgpu_ring_write(ring, HEVC_ENC_CMD_END);
 	amdgpu_ring_commit(ring);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
index 057151b17b456..ce16b8329af04 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
@@ -175,7 +175,7 @@ static void uvd_v7_0_enc_ring_set_wptr(struct amdgpu_ring *ring)
 static int uvd_v7_0_enc_ring_test_ring(struct amdgpu_ring *ring)
 {
 	struct amdgpu_device *adev = ring->adev;
-	uint32_t rptr = amdgpu_ring_get_rptr(ring);
+	uint32_t rptr;
 	unsigned i;
 	int r;
 
@@ -188,6 +188,9 @@ static int uvd_v7_0_enc_ring_test_ring(struct amdgpu_ring *ring)
 			  ring->me, ring->idx, r);
 		return r;
 	}
+
+	rptr = amdgpu_ring_get_rptr(ring);
+
 	amdgpu_ring_write(ring, HEVC_ENC_CMD_END);
 	amdgpu_ring_commit(ring);
 
-- 
2.20.1

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