Re: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3)
On Fri, Feb 07, 2020 at 04:17:13PM -0500, Alex Deucher wrote:
> Split into init and register functions to avoid a segfault
> in some configs when the load/unload callbacks are removed.
>
> v2:
> - add back accidently dropped has_aux setting
> - set dev in late_register
>
> v3:
> - fix dp cec ordering
Why did you move this back out of the late_register callback when going
from v2->v3? In i915 we register the cec stuff from ->late_register, like
anything else userspace visible. Maybe follow-up patch (the idea behind
removing the ->load callback is to close all the driver load races,
instead of only open("/dev/dri/0"), which is protected by
drm_global_mutex). On this:
Reviewed-by: Daniel Vetter
Cheers, Daniel
>
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16
> drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 10 ++
> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 7 ++-
> 3 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index ec1501e3a63a..f355d9a752d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -1461,6 +1461,20 @@ static enum drm_mode_status
> amdgpu_connector_dp_mode_valid(struct drm_connector
> return MODE_OK;
> }
>
> +static int
> +amdgpu_connector_late_register(struct drm_connector *connector)
> +{
> + struct amdgpu_connector *amdgpu_connector =
> to_amdgpu_connector(connector);
> + int r = 0;
> +
> + if (amdgpu_connector->ddc_bus->has_aux) {
> + amdgpu_connector->ddc_bus->aux.dev =
> amdgpu_connector->base.kdev;
> + r = drm_dp_aux_register(_connector->ddc_bus->aux);
> + }
> +
> + return r;
> +}
> +
> static const struct drm_connector_helper_funcs
> amdgpu_connector_dp_helper_funcs = {
> .get_modes = amdgpu_connector_dp_get_modes,
> .mode_valid = amdgpu_connector_dp_mode_valid,
> @@ -1475,6 +1489,7 @@ static const struct drm_connector_funcs
> amdgpu_connector_dp_funcs = {
> .early_unregister = amdgpu_connector_unregister,
> .destroy = amdgpu_connector_destroy,
> .force = amdgpu_connector_dvi_force,
> + .late_register = amdgpu_connector_late_register,
> };
>
> static const struct drm_connector_funcs amdgpu_connector_edp_funcs = {
> @@ -1485,6 +1500,7 @@ static const struct drm_connector_funcs
> amdgpu_connector_edp_funcs = {
> .early_unregister = amdgpu_connector_unregister,
> .destroy = amdgpu_connector_destroy,
> .force = amdgpu_connector_dvi_force,
> + .late_register = amdgpu_connector_late_register,
> };
>
> void
> diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> index ea702a64f807..9b74cfdba7b8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> @@ -186,16 +186,10 @@ amdgpu_atombios_dp_aux_transfer(struct drm_dp_aux *aux,
> struct drm_dp_aux_msg *m
>
> void amdgpu_atombios_dp_aux_init(struct amdgpu_connector *amdgpu_connector)
> {
> - int ret;
> -
> amdgpu_connector->ddc_bus->rec.hpd = amdgpu_connector->hpd.hpd;
> - amdgpu_connector->ddc_bus->aux.dev = amdgpu_connector->base.kdev;
> amdgpu_connector->ddc_bus->aux.transfer =
> amdgpu_atombios_dp_aux_transfer;
> - ret = drm_dp_aux_register(_connector->ddc_bus->aux);
> - if (!ret)
> - amdgpu_connector->ddc_bus->has_aux = true;
> -
> - WARN(ret, "drm_dp_aux_register_i2c_bus() failed with error %d\n", ret);
> + drm_dp_aux_init(_connector->ddc_bus->aux);
> + amdgpu_connector->ddc_bus->has_aux = true;
> }
>
> /* general DP utility functions */
> 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 3959c942c88b..d5b9e72f2649 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
> @@ -155,6 +155,11 @@ amdgpu_dm_mst_connector_late_register(struct
> drm_connector *connector)
> struct amdgpu_dm_connector *amdgpu_dm_connector =
> to_amdgpu_dm_connector(connector);
> struct drm_dp_mst_port *port = amdgpu_dm_connector->port;
> + int r;
> +
> + r = drm_dp_aux_register(_dm_connector->dm_dp_aux.aux);
> + if (r)
> + return r;
>
> #if defined(CONFIG_DEBUG_FS)
> connector_debugfs_init(amdgpu_dm_connector);
> @@ -484,7 +489,7 @@ void amdgpu_dm_initialize_dp_connector(struct
> amdgpu_display_manager *dm,
> aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer;
> aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc;
>
> - drm_dp_aux_register(>dm_dp_aux.aux);
> + drm_dp_aux_init(>dm_dp_aux.aux);
>
[PATCH] drm/amdgpu: add is_raven_kicker judgement for raven1
From: changzhu
The rlc version of raven_kicer_rlc is different from the legacy rlc
version of raven_rlc. So it needs to add a judgement function for
raven_kicer_rlc and avoid disable GFXOFF when loading raven_kicer_rlc.
Change-Id: I00d726cc39eae4ea788c1d5faeb8ce75ec0b884d
Signed-off-by: changzhu
---
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 4d8b58e9d0ae..9b7ff783e9a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -1193,6 +1193,14 @@ static bool gfx_v9_0_should_disable_gfxoff(struct
pci_dev *pdev)
return false;
}
+static bool is_raven_kicker(struct amdgpu_device *adev)
+{
+ if (adev->pm.fw_version >= 0x41e2b)
+ return true;
+ else
+ return false;
+}
+
static void gfx_v9_0_check_if_need_gfxoff(struct amdgpu_device *adev)
{
if (gfx_v9_0_should_disable_gfxoff(adev->pdev))
@@ -1205,9 +1213,8 @@ static void gfx_v9_0_check_if_need_gfxoff(struct
amdgpu_device *adev)
break;
case CHIP_RAVEN:
if (!(adev->rev_id >= 0x8 || adev->pdev->device == 0x15d8) &&
- ((adev->gfx.rlc_fw_version != 106 &&
+ ((!is_raven_kicker(adev) &&
adev->gfx.rlc_fw_version < 531) ||
-(adev->gfx.rlc_fw_version == 53815) ||
(adev->gfx.rlc_feature_version < 1) ||
!adev->gfx.rlc.is_rlc_v2_1))
adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: add is_raven_kicker judgement for raven1
From: changzhu
The rlc version of raven_kicer_rlc is different from the legacy rlc
version of raven_rlc. So it needs to add a judgement function for
raven_kicer_rlc and avoid disable GFXOFF when loading raven_kicer_rlc.
Change-Id: I00d726cc39eae4ea788c1d5faeb8ce75ec0b884d
Signed-off-by: changzhu
---
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 4d8b58e9d0ae..9b7ff783e9a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -1193,6 +1193,14 @@ static bool gfx_v9_0_should_disable_gfxoff(struct
pci_dev *pdev)
return false;
}
+static bool is_raven_kicker(struct amdgpu_device *adev)
+{
+ if (adev->pm.fw_version >= 0x41e2b)
+ return true;
+ else
+ return false;
+}
+
static void gfx_v9_0_check_if_need_gfxoff(struct amdgpu_device *adev)
{
if (gfx_v9_0_should_disable_gfxoff(adev->pdev))
@@ -1205,9 +1213,8 @@ static void gfx_v9_0_check_if_need_gfxoff(struct
amdgpu_device *adev)
break;
case CHIP_RAVEN:
if (!(adev->rev_id >= 0x8 || adev->pdev->device == 0x15d8) &&
- ((adev->gfx.rlc_fw_version != 106 &&
+ ((!is_raven_kicker(adev) &&
adev->gfx.rlc_fw_version < 531) ||
-(adev->gfx.rlc_fw_version == 53815) ||
(adev->gfx.rlc_feature_version < 1) ||
!adev->gfx.rlc.is_rlc_v2_1))
adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)
On 2020-02-13 09:57, Huang Rui wrote:
> On Thu, Feb 13, 2020 at 11:30:43AM +0100, Christian König wrote:
>> Am 13.02.20 um 01:54 schrieb Luben Tuikov:
>>> Move from a per-CS secure flag (TMZ) to a per-IB
>>> secure flag.
>>
>> Well that seems to make a lot of sense to me, but need to bit of time to
>> read into the PM4 handling of TMZ.
>>
>> Especially what is the "start" parameter good for?
>
> I see this patch just now. And yes, that's my purpose for the discussion.
Yes, I did discuss this implementation with Alex and we seem to be thinking
this would be the most flexible and accomodating way at the moment.
>
> Thanks!
>
> Reviewed-by: Huang Rui
Thans Ray, I'll wheel it off to submit into amdgpu-staging-drm-next.
Regards,
Luben
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Luben Tuikov
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 --
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 -
>>> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 23 +++
>>> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c| 3 +--
>>> drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c| 3 +--
>>> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 3 +--
>>> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 20 ++--
>>> include/uapi/drm/amdgpu_drm.h| 7 ---
>>> 10 files changed, 44 insertions(+), 52 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> index 80ba6dfc54e2..f9fa6e104fef 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> @@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct
>>> amdgpu_cs_parser *p, union drm_amdgpu_cs
>>> if (ret)
>>> goto free_all_kdata;
>>> - p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
>>> -
>>> if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
>>> ret = -ECANCELED;
>>> goto free_all_kdata;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> index 6e0f97afb030..cae81914c821 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> @@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>>> unsigned num_ibs,
>>> uint64_t fence_ctx;
>>> uint32_t status = 0, alloc_size;
>>> unsigned fence_flags = 0;
>>> + bool secure;
>>> unsigned i;
>>> int r = 0;
>>> @@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>>> unsigned num_ibs,
>>> if (job && ring->funcs->emit_cntxcntl) {
>>> status |= job->preamble_status;
>>> status |= job->preemption_status;
>>> - amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
>>> + amdgpu_ring_emit_cntxcntl(ring, status);
>>> }
>>> + secure = false;
>>> for (i = 0; i < num_ibs; ++i) {
>>> ib = [i];
>>> @@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>>> unsigned num_ibs,
>>> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble
>>> CE ib must be inserted anyway */
>>> continue;
>>> + /* If this IB is TMZ, add frame TMZ start packet,
>>> +* else, turn off TMZ.
>>> +*/
>>> + if (ib->flags & AMDGPU_IB_FLAGS_SECURE &&
>>> ring->funcs->emit_tmz) {
>>> + if (!secure) {
>>> + secure = true;
>>> + amdgpu_ring_emit_tmz(ring, true);
>>> + }
>>> + } else if (secure) {
>>> + secure = false;
>>> + amdgpu_ring_emit_tmz(ring, false);
>>> + }
>>> +
>>> amdgpu_ring_emit_ib(ring, job, ib, status);
>>> status &= ~AMDGPU_HAVE_CTX_SWITCH;
>>> }
>>> - if (ring->funcs->emit_tmz)
>>> - amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
>>> + if (secure) {
>>> + secure = false;
>>> + amdgpu_ring_emit_tmz(ring, false);
>>> + }
>>> #ifdef CONFIG_X86_64
>>> if (!(adev->flags & AMD_IS_APU))
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>> index 31cb0203..2e2110dddb76 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>> @@ -61,9 +61,6 @@ struct amdgpu_job {
>>> /* user fence handling */
>>> uint64_tuf_addr;
>>> uint64_tuf_sequence;
>>> -
>>> - /* the job is due to a secure command submission */
>>> - boolsecure;
>>> };
>>> int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>
Re: [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)
On 2020-02-13 05:30, Christian König wrote:
> Am 13.02.20 um 01:54 schrieb Luben Tuikov:
>> Move from a per-CS secure flag (TMZ) to a per-IB
>> secure flag.
>
> Well that seems to make a lot of sense to me, but need to bit of time to
> read into the PM4 handling of TMZ.
Note that this patch is _not_ adding new functionality,
or anything new. It is not adding new PM3 or even PM4
functionality. Nothing new here.
It only moves the hierarchy from CS to IB of the
secure flag. That's all.
I think it is safe to submit it now, as it would alleviate
libdrm secure buffer submission.
>
> Especially what is the "start" parameter good for?
Note that this is existing code. It must've been reviewed
by someone when it was submitted and git-blame is good
with that.
The "start" parameter controls whether the packet
switches the ring to TMZ or non-TMZ processing.
Regards,
Luben
>
> Regards,
> Christian.
>
>>
>> Signed-off-by: Luben Tuikov
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 --
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 -
>> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 23 +++
>> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c| 3 +--
>> drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c| 3 +--
>> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 3 +--
>> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 20 ++--
>> include/uapi/drm/amdgpu_drm.h| 7 ---
>> 10 files changed, 44 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index 80ba6dfc54e2..f9fa6e104fef 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
>> *p, union drm_amdgpu_cs
>> if (ret)
>> goto free_all_kdata;
>>
>> -p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
>> -
>> if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
>> ret = -ECANCELED;
>> goto free_all_kdata;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 6e0f97afb030..cae81914c821 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>> unsigned num_ibs,
>> uint64_t fence_ctx;
>> uint32_t status = 0, alloc_size;
>> unsigned fence_flags = 0;
>> +bool secure;
>>
>> unsigned i;
>> int r = 0;
>> @@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>> unsigned num_ibs,
>> if (job && ring->funcs->emit_cntxcntl) {
>> status |= job->preamble_status;
>> status |= job->preemption_status;
>> -amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
>> +amdgpu_ring_emit_cntxcntl(ring, status);
>> }
>>
>> +secure = false;
>> for (i = 0; i < num_ibs; ++i) {
>> ib = [i];
>>
>> @@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>> unsigned num_ibs,
>> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble
>> CE ib must be inserted anyway */
>> continue;
>>
>> +/* If this IB is TMZ, add frame TMZ start packet,
>> + * else, turn off TMZ.
>> + */
>> +if (ib->flags & AMDGPU_IB_FLAGS_SECURE &&
>> ring->funcs->emit_tmz) {
>> +if (!secure) {
>> +secure = true;
>> +amdgpu_ring_emit_tmz(ring, true);
>> +}
>> +} else if (secure) {
>> +secure = false;
>> +amdgpu_ring_emit_tmz(ring, false);
>> +}
>> +
>> amdgpu_ring_emit_ib(ring, job, ib, status);
>> status &= ~AMDGPU_HAVE_CTX_SWITCH;
>> }
>>
>> -if (ring->funcs->emit_tmz)
>> -amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
>> +if (secure) {
>> +secure = false;
>> +amdgpu_ring_emit_tmz(ring, false);
>> +}
>>
>> #ifdef CONFIG_X86_64
>> if (!(adev->flags & AMD_IS_APU))
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>> index 31cb0203..2e2110dddb76 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>> @@ -61,9 +61,6 @@ struct amdgpu_job {
>> /* user fence handling */
>> uint64_tuf_addr;
>> uint64_tuf_sequence;
>> -
>> -/* the job is due to a secure command submission */
>> -boolsecure;
>> };
>>
>> int
Re: [PATCH 14/15] drm/amdgpu/ring: move debugfs init into core amdgpu debugfs
On Thu, Feb 13, 2020 at 12:28 PM Christian König
wrote:
>
> Am 13.02.20 um 15:32 schrieb Alex Deucher:
> > On Thu, Feb 13, 2020 at 5:17 AM Christian König
> > wrote:
> >> Am 13.02.20 um 10:54 schrieb Daniel Vetter:
> >>> On Fri, Feb 07, 2020 at 02:50:57PM -0500, Alex Deucher wrote:
> In order to remove the load and unload drm callbacks,
> we need to reorder the init sequence to move all the drm
> debugfs file handling. Do this for rings.
>
> Acked-by: Christian König
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 23 -
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c| 15 +++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h| 4
> 3 files changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index df3919ef886b..7379910790c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1218,7 +1218,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
>
> int amdgpu_debugfs_init(struct amdgpu_device *adev)
> {
> -int r;
> +int r, i;
>
> adev->debugfs_preempt =
> debugfs_create_file("amdgpu_preempt_ib", 0600,
> @@ -1268,12 +1268,33 @@ int amdgpu_debugfs_init(struct amdgpu_device
> *adev)
> }
> #endif
>
> +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> +struct amdgpu_ring *ring = adev->rings[i];
> +
> +if (!ring)
> +continue;
> +
> +if (amdgpu_debugfs_ring_init(adev, ring)) {
> +DRM_ERROR("Failed to register debugfs file for
> rings !\n");
> +}
> +}
> +
> return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list,
> ARRAY_SIZE(amdgpu_debugfs_list));
> }
>
> void amdgpu_debugfs_fini(struct amdgpu_device *adev)
> >>> btw debugfs_fini shouldn't be needed anymore, Greg KH removed this all.
> >>> drm core removes all debugfs files recusrively for you, there should be 0
> >>> need for debugfs cleanup.
> >> Oh, yes please. Removing that was on my TODO list for an eternity as well.
> >>
> >>> Also at least my tree doesn't even have this, where does this apply to?
> >> I would guess amd-staging-drm-next, but it might be that Alex is waiting
> >> for the next rebase to land.
> >>
> > Patches are against my drm-next branch which is based on fdo drm-next.
> > There are a number of files which the driver creates directly rather
> > than through drm.
>
> The last time I locked it I was about to completely nuke creating
> anything through DRM and just create it directly.
>
> As Daniel wrote we also don't have to remove anything explicitly, that
> is done implicitly when the whole directory is removed.
I dunno. Most of this stuff has been there for years. Maybe someone
wants to take a look if it can be further cleaned up. It builds fine
against current kernels.
Alex
>
> Christian.
>
> >
> > Alex
> >
> >> Christian.
> >>
> >>> -Daniel
> >>>
> {
> +int i;
> +
> +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> +struct amdgpu_ring *ring = adev->rings[i];
> +
> +if (!ring)
> +continue;
> +
> +amdgpu_debugfs_ring_fini(ring);
> +}
> amdgpu_ttm_debugfs_fini(adev);
> debugfs_remove(adev->debugfs_preempt);
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index e5c83e164d82..539be138260e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -48,9 +48,6 @@
> * wptr. The GPU then starts fetching commands and executes
> * them until the pointers are equal again.
> */
> -static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
> -struct amdgpu_ring *ring);
> -static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring);
>
> /**
> * amdgpu_ring_alloc - allocate space on the ring buffer
> @@ -334,10 +331,6 @@ int amdgpu_ring_init(struct amdgpu_device *adev,
> struct amdgpu_ring *ring,
> for (i = 0; i < DRM_SCHED_PRIORITY_MAX; ++i)
> atomic_set(>num_jobs[i], 0);
>
> -if (amdgpu_debugfs_ring_init(adev, ring)) {
> -DRM_ERROR("Failed to register debugfs file for rings !\n");
> -}
> -
> return 0;
> }
>
> @@ -367,8 +360,6 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring)
>
Re: [PATCH 14/15] drm/amdgpu/ring: move debugfs init into core amdgpu debugfs
Am 13.02.20 um 15:32 schrieb Alex Deucher:
On Thu, Feb 13, 2020 at 5:17 AM Christian König
wrote:
Am 13.02.20 um 10:54 schrieb Daniel Vetter:
On Fri, Feb 07, 2020 at 02:50:57PM -0500, Alex Deucher wrote:
In order to remove the load and unload drm callbacks,
we need to reorder the init sequence to move all the drm
debugfs file handling. Do this for rings.
Acked-by: Christian König
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 23 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c| 15 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h| 4
3 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index df3919ef886b..7379910790c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1218,7 +1218,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
int amdgpu_debugfs_init(struct amdgpu_device *adev)
{
-int r;
+int r, i;
adev->debugfs_preempt =
debugfs_create_file("amdgpu_preempt_ib", 0600,
@@ -1268,12 +1268,33 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
}
#endif
+for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
+struct amdgpu_ring *ring = adev->rings[i];
+
+if (!ring)
+continue;
+
+if (amdgpu_debugfs_ring_init(adev, ring)) {
+DRM_ERROR("Failed to register debugfs file for rings !\n");
+}
+}
+
return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list,
ARRAY_SIZE(amdgpu_debugfs_list));
}
void amdgpu_debugfs_fini(struct amdgpu_device *adev)
btw debugfs_fini shouldn't be needed anymore, Greg KH removed this all.
drm core removes all debugfs files recusrively for you, there should be 0
need for debugfs cleanup.
Oh, yes please. Removing that was on my TODO list for an eternity as well.
Also at least my tree doesn't even have this, where does this apply to?
I would guess amd-staging-drm-next, but it might be that Alex is waiting
for the next rebase to land.
Patches are against my drm-next branch which is based on fdo drm-next.
There are a number of files which the driver creates directly rather
than through drm.
The last time I locked it I was about to completely nuke creating
anything through DRM and just create it directly.
As Daniel wrote we also don't have to remove anything explicitly, that
is done implicitly when the whole directory is removed.
Christian.
Alex
Christian.
-Daniel
{
+int i;
+
+for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
+struct amdgpu_ring *ring = adev->rings[i];
+
+if (!ring)
+continue;
+
+amdgpu_debugfs_ring_fini(ring);
+}
amdgpu_ttm_debugfs_fini(adev);
debugfs_remove(adev->debugfs_preempt);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index e5c83e164d82..539be138260e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -48,9 +48,6 @@
* wptr. The GPU then starts fetching commands and executes
* them until the pointers are equal again.
*/
-static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
-struct amdgpu_ring *ring);
-static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring);
/**
* amdgpu_ring_alloc - allocate space on the ring buffer
@@ -334,10 +331,6 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct
amdgpu_ring *ring,
for (i = 0; i < DRM_SCHED_PRIORITY_MAX; ++i)
atomic_set(>num_jobs[i], 0);
-if (amdgpu_debugfs_ring_init(adev, ring)) {
-DRM_ERROR("Failed to register debugfs file for rings !\n");
-}
-
return 0;
}
@@ -367,8 +360,6 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring)
>gpu_addr,
(void **)>ring);
-amdgpu_debugfs_ring_fini(ring);
-
dma_fence_put(ring->vmid_wait);
ring->vmid_wait = NULL;
ring->me = 0;
@@ -485,8 +476,8 @@ static const struct file_operations
amdgpu_debugfs_ring_fops = {
#endif
-static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
-struct amdgpu_ring *ring)
+int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
+ struct amdgpu_ring *ring)
{
#if defined(CONFIG_DEBUG_FS)
struct drm_minor *minor = adev->ddev->primary;
@@ -507,7 +498,7 @@ static int amdgpu_debugfs_ring_init(struct amdgpu_device
*adev,
return 0;
}
-static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring)
+void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring)
{
#if defined(CONFIG_DEBUG_FS)
debugfs_remove(ring->ent);
diff --git
Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset
Am 13.02.20 um 15:30 schrieb Gerd Hoffmann: @@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo, (bo->tbo.mem.mem_type == TTM_PL_VRAM) ? >main_slot : >surfaces_slot; - WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset); - - /* TODO - need to hold one of the locks to read tbo.offset */ - return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset); + return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) + + slot->gpu_offset + offset); } --verbose please. The warning and the TODO should probably stay, don't they? I don't get the logic behind this change. We ran into the problem that the whole offset handling in TTM is actually completely hardware specific. E.g. Some need pfn, some need byte offsets, some need 32bit, some 64bit and some don't have a consistent offset at all (e.g. amdgpu for exmaple). So we try to move that back into the drivers to concentrate on memory management in TTM. The other chunks look sane, calculating slot->gpu_offset in setup_slot() certainly makes sense. Thanks for the review, Christian. cheers, Gerd ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3)
Anyone want to take a shot at this one?
Alex
On Fri, Feb 7, 2020 at 4:17 PM Alex Deucher wrote:
>
> Split into init and register functions to avoid a segfault
> in some configs when the load/unload callbacks are removed.
>
> v2:
> - add back accidently dropped has_aux setting
> - set dev in late_register
>
> v3:
> - fix dp cec ordering
>
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16
> drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 10 ++
> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 7 ++-
> 3 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index ec1501e3a63a..f355d9a752d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -1461,6 +1461,20 @@ static enum drm_mode_status
> amdgpu_connector_dp_mode_valid(struct drm_connector
> return MODE_OK;
> }
>
> +static int
> +amdgpu_connector_late_register(struct drm_connector *connector)
> +{
> + struct amdgpu_connector *amdgpu_connector =
> to_amdgpu_connector(connector);
> + int r = 0;
> +
> + if (amdgpu_connector->ddc_bus->has_aux) {
> + amdgpu_connector->ddc_bus->aux.dev =
> amdgpu_connector->base.kdev;
> + r = drm_dp_aux_register(_connector->ddc_bus->aux);
> + }
> +
> + return r;
> +}
> +
> static const struct drm_connector_helper_funcs
> amdgpu_connector_dp_helper_funcs = {
> .get_modes = amdgpu_connector_dp_get_modes,
> .mode_valid = amdgpu_connector_dp_mode_valid,
> @@ -1475,6 +1489,7 @@ static const struct drm_connector_funcs
> amdgpu_connector_dp_funcs = {
> .early_unregister = amdgpu_connector_unregister,
> .destroy = amdgpu_connector_destroy,
> .force = amdgpu_connector_dvi_force,
> + .late_register = amdgpu_connector_late_register,
> };
>
> static const struct drm_connector_funcs amdgpu_connector_edp_funcs = {
> @@ -1485,6 +1500,7 @@ static const struct drm_connector_funcs
> amdgpu_connector_edp_funcs = {
> .early_unregister = amdgpu_connector_unregister,
> .destroy = amdgpu_connector_destroy,
> .force = amdgpu_connector_dvi_force,
> + .late_register = amdgpu_connector_late_register,
> };
>
> void
> diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> index ea702a64f807..9b74cfdba7b8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> @@ -186,16 +186,10 @@ amdgpu_atombios_dp_aux_transfer(struct drm_dp_aux *aux,
> struct drm_dp_aux_msg *m
>
> void amdgpu_atombios_dp_aux_init(struct amdgpu_connector *amdgpu_connector)
> {
> - int ret;
> -
> amdgpu_connector->ddc_bus->rec.hpd = amdgpu_connector->hpd.hpd;
> - amdgpu_connector->ddc_bus->aux.dev = amdgpu_connector->base.kdev;
> amdgpu_connector->ddc_bus->aux.transfer =
> amdgpu_atombios_dp_aux_transfer;
> - ret = drm_dp_aux_register(_connector->ddc_bus->aux);
> - if (!ret)
> - amdgpu_connector->ddc_bus->has_aux = true;
> -
> - WARN(ret, "drm_dp_aux_register_i2c_bus() failed with error %d\n",
> ret);
> + drm_dp_aux_init(_connector->ddc_bus->aux);
> + amdgpu_connector->ddc_bus->has_aux = true;
> }
>
> /* general DP utility functions */
> 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 3959c942c88b..d5b9e72f2649 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
> @@ -155,6 +155,11 @@ amdgpu_dm_mst_connector_late_register(struct
> drm_connector *connector)
> struct amdgpu_dm_connector *amdgpu_dm_connector =
> to_amdgpu_dm_connector(connector);
> struct drm_dp_mst_port *port = amdgpu_dm_connector->port;
> + int r;
> +
> + r = drm_dp_aux_register(_dm_connector->dm_dp_aux.aux);
> + if (r)
> + return r;
>
> #if defined(CONFIG_DEBUG_FS)
> connector_debugfs_init(amdgpu_dm_connector);
> @@ -484,7 +489,7 @@ void amdgpu_dm_initialize_dp_connector(struct
> amdgpu_display_manager *dm,
> aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer;
> aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc;
>
> - drm_dp_aux_register(>dm_dp_aux.aux);
> + drm_dp_aux_init(>dm_dp_aux.aux);
> drm_dp_cec_register_connector(>dm_dp_aux.aux,
> >base);
>
> --
> 2.24.1
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 4/4] drm/radeon: align short build log
This beautifies the build log. [Before] HOSTCC drivers/gpu/drm/radeon/mkregtable MKREGTABLE drivers/gpu/drm/radeon/r100_reg_safe.h MKREGTABLE drivers/gpu/drm/radeon/rn50_reg_safe.h CC [M] drivers/gpu/drm/radeon/r100.o MKREGTABLE drivers/gpu/drm/radeon/r300_reg_safe.h CC [M] drivers/gpu/drm/radeon/r300.o [After] HOSTCC drivers/gpu/drm/radeon/mkregtable MKREG drivers/gpu/drm/radeon/r100_reg_safe.h MKREG drivers/gpu/drm/radeon/rn50_reg_safe.h CC [M] drivers/gpu/drm/radeon/r100.o MKREG drivers/gpu/drm/radeon/r300_reg_safe.h CC [M] drivers/gpu/drm/radeon/r300.o Signed-off-by: Masahiro Yamada --- drivers/gpu/drm/radeon/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile index 480a8d4a3c82..11c97edde54d 100644 --- a/drivers/gpu/drm/radeon/Makefile +++ b/drivers/gpu/drm/radeon/Makefile @@ -6,7 +6,7 @@ hostprogs := mkregtable targets := rn50_reg_safe.h r100_reg_safe.h r200_reg_safe.h rv515_reg_safe.h r300_reg_safe.h r420_reg_safe.h rs600_reg_safe.h r600_reg_safe.h evergreen_reg_safe.h cayman_reg_safe.h -quiet_cmd_mkregtable = MKREGTABLE $@ +quiet_cmd_mkregtable = MKREG $@ cmd_mkregtable = $(obj)/mkregtable $< > $@ $(obj)/%_reg_safe.h: $(src)/reg_srcs/% $(obj)/mkregtable FORCE -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/4] drm/radeon: use pattern rule to avoid code duplication in Makefile
This Makefile repeats similar build rules. Use a pattern rule. Signed-off-by: Masahiro Yamada --- drivers/gpu/drm/radeon/Makefile | 29 + 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile index fda115cefe4d..480a8d4a3c82 100644 --- a/drivers/gpu/drm/radeon/Makefile +++ b/drivers/gpu/drm/radeon/Makefile @@ -9,34 +9,7 @@ targets := rn50_reg_safe.h r100_reg_safe.h r200_reg_safe.h rv515_reg_safe.h r300 quiet_cmd_mkregtable = MKREGTABLE $@ cmd_mkregtable = $(obj)/mkregtable $< > $@ -$(obj)/rn50_reg_safe.h: $(src)/reg_srcs/rn50 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/r100_reg_safe.h: $(src)/reg_srcs/r100 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/r200_reg_safe.h: $(src)/reg_srcs/r200 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/rv515_reg_safe.h: $(src)/reg_srcs/rv515 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/r300_reg_safe.h: $(src)/reg_srcs/r300 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/r420_reg_safe.h: $(src)/reg_srcs/r420 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/rs600_reg_safe.h: $(src)/reg_srcs/rs600 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/r600_reg_safe.h: $(src)/reg_srcs/r600 $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/evergreen_reg_safe.h: $(src)/reg_srcs/evergreen $(obj)/mkregtable FORCE - $(call if_changed,mkregtable) - -$(obj)/cayman_reg_safe.h: $(src)/reg_srcs/cayman $(obj)/mkregtable FORCE +$(obj)/%_reg_safe.h: $(src)/reg_srcs/% $(obj)/mkregtable FORCE $(call if_changed,mkregtable) $(obj)/r100.o: $(obj)/r100_reg_safe.h $(obj)/rn50_reg_safe.h -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/4] drm/radeon: fix build rules of *_reg_safe.h
if_changed must have FORCE as a prerequisite, and the targets must be added to 'targets'. Signed-off-by: Masahiro Yamada --- drivers/gpu/drm/radeon/Makefile | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile index 9d5d3dc1011f..fda115cefe4d 100644 --- a/drivers/gpu/drm/radeon/Makefile +++ b/drivers/gpu/drm/radeon/Makefile @@ -4,39 +4,39 @@ # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. hostprogs := mkregtable -clean-files := rn50_reg_safe.h r100_reg_safe.h r200_reg_safe.h rv515_reg_safe.h r300_reg_safe.h r420_reg_safe.h rs600_reg_safe.h r600_reg_safe.h evergreen_reg_safe.h cayman_reg_safe.h +targets := rn50_reg_safe.h r100_reg_safe.h r200_reg_safe.h rv515_reg_safe.h r300_reg_safe.h r420_reg_safe.h rs600_reg_safe.h r600_reg_safe.h evergreen_reg_safe.h cayman_reg_safe.h quiet_cmd_mkregtable = MKREGTABLE $@ cmd_mkregtable = $(obj)/mkregtable $< > $@ -$(obj)/rn50_reg_safe.h: $(src)/reg_srcs/rn50 $(obj)/mkregtable +$(obj)/rn50_reg_safe.h: $(src)/reg_srcs/rn50 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/r100_reg_safe.h: $(src)/reg_srcs/r100 $(obj)/mkregtable +$(obj)/r100_reg_safe.h: $(src)/reg_srcs/r100 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/r200_reg_safe.h: $(src)/reg_srcs/r200 $(obj)/mkregtable +$(obj)/r200_reg_safe.h: $(src)/reg_srcs/r200 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/rv515_reg_safe.h: $(src)/reg_srcs/rv515 $(obj)/mkregtable +$(obj)/rv515_reg_safe.h: $(src)/reg_srcs/rv515 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/r300_reg_safe.h: $(src)/reg_srcs/r300 $(obj)/mkregtable +$(obj)/r300_reg_safe.h: $(src)/reg_srcs/r300 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/r420_reg_safe.h: $(src)/reg_srcs/r420 $(obj)/mkregtable +$(obj)/r420_reg_safe.h: $(src)/reg_srcs/r420 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/rs600_reg_safe.h: $(src)/reg_srcs/rs600 $(obj)/mkregtable +$(obj)/rs600_reg_safe.h: $(src)/reg_srcs/rs600 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/r600_reg_safe.h: $(src)/reg_srcs/r600 $(obj)/mkregtable +$(obj)/r600_reg_safe.h: $(src)/reg_srcs/r600 $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/evergreen_reg_safe.h: $(src)/reg_srcs/evergreen $(obj)/mkregtable +$(obj)/evergreen_reg_safe.h: $(src)/reg_srcs/evergreen $(obj)/mkregtable FORCE $(call if_changed,mkregtable) -$(obj)/cayman_reg_safe.h: $(src)/reg_srcs/cayman $(obj)/mkregtable +$(obj)/cayman_reg_safe.h: $(src)/reg_srcs/cayman $(obj)/mkregtable FORCE $(call if_changed,mkregtable) $(obj)/r100.o: $(obj)/r100_reg_safe.h $(obj)/rn50_reg_safe.h -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)
On Thu, Feb 13, 2020 at 11:30:43AM +0100, Christian König wrote:
> Am 13.02.20 um 01:54 schrieb Luben Tuikov:
> > Move from a per-CS secure flag (TMZ) to a per-IB
> > secure flag.
>
> Well that seems to make a lot of sense to me, but need to bit of time to
> read into the PM4 handling of TMZ.
>
> Especially what is the "start" parameter good for?
I see this patch just now. And yes, that's my purpose for the discussion.
Thanks!
Reviewed-by: Huang Rui
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Luben Tuikov
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 --
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 -
> > drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 23 +++
> > drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c| 3 +--
> > drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c| 3 +--
> > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 3 +--
> > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 20 ++--
> > include/uapi/drm/amdgpu_drm.h| 7 ---
> > 10 files changed, 44 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 80ba6dfc54e2..f9fa6e104fef 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct
> > amdgpu_cs_parser *p, union drm_amdgpu_cs
> > if (ret)
> > goto free_all_kdata;
> > - p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
> > -
> > if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
> > ret = -ECANCELED;
> > goto free_all_kdata;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > index 6e0f97afb030..cae81914c821 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > @@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
> > unsigned num_ibs,
> > uint64_t fence_ctx;
> > uint32_t status = 0, alloc_size;
> > unsigned fence_flags = 0;
> > + bool secure;
> > unsigned i;
> > int r = 0;
> > @@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
> > unsigned num_ibs,
> > if (job && ring->funcs->emit_cntxcntl) {
> > status |= job->preamble_status;
> > status |= job->preemption_status;
> > - amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
> > + amdgpu_ring_emit_cntxcntl(ring, status);
> > }
> > + secure = false;
> > for (i = 0; i < num_ibs; ++i) {
> > ib = [i];
> > @@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
> > unsigned num_ibs,
> > !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble
> > CE ib must be inserted anyway */
> > continue;
> > + /* If this IB is TMZ, add frame TMZ start packet,
> > +* else, turn off TMZ.
> > +*/
> > + if (ib->flags & AMDGPU_IB_FLAGS_SECURE &&
> > ring->funcs->emit_tmz) {
> > + if (!secure) {
> > + secure = true;
> > + amdgpu_ring_emit_tmz(ring, true);
> > + }
> > + } else if (secure) {
> > + secure = false;
> > + amdgpu_ring_emit_tmz(ring, false);
> > + }
> > +
> > amdgpu_ring_emit_ib(ring, job, ib, status);
> > status &= ~AMDGPU_HAVE_CTX_SWITCH;
> > }
> > - if (ring->funcs->emit_tmz)
> > - amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
> > + if (secure) {
> > + secure = false;
> > + amdgpu_ring_emit_tmz(ring, false);
> > + }
> > #ifdef CONFIG_X86_64
> > if (!(adev->flags & AMD_IS_APU))
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > index 31cb0203..2e2110dddb76 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > @@ -61,9 +61,6 @@ struct amdgpu_job {
> > /* user fence handling */
> > uint64_tuf_addr;
> > uint64_tuf_sequence;
> > -
> > - /* the job is due to a secure command submission */
> > - boolsecure;
> > };
> > int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > index 5134d0dd6dc2..930316e60155 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > @@ -158,8 +158,7 @@ struct amdgpu_ring_funcs {
> > void (*begin_use)(struct amdgpu_ring *ring);
> >
Re: [RFC PATCH 0/6] do not store GPU address in TTM
On Thu, Feb 13, 2020 at 01:01:57PM +0100, Nirmoy Das wrote: > With this patch series I am trying to remove GPU address dependency in > TTM and moving GPU address calculation to individual drm drivers. > This is required[1] to continue the work started by Brian Welty to create > struct drm_mem_region which can be leverage by DRM cgroup controller to > manage memory > limits. > > > I have only manage to test amdgpu driver as I only have GPU for that. > I might be doing something really stupid while calculeting gpu offset for > some of the drivers so please be patient and let me know how can I improve > that. > > [1] > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.mail-archive.com%2Fdri-devel%40lists.freedesktop.org%2Fmsg272238.htmldata=02%7C01%7Cray.huang%40amd.com%7Cad8c8464b13e4764556008d7b07c3344%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171919805856109sdata=zlA%2FHePGKcILKg7Ezc9CGc%2FWXJkRa5xmrBznvJcAomk%3Dreserved=0 Looks good for me as well for amd part. Acked-by: Huang Rui > > Nirmoy Das (6): > drm/amdgpu: move ttm bo->offset to amdgpu_bo > drm/radeon: don't use ttm bo->offset > drm/vmwgfx: don't use ttm bo->offset > drm/nouveau: don't use ttm bo->offset > drm/qxl: don't use ttm bo->offset > drm/ttm: do not keep GPU dependent addresses > > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 29 - > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 4 +-- > drivers/gpu/drm/nouveau/dispnv04/crtc.c | 6 ++--- > drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +- > drivers/gpu/drm/nouveau/dispnv04/overlay.c | 6 ++--- > drivers/gpu/drm/nouveau/dispnv50/base507c.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/core507d.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_abi16.c | 8 +++--- > drivers/gpu/drm/nouveau/nouveau_bo.c| 1 + > drivers/gpu/drm/nouveau/nouveau_bo.h| 3 +++ > drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +++ > drivers/gpu/drm/qxl/qxl_drv.h | 6 ++--- > drivers/gpu/drm/qxl/qxl_kms.c | 3 +++ > drivers/gpu/drm/qxl/qxl_object.h| 5 > drivers/gpu/drm/qxl/qxl_ttm.c | 9 --- > drivers/gpu/drm/radeon/radeon.h | 1 + > drivers/gpu/drm/radeon/radeon_object.h | 16 +++- > drivers/gpu/drm/radeon/radeon_ttm.c | 4 +-- > drivers/gpu/drm/ttm/ttm_bo.c| 7 - > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +-- > drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c| 2 +- > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 -- > include/drm/ttm/ttm_bo_api.h| 2 -- > include/drm/ttm/ttm_bo_driver.h | 1 - > 33 files changed, 99 insertions(+), 72 deletions(-) > > -- > 2.25.0 > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cray.huang%40amd.com%7Cad8c8464b13e4764556008d7b07c3344%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171919805856109sdata=lnJkwlCEbUmtsBhBY94rB3hRgaYg4ENQ0DNTXxxwPL4%3Dreserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 14/15] drm/amdgpu/ring: move debugfs init into core amdgpu debugfs
On Thu, Feb 13, 2020 at 5:17 AM Christian König
wrote:
>
> Am 13.02.20 um 10:54 schrieb Daniel Vetter:
> > On Fri, Feb 07, 2020 at 02:50:57PM -0500, Alex Deucher wrote:
> >> In order to remove the load and unload drm callbacks,
> >> we need to reorder the init sequence to move all the drm
> >> debugfs file handling. Do this for rings.
> >>
> >> Acked-by: Christian König
> >> Signed-off-by: Alex Deucher
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 23 -
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c| 15 +++---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h| 4
> >> 3 files changed, 29 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> >> index df3919ef886b..7379910790c9 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> >> @@ -1218,7 +1218,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
> >>
> >> int amdgpu_debugfs_init(struct amdgpu_device *adev)
> >> {
> >> -int r;
> >> +int r, i;
> >>
> >> adev->debugfs_preempt =
> >> debugfs_create_file("amdgpu_preempt_ib", 0600,
> >> @@ -1268,12 +1268,33 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
> >> }
> >> #endif
> >>
> >> +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> >> +struct amdgpu_ring *ring = adev->rings[i];
> >> +
> >> +if (!ring)
> >> +continue;
> >> +
> >> +if (amdgpu_debugfs_ring_init(adev, ring)) {
> >> +DRM_ERROR("Failed to register debugfs file for rings
> >> !\n");
> >> +}
> >> +}
> >> +
> >> return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list,
> >> ARRAY_SIZE(amdgpu_debugfs_list));
> >> }
> >>
> >> void amdgpu_debugfs_fini(struct amdgpu_device *adev)
> > btw debugfs_fini shouldn't be needed anymore, Greg KH removed this all.
> > drm core removes all debugfs files recusrively for you, there should be 0
> > need for debugfs cleanup.
>
> Oh, yes please. Removing that was on my TODO list for an eternity as well.
>
> >
> > Also at least my tree doesn't even have this, where does this apply to?
>
> I would guess amd-staging-drm-next, but it might be that Alex is waiting
> for the next rebase to land.
>
Patches are against my drm-next branch which is based on fdo drm-next.
There are a number of files which the driver creates directly rather
than through drm.
Alex
> Christian.
>
> > -Daniel
> >
> >> {
> >> +int i;
> >> +
> >> +for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> >> +struct amdgpu_ring *ring = adev->rings[i];
> >> +
> >> +if (!ring)
> >> +continue;
> >> +
> >> +amdgpu_debugfs_ring_fini(ring);
> >> +}
> >> amdgpu_ttm_debugfs_fini(adev);
> >> debugfs_remove(adev->debugfs_preempt);
> >> }
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> >> index e5c83e164d82..539be138260e 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> >> @@ -48,9 +48,6 @@
> >>* wptr. The GPU then starts fetching commands and executes
> >>* them until the pointers are equal again.
> >>*/
> >> -static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
> >> -struct amdgpu_ring *ring);
> >> -static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring);
> >>
> >> /**
> >>* amdgpu_ring_alloc - allocate space on the ring buffer
> >> @@ -334,10 +331,6 @@ int amdgpu_ring_init(struct amdgpu_device *adev,
> >> struct amdgpu_ring *ring,
> >> for (i = 0; i < DRM_SCHED_PRIORITY_MAX; ++i)
> >> atomic_set(>num_jobs[i], 0);
> >>
> >> -if (amdgpu_debugfs_ring_init(adev, ring)) {
> >> -DRM_ERROR("Failed to register debugfs file for rings !\n");
> >> -}
> >> -
> >> return 0;
> >> }
> >>
> >> @@ -367,8 +360,6 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring)
> >>>gpu_addr,
> >>(void **)>ring);
> >>
> >> -amdgpu_debugfs_ring_fini(ring);
> >> -
> >> dma_fence_put(ring->vmid_wait);
> >> ring->vmid_wait = NULL;
> >> ring->me = 0;
> >> @@ -485,8 +476,8 @@ static const struct file_operations
> >> amdgpu_debugfs_ring_fops = {
> >>
> >> #endif
> >>
> >> -static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
> >> -struct amdgpu_ring *ring)
> >> +int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
> >> + struct amdgpu_ring *ring)
> >> {
> >> #if defined(CONFIG_DEBUG_FS)
> >> struct drm_minor *minor = adev->ddev->primary;
> >> @@ -507,7 +498,7 @@ static int amdgpu_debugfs_ring_init(struct
> >> amdgpu_device *adev,
> >>
Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset
> @@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct > qxl_bo *bo, > (bo->tbo.mem.mem_type == TTM_PL_VRAM) > ? >main_slot : >surfaces_slot; > > - WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset); > - > - /* TODO - need to hold one of the locks to read tbo.offset */ > - return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset); > + return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) + > + slot->gpu_offset + offset); > } --verbose please. I don't get the logic behind this change. The other chunks look sane, calculating slot->gpu_offset in setup_slot() certainly makes sense. cheers, Gerd ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset
Am 13.02.20 um 13:31 schrieb Nirmoy: On 2/13/20 1:18 PM, Christian König wrote: Looks like most of the patch is missing? We should have quite a number of uses of the BO offset in radeon or do all of those go through radeon_bo_gpu_offset? Compilation worked so I think all those(bo->offset) accesses went through radeon_bo_gpu_offset. Cool, than that is a lot easier to implement than I thought it would be. I assume you don't have Radeon hardware lying around to test this? If you can you give me a branch on the AMD (or public) servers I can give it at least a quick round of testing. Christian. If yes then the change is much smaller than I thought i needs to be. Christian. Regards, Nirmoy ___ 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: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset
On 2/13/20 1:18 PM, Christian König wrote: Looks like most of the patch is missing? We should have quite a number of uses of the BO offset in radeon or do all of those go through radeon_bo_gpu_offset? Compilation worked so I think all those(bo->offset) accesses went through radeon_bo_gpu_offset. If yes then the change is much smaller than I thought i needs to be. Christian. Regards, Nirmoy ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH 0/6] do not store GPU address in TTM
Am 13.02.20 um 13:01 schrieb Nirmoy Das: With this patch series I am trying to remove GPU address dependency in TTM and moving GPU address calculation to individual drm drivers. This is required[1] to continue the work started by Brian Welty to create struct drm_mem_region which can be leverage by DRM cgroup controller to manage memory limits. Nice work. I wouldn't say that it is necessary for [1], but it is certainly nice to have that cleaned up. Christian. I have only manage to test amdgpu driver as I only have GPU for that. I might be doing something really stupid while calculeting gpu offset for some of the drivers so please be patient and let me know how can I improve that. [1] https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg272238.html Nirmoy Das (6): drm/amdgpu: move ttm bo->offset to amdgpu_bo drm/radeon: don't use ttm bo->offset drm/vmwgfx: don't use ttm bo->offset drm/nouveau: don't use ttm bo->offset drm/qxl: don't use ttm bo->offset drm/ttm: do not keep GPU dependent addresses drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 29 - drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 4 +-- drivers/gpu/drm/nouveau/dispnv04/crtc.c | 6 ++--- drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 6 ++--- drivers/gpu/drm/nouveau/dispnv50/base507c.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/core507d.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 2 +- drivers/gpu/drm/nouveau/nouveau_abi16.c | 8 +++--- drivers/gpu/drm/nouveau/nouveau_bo.c| 1 + drivers/gpu/drm/nouveau/nouveau_bo.h| 3 +++ drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +++ drivers/gpu/drm/qxl/qxl_drv.h | 6 ++--- drivers/gpu/drm/qxl/qxl_kms.c | 3 +++ drivers/gpu/drm/qxl/qxl_object.h| 5 drivers/gpu/drm/qxl/qxl_ttm.c | 9 --- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_object.h | 16 +++- drivers/gpu/drm/radeon/radeon_ttm.c | 4 +-- drivers/gpu/drm/ttm/ttm_bo.c| 7 - drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c| 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 -- include/drm/ttm/ttm_bo_api.h| 2 -- include/drm/ttm/ttm_bo_driver.h | 1 - 33 files changed, 99 insertions(+), 72 deletions(-) -- 2.25.0 ___ 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: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset
Looks like most of the patch is missing?
We should have quite a number of uses of the BO offset in radeon or do
all of those go through radeon_bo_gpu_offset?
If yes then the change is much smaller than I thought i needs to be.
Christian.
Am 13.02.20 um 13:01 schrieb Nirmoy Das:
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/radeon/radeon.h| 1 +
drivers/gpu/drm/radeon/radeon_object.h | 16 +++-
drivers/gpu/drm/radeon/radeon_ttm.c| 4 +---
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index d59b004f6695..97cfcc2870af 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2823,6 +2823,7 @@ extern void radeon_ttm_set_active_vram_size(struct
radeon_device *rdev, u64 size
extern void radeon_program_register_sequence(struct radeon_device *rdev,
const u32 *registers,
const u32 array_size);
+struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev);
/*
* vm
diff --git a/drivers/gpu/drm/radeon/radeon_object.h
b/drivers/gpu/drm/radeon/radeon_object.h
index d23f2ed4126e..4d37571c7ff5 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo *bo)
*/
static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo)
{
- return bo->tbo.offset;
+ struct radeon_device *rdev;
+ u64 start = 0;
+
+ rdev = radeon_get_rdev(bo->tbo.bdev);
+
+ switch(bo->tbo.mem.mem_type) {
+ case TTM_PL_TT:
+ start = rdev->mc.gtt_start;
+ break;
+ case TTM_PL_VRAM:
+ start = rdev->mc.vram_start;
+ break;
+ }
+
+ return (bo->tbo.mem.start << PAGE_SHIFT) + start;
}
static inline unsigned long radeon_bo_size(struct radeon_bo *bo)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 098bc9f40b98..b10654494262 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -56,7 +56,7 @@
static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);
-static struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
+struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
{
struct radeon_mman *mman;
struct radeon_device *rdev;
@@ -87,7 +87,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
break;
case TTM_PL_TT:
man->func = _bo_manager_func;
- man->gpu_offset = rdev->mc.gtt_start;
man->available_caching = TTM_PL_MASK_CACHING;
man->default_caching = TTM_PL_FLAG_CACHED;
man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -109,7 +108,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _bo_manager_func;
- man->gpu_offset = rdev->mc.vram_start;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[RFC PATCH 6/6] drm/ttm: do not keep GPU dependent addresses
GPU address handling is device specific and should be handle by its device
driver.
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/ttm/ttm_bo.c| 7 ---
include/drm/ttm/ttm_bo_api.h| 2 --
include/drm/ttm/ttm_bo_driver.h | 1 -
3 files changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 229205e499db..2ccfebc3c9a2 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -85,7 +85,6 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev,
struct drm_printer *p
drm_printf(p, "has_type: %d\n", man->has_type);
drm_printf(p, "use_type: %d\n", man->use_type);
drm_printf(p, "flags: 0x%08X\n", man->flags);
- drm_printf(p, "gpu_offset: 0x%08llX\n", man->gpu_offset);
drm_printf(p, "size: %llu\n", man->size);
drm_printf(p, "available_caching: 0x%08X\n",
man->available_caching);
drm_printf(p, "default_caching: 0x%08X\n", man->default_caching);
@@ -382,12 +381,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
bo->evicted = false;
}
- if (bo->mem.mm_node)
- bo->offset = (bo->mem.start << PAGE_SHIFT) +
- bdev->man[bo->mem.mem_type].gpu_offset;
- else
- bo->offset = 0;
-
ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
return 0;
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 65e399d280f7..3cf8bb82c899 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -219,8 +219,6 @@ struct ttm_buffer_object {
* either of these locks held.
*/
- uint64_t offset; /* GPU address space is independent of CPU word size */
-
struct sg_table *sg;
struct mutex wu_mutex;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index cac7a8a0825a..302b0aaf8d13 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -177,7 +177,6 @@ struct ttm_mem_type_manager {
bool has_type;
bool use_type;
uint32_t flags;
- uint64_t gpu_offset; /* GPU address space is independent of CPU word
size */
uint64_t size;
uint32_t available_caching;
uint32_t default_caching;
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[RFC PATCH 0/6] do not store GPU address in TTM
With this patch series I am trying to remove GPU address dependency in TTM and moving GPU address calculation to individual drm drivers. This is required[1] to continue the work started by Brian Welty to create struct drm_mem_region which can be leverage by DRM cgroup controller to manage memory limits. I have only manage to test amdgpu driver as I only have GPU for that. I might be doing something really stupid while calculeting gpu offset for some of the drivers so please be patient and let me know how can I improve that. [1] https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg272238.html Nirmoy Das (6): drm/amdgpu: move ttm bo->offset to amdgpu_bo drm/radeon: don't use ttm bo->offset drm/vmwgfx: don't use ttm bo->offset drm/nouveau: don't use ttm bo->offset drm/qxl: don't use ttm bo->offset drm/ttm: do not keep GPU dependent addresses drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 29 - drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 4 +-- drivers/gpu/drm/nouveau/dispnv04/crtc.c | 6 ++--- drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 6 ++--- drivers/gpu/drm/nouveau/dispnv50/base507c.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/core507d.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 2 +- drivers/gpu/drm/nouveau/nouveau_abi16.c | 8 +++--- drivers/gpu/drm/nouveau/nouveau_bo.c| 1 + drivers/gpu/drm/nouveau/nouveau_bo.h| 3 +++ drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +++ drivers/gpu/drm/qxl/qxl_drv.h | 6 ++--- drivers/gpu/drm/qxl/qxl_kms.c | 3 +++ drivers/gpu/drm/qxl/qxl_object.h| 5 drivers/gpu/drm/qxl/qxl_ttm.c | 9 --- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_object.h | 16 +++- drivers/gpu/drm/radeon/radeon_ttm.c | 4 +-- drivers/gpu/drm/ttm/ttm_bo.c| 7 - drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c| 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 -- include/drm/ttm/ttm_bo_api.h| 2 -- include/drm/ttm/ttm_bo_driver.h | 1 - 33 files changed, 99 insertions(+), 72 deletions(-) -- 2.25.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/qxl/qxl_drv.h| 6 ++
drivers/gpu/drm/qxl/qxl_kms.c| 3 +++
drivers/gpu/drm/qxl/qxl_object.h | 5 -
drivers/gpu/drm/qxl/qxl_ttm.c| 9 -
4 files changed, 5 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 27e45a2d6b52..9a76a2a0283d 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct
qxl_bo *bo,
(bo->tbo.mem.mem_type == TTM_PL_VRAM)
? >main_slot : >surfaces_slot;
- WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset);
-
- /* TODO - need to hold one of the locks to read tbo.offset */
- return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset);
+ return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) +
+ slot->gpu_offset + offset);
}
/* qxl_display.c */
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 611cbe7aee69..937cac9ba384 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -71,11 +71,14 @@ static void setup_slot(struct qxl_device *qdev,
unsigned long size)
{
uint64_t high_bits;
+ unsigned int gpu_offset_shift =
+ 64 - (qdev->rom->slot_gen_bits + qdev->rom->slot_id_bits + 8);
slot->index = slot_index;
slot->name = slot_name;
slot->start_phys_addr = start_phys_addr;
slot->size = size;
+ slot->gpu_offset = (uint64_t)slot_index << gpu_offset_shift;
setup_hw_slot(qdev, slot);
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 8ae54ba7857c..21fa81048f4f 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -48,11 +48,6 @@ static inline void qxl_bo_unreserve(struct qxl_bo *bo)
ttm_bo_unreserve(>tbo);
}
-static inline u64 qxl_bo_gpu_offset(struct qxl_bo *bo)
-{
- return bo->tbo.offset;
-}
-
static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
{
return bo->tbo.num_pages << PAGE_SHIFT;
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 16a5e903533d..2a43d0ef9ba1 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -56,11 +56,6 @@ static int qxl_invalidate_caches(struct ttm_bo_device *bdev,
uint32_t flags)
static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
struct ttm_mem_type_manager *man)
{
- struct qxl_device *qdev = qxl_get_qdev(bdev);
- unsigned int gpu_offset_shift =
- 64 - (qdev->rom->slot_gen_bits + qdev->rom->slot_id_bits + 8);
- struct qxl_memslot *slot;
-
switch (type) {
case TTM_PL_SYSTEM:
/* System memory */
@@ -71,11 +66,7 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_VRAM:
case TTM_PL_PRIV:
/* "On-card" video ram */
- slot = (type == TTM_PL_VRAM) ?
- >main_slot : >surfaces_slot;
- slot->gpu_offset = (uint64_t)type << gpu_offset_shift;
man->func = _bo_manager_func;
- man->gpu_offset = slot->gpu_offset;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_MASK_CACHING;
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[RFC PATCH 4/6] drm/nouveau: don't use ttm bo->offset
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 6 +++---
drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
drivers/gpu/drm/nouveau/dispnv04/overlay.c | 6 +++---
drivers/gpu/drm/nouveau/dispnv50/base507c.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/core507d.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_abi16.c | 8
drivers/gpu/drm/nouveau/nouveau_bo.c| 1 +
drivers/gpu/drm/nouveau/nouveau_bo.h| 3 +++
drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +-
14 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 37c50ea8f847..18a06cf03fa1 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -845,7 +845,7 @@ nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
fb = nouveau_framebuffer(crtc->primary->fb);
}
- nv_crtc->fb.offset = fb->nvbo->bo.offset;
+ nv_crtc->fb.offset = fb->nvbo->offset;
if (nv_crtc->lut.depth != drm_fb->format->depth) {
nv_crtc->lut.depth = drm_fb->format->depth;
@@ -1013,7 +1013,7 @@ nv04_crtc_cursor_set(struct drm_crtc *crtc, struct
drm_file *file_priv,
nv04_cursor_upload(dev, cursor, nv_crtc->cursor.nvbo);
nouveau_bo_unmap(cursor);
- nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->bo.offset;
+ nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->offset;
nv_crtc->cursor.set_offset(nv_crtc, nv_crtc->cursor.offset);
nv_crtc->cursor.show(nv_crtc, true);
out:
@@ -1191,7 +1191,7 @@ nv04_crtc_page_flip(struct drm_crtc *crtc, struct
drm_framebuffer *fb,
/* Initialize a page flip struct */
*s = (struct nv04_page_flip_state)
{ { }, event, crtc, fb->format->cpp[0] * 8, fb->pitches[0],
- new_bo->bo.offset };
+ new_bo->offset };
/* Keep vblanks on during flip, for the target crtc of this flip */
drm_crtc_vblank_get(crtc);
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c
b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index 44ee82d0c9b6..89a4ddfcc55f 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -151,7 +151,7 @@ nv04_display_init(struct drm_device *dev, bool resume, bool
runtime)
continue;
if (nv_crtc->cursor.set_offset)
- nv_crtc->cursor.set_offset(nv_crtc,
nv_crtc->cursor.nvbo->bo.offset);
+ nv_crtc->cursor.set_offset(nv_crtc,
nv_crtc->cursor.nvbo->offset);
nv_crtc->cursor.set_pos(nv_crtc, nv_crtc->cursor_saved_x,
nv_crtc->cursor_saved_y);
}
diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
index a3a0a73ae8ab..9529bd9053e7 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
@@ -150,7 +150,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
nvif_mask(dev, NV_PCRTC_ENGINE_CTRL + soff2, NV_CRTC_FSEL_OVERLAY, 0);
nvif_wr32(dev, NV_PVIDEO_BASE(flip), 0);
- nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->bo.offset);
+ nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->offset);
nvif_wr32(dev, NV_PVIDEO_SIZE_IN(flip), src_h << 16 | src_w);
nvif_wr32(dev, NV_PVIDEO_POINT_IN(flip), src_y << 16 | src_x);
nvif_wr32(dev, NV_PVIDEO_DS_DX(flip), (src_w << 20) / crtc_w);
@@ -172,7 +172,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
if (format & NV_PVIDEO_FORMAT_PLANAR) {
nvif_wr32(dev, NV_PVIDEO_UVPLANE_BASE(flip), 0);
nvif_wr32(dev, NV_PVIDEO_UVPLANE_OFFSET_BUFF(flip),
- nv_fb->nvbo->bo.offset + fb->offsets[1]);
+ nv_fb->nvbo->offset + fb->offsets[1]);
}
nvif_wr32(dev, NV_PVIDEO_FORMAT(flip), format | fb->pitches[0]);
nvif_wr32(dev, NV_PVIDEO_STOP, 0);
@@ -396,7 +396,7 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
for (i = 0; i < 2; i++) {
nvif_wr32(dev, NV_PVIDEO_BUFF0_START_ADDRESS + 4 * i,
- nv_fb->nvbo->bo.offset);
+ nv_fb->nvbo->offset);
nvif_wr32(dev, NV_PVIDEO_BUFF0_PITCH_LENGTH + 4 * i,
fb->pitches[0]);
nvif_wr32(dev, NV_PVIDEO_BUFF0_OFFSET + 4 * i, 0);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/base507c.c
[RFC PATCH 1/6] drm/amdgpu: move ttm bo->offset to amdgpu_bo
GPU address should belong to driver not in memory management.
This patch moves ttm bo.offset and gpu_offset calculation to amdgpu driver.
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 29 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 4 +--
5 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 6f60a581e3ba..1b1c393587a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -917,7 +917,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32
domain,
bo->pin_count++;
if (max_offset != 0) {
- u64 domain_start =
bo->tbo.bdev->man[mem_type].gpu_offset;
+ u64 domain_start = amdgpu_ttm_domain_start(adev,
mem_type);
WARN_ON_ONCE(max_offset <
(amdgpu_bo_gpu_offset(bo) - domain_start));
}
@@ -1467,7 +1467,25 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_VRAM &&
!(bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS));
- return amdgpu_gmc_sign_extend(bo->tbo.offset);
+ return amdgpu_bo_gpu_offset_no_check(bo);
+}
+
+/**
+ * amdgpu_bo_gpu_offset_no_check - return GPU offset of bo
+ * @bo:amdgpu object for which we query the offset
+ *
+ * Returns:
+ * current GPU offset of the object without raising warnings.
+ */
+u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo)
+{
+ struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+ uint64_t offset;
+
+offset = (bo->tbo.mem.start << PAGE_SHIFT) +
+amdgpu_ttm_domain_start(adev, bo->tbo.mem.mem_type);
+
+ return amdgpu_gmc_sign_extend(offset);
}
/**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 96d805889e8d..9075ef20ce02 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -288,6 +288,7 @@ int amdgpu_bo_sync_wait_resv(struct amdgpu_device *adev,
struct dma_resv *resv,
bool intr);
int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void *owner, bool intr);
u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo);
+u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
int amdgpu_bo_validate(struct amdgpu_bo *bo);
int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
struct dma_fence **fence);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2c1d1eb1a7e1..4bb02d787945 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -103,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_TT:
/* GTT memory */
man->func = _gtt_mgr_func;
- man->gpu_offset = adev->gmc.gart_start;
man->available_caching = TTM_PL_MASK_CACHING;
man->default_caching = TTM_PL_FLAG_CACHED;
man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -111,7 +110,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _vram_mgr_func;
- man->gpu_offset = adev->gmc.vram_start;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
@@ -122,7 +120,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case AMDGPU_PL_OA:
/* On-chip GDS memory*/
man->func = _bo_manager_func;
- man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_CMA;
man->available_caching = TTM_PL_FLAG_UNCACHED;
man->default_caching = TTM_PL_FLAG_UNCACHED;
@@ -270,7 +267,7 @@ static uint64_t amdgpu_mm_node_addr(struct
ttm_buffer_object *bo,
if (mm_node->start != AMDGPU_BO_INVALID_OFFSET) {
addr = mm_node->start << PAGE_SHIFT;
- addr += bo->bdev->man[mem->mem_type].gpu_offset;
+ addr += amdgpu_ttm_domain_start(amdgpu_ttm_adev(bo->bdev),
mem->mem_type);
}
return addr;
}
@@ -757,6 +754,27 @@ static unsigned long amdgpu_ttm_io_mem_pfn(struct
ttm_buffer_object *bo,
(offset >> PAGE_SHIFT);
}
+/**
+ * amdgpu_ttm_domain_start - Returns GPU start address
+ *
[RFC PATCH 3/6] drm/vmwgfx: don't use ttm bo->offset
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 --
4 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 74016a08d118..dd9fd609d37c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -258,7 +258,7 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private
*dev_priv,
ret = ttm_bo_validate(bo, , );
/* For some reason we didn't end up at the start of vram */
- WARN_ON(ret == 0 && bo->offset != 0);
+ WARN_ON(ret == 0 && (bo->mem.start << PAGE_SHIFT) != 0);
if (!ret)
vmw_bo_pin_reserved(buf, true);
@@ -317,7 +317,7 @@ void vmw_bo_get_guest_ptr(const struct ttm_buffer_object
*bo,
{
if (bo->mem.mem_type == TTM_PL_VRAM) {
ptr->gmrId = SVGA_GMR_FRAMEBUFFER;
- ptr->offset = bo->offset;
+ ptr->offset = bo->mem.start << PAGE_SHIFT;
} else {
ptr->gmrId = bo->mem.start;
ptr->offset = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index ff86d49dc5e8..e8a3351f35cf 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -3305,7 +3305,7 @@ static void vmw_apply_relocations(struct vmw_sw_context
*sw_context)
bo = >vbo->base;
switch (bo->mem.mem_type) {
case TTM_PL_VRAM:
- reloc->location->offset += bo->offset;
+ reloc->location->offset += bo->mem.start << PAGE_SHIFT;
reloc->location->gmrId = SVGA_GMR_FRAMEBUFFER;
break;
case VMW_PL_GMR:
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
index e5252ef3812f..1cdc445b24c3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
@@ -612,7 +612,7 @@ static int vmw_fifo_emit_dummy_legacy_query(struct
vmw_private *dev_priv,
if (bo->mem.mem_type == TTM_PL_VRAM) {
cmd->body.guestResult.gmrId = SVGA_GMR_FRAMEBUFFER;
- cmd->body.guestResult.offset = bo->offset;
+ cmd->body.guestResult.offset = bo->mem.start << PAGE_SHIFT;
} else {
cmd->body.guestResult.gmrId = bo->mem.start;
cmd->body.guestResult.offset = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index d8ea3dd10af0..1e69c013b47f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -755,7 +755,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _bo_manager_func;
- man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_CACHED;
man->default_caching = TTM_PL_FLAG_CACHED;
@@ -768,7 +767,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
* slots as well as the bo size.
*/
man->func = _gmrid_manager_func;
- man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_CMA | TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_CACHED;
man->default_caching = TTM_PL_FLAG_CACHED;
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/radeon/radeon.h| 1 +
drivers/gpu/drm/radeon/radeon_object.h | 16 +++-
drivers/gpu/drm/radeon/radeon_ttm.c| 4 +---
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index d59b004f6695..97cfcc2870af 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2823,6 +2823,7 @@ extern void radeon_ttm_set_active_vram_size(struct
radeon_device *rdev, u64 size
extern void radeon_program_register_sequence(struct radeon_device *rdev,
const u32 *registers,
const u32 array_size);
+struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev);
/*
* vm
diff --git a/drivers/gpu/drm/radeon/radeon_object.h
b/drivers/gpu/drm/radeon/radeon_object.h
index d23f2ed4126e..4d37571c7ff5 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo *bo)
*/
static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo)
{
- return bo->tbo.offset;
+ struct radeon_device *rdev;
+ u64 start = 0;
+
+ rdev = radeon_get_rdev(bo->tbo.bdev);
+
+ switch(bo->tbo.mem.mem_type) {
+ case TTM_PL_TT:
+ start = rdev->mc.gtt_start;
+ break;
+ case TTM_PL_VRAM:
+ start = rdev->mc.vram_start;
+ break;
+ }
+
+ return (bo->tbo.mem.start << PAGE_SHIFT) + start;
}
static inline unsigned long radeon_bo_size(struct radeon_bo *bo)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 098bc9f40b98..b10654494262 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -56,7 +56,7 @@
static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);
-static struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
+struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
{
struct radeon_mman *mman;
struct radeon_device *rdev;
@@ -87,7 +87,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
break;
case TTM_PL_TT:
man->func = _bo_manager_func;
- man->gpu_offset = rdev->mc.gtt_start;
man->available_caching = TTM_PL_MASK_CACHING;
man->default_caching = TTM_PL_FLAG_CACHED;
man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -109,7 +108,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _bo_manager_func;
- man->gpu_offset = rdev->mc.vram_start;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/ttm: replace dma_resv object on deleted BOs v3
> 2020年2月13日 18:01，Koenig, Christian 写道：
>
> Am 13.02.20 um 05:11 schrieb Pan, Xinhui:
>>
>>
>>> 2020年2月12日 19:53，Christian König 写道：
>>>
>>> Am 12.02.20 um 07:23 schrieb Pan, Xinhui:
> 2020年2月11日 23:43，Christian König 写道：
>
> When non-imported BOs are resurrected for delayed delete we replace
> the dma_resv object to allow for easy reclaiming of the resources.
>
> v2: move that to ttm_bo_individualize_resv
> v3: add a comment to explain what's going on
>
> Signed-off-by: Christian König
> Reviewed-by: xinhui pan
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index bfc42a9e4fb4..8174603d390f 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -393,6 +393,18 @@ static int ttm_bo_individualize_resv(struct
> ttm_buffer_object *bo)
>
> r = dma_resv_copy_fences(>base._resv, bo->base.resv);
> dma_resv_unlock(>base._resv);
> + if (r)
> + return r;
> +
> + if (bo->type != ttm_bo_type_sg) {
> + /* This works because the BO is about to be destroyed and nobody
> + * reference it any more. The only tricky case is the trylock on
> + * the resv object while holding the lru_lock.
> + */
> + spin_lock(_bo_glob.lru_lock);
> + bo->base.resv = >base._resv;
> + spin_unlock(_bo_glob.lru_lock);
> + }
>
how about something like that.
the basic idea is to do the bo cleanup work in bo release first and avoid
any race with evict.
As in bo dieing progress, evict also just do bo cleanup work.
If bo is busy, neither bo_release nor evict can do cleanupwork on it.
For the bo release case, we just add bo back to lru list.
So we can clean it up both in workqueue and shrinker as the past way did.
@@ -405,8 +405,9 @@ static int ttm_bo_individualize_resv(struct
ttm_buffer_object *bo)
if (bo->type != ttm_bo_type_sg) {
spin_lock(_bo_glob.lru_lock);
- bo->base.resv = >base._resv;
+ ttm_bo_del_from_lru(bo);
spin_unlock(_bo_glob.lru_lock);
+ bo->base.resv = >base._resv;
}
return r;
@@ -606,10 +607,9 @@ static void ttm_bo_release(struct kref *kref)
* shrinkers, now that they are queued for
* destruction.
*/
- if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
+ if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT)
bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT;
- ttm_bo_move_to_lru_tail(bo, NULL);
- }
+ ttm_bo_add_mem_to_lru(bo, >mem);
kref_init(>kref);
list_add_tail(>ddestroy, >ddestroy);
>>> Yeah, thought about that as well. But this has the major drawback that the
>>> deleted BO moves to the end of the LRU, which is something we don't want.
>> well, as the bo is busy, looks like it needs time to being idle. putting it
>> to tail seems fair.
>
> No, see BOs should move to the tail of the LRU whenever they are used.
> Freeing up a BO is basically the opposite of using it.
>
> So what would happen on the next memory contention is that the MM would evict
> BOs which are still used and only after come to the delete BO which could
> have been removed long ago.
>
>>> I think the real solution to this problem is to go a completely different
>>> way and remove the delayed delete feature from TTM altogether. Instead this
>>> should be part of some DRM domain handler component.
>>>
>> yes, completely agree. As long as we can shrink bos when OOM, the workqueue
>> is not necessary, The workqueue does not help in a heavy workload case.
>>
>> Pls see my patches below, I remove the workqueue, and what’s more, we can
>> clearup the bo without lru lock hold.
>> That would reduce the lock contention. I run kfdtest and got a good
>> performance result.
>
> No, that's an approach we had before as well and it also doesn't work that
> well.
>
> See the problem is that we can only remove the BO from the LRU after it has
> released the memory it references. Otherwise we run into the issue that some
> threads can't wait for the memory to be freed any more and run into an OOM
> situation.
>
ok, we can keep the workqueue at it is.
Now I come up with another idea that evict and swap can touch the destroy list
first, then lru list.
Looks like putting a dieing bo in lru list is useless.
thanks
xinhui
> Regards,
> Christian.
>
>>
>>
>>> In other words it should not matter if a BO is evicted, moved or freed.
>>> Whenever a piece of memory becomes available again we keep around a fence
>>> which marks the end of using this piece of memory.
>>>
>>> When then
Re: [PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)
Am 13.02.20 um 01:54 schrieb Luben Tuikov:
Move from a per-CS secure flag (TMZ) to a per-IB
secure flag.
Well that seems to make a lot of sense to me, but need to bit of time to
read into the PM4 handling of TMZ.
Especially what is the "start" parameter good for?
Regards,
Christian.
Signed-off-by: Luben Tuikov
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 --
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 -
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 23 +++
drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c| 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c| 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 3 +--
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 20 ++--
include/uapi/drm/amdgpu_drm.h| 7 ---
10 files changed, 44 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 80ba6dfc54e2..f9fa6e104fef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, union drm_amdgpu_cs
if (ret)
goto free_all_kdata;
- p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
-
if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
ret = -ECANCELED;
goto free_all_kdata;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 6e0f97afb030..cae81914c821 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
num_ibs,
uint64_t fence_ctx;
uint32_t status = 0, alloc_size;
unsigned fence_flags = 0;
+ bool secure;
unsigned i;
int r = 0;
@@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
num_ibs,
if (job && ring->funcs->emit_cntxcntl) {
status |= job->preamble_status;
status |= job->preemption_status;
- amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
+ amdgpu_ring_emit_cntxcntl(ring, status);
}
+ secure = false;
for (i = 0; i < num_ibs; ++i) {
ib = [i];
@@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
!amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble
CE ib must be inserted anyway */
continue;
+ /* If this IB is TMZ, add frame TMZ start packet,
+* else, turn off TMZ.
+*/
+ if (ib->flags & AMDGPU_IB_FLAGS_SECURE &&
ring->funcs->emit_tmz) {
+ if (!secure) {
+ secure = true;
+ amdgpu_ring_emit_tmz(ring, true);
+ }
+ } else if (secure) {
+ secure = false;
+ amdgpu_ring_emit_tmz(ring, false);
+ }
+
amdgpu_ring_emit_ib(ring, job, ib, status);
status &= ~AMDGPU_HAVE_CTX_SWITCH;
}
- if (ring->funcs->emit_tmz)
- amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
+ if (secure) {
+ secure = false;
+ amdgpu_ring_emit_tmz(ring, false);
+ }
#ifdef CONFIG_X86_64
if (!(adev->flags & AMD_IS_APU))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 31cb0203..2e2110dddb76 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -61,9 +61,6 @@ struct amdgpu_job {
/* user fence handling */
uint64_tuf_addr;
uint64_tuf_sequence;
-
- /* the job is due to a secure command submission */
- boolsecure;
};
int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 5134d0dd6dc2..930316e60155 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -158,8 +158,7 @@ struct amdgpu_ring_funcs {
void (*begin_use)(struct amdgpu_ring *ring);
void (*end_use)(struct amdgpu_ring *ring);
void (*emit_switch_buffer) (struct amdgpu_ring *ring);
- void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags,
- bool trusted);
+ void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
void (*emit_wreg)(struct amdgpu_ring
Re: [PATCH 14/15] drm/amdgpu/ring: move debugfs init into core amdgpu debugfs
Am 13.02.20 um 10:54 schrieb Daniel Vetter:
On Fri, Feb 07, 2020 at 02:50:57PM -0500, Alex Deucher wrote:
In order to remove the load and unload drm callbacks,
we need to reorder the init sequence to move all the drm
debugfs file handling. Do this for rings.
Acked-by: Christian König
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 23 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c| 15 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h| 4
3 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index df3919ef886b..7379910790c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1218,7 +1218,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
int amdgpu_debugfs_init(struct amdgpu_device *adev)
{
- int r;
+ int r, i;
adev->debugfs_preempt =
debugfs_create_file("amdgpu_preempt_ib", 0600,
@@ -1268,12 +1268,33 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
}
#endif
+ for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
+ struct amdgpu_ring *ring = adev->rings[i];
+
+ if (!ring)
+ continue;
+
+ if (amdgpu_debugfs_ring_init(adev, ring)) {
+ DRM_ERROR("Failed to register debugfs file for rings
!\n");
+ }
+ }
+
return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list,
ARRAY_SIZE(amdgpu_debugfs_list));
}
void amdgpu_debugfs_fini(struct amdgpu_device *adev)
btw debugfs_fini shouldn't be needed anymore, Greg KH removed this all.
drm core removes all debugfs files recusrively for you, there should be 0
need for debugfs cleanup.
Oh, yes please. Removing that was on my TODO list for an eternity as well.
Also at least my tree doesn't even have this, where does this apply to?
I would guess amd-staging-drm-next, but it might be that Alex is waiting
for the next rebase to land.
Christian.
-Daniel
{
+ int i;
+
+ for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
+ struct amdgpu_ring *ring = adev->rings[i];
+
+ if (!ring)
+ continue;
+
+ amdgpu_debugfs_ring_fini(ring);
+ }
amdgpu_ttm_debugfs_fini(adev);
debugfs_remove(adev->debugfs_preempt);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index e5c83e164d82..539be138260e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -48,9 +48,6 @@
* wptr. The GPU then starts fetching commands and executes
* them until the pointers are equal again.
*/
-static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
- struct amdgpu_ring *ring);
-static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring);
/**
* amdgpu_ring_alloc - allocate space on the ring buffer
@@ -334,10 +331,6 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct
amdgpu_ring *ring,
for (i = 0; i < DRM_SCHED_PRIORITY_MAX; ++i)
atomic_set(>num_jobs[i], 0);
- if (amdgpu_debugfs_ring_init(adev, ring)) {
- DRM_ERROR("Failed to register debugfs file for rings !\n");
- }
-
return 0;
}
@@ -367,8 +360,6 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring)
>gpu_addr,
(void **)>ring);
- amdgpu_debugfs_ring_fini(ring);
-
dma_fence_put(ring->vmid_wait);
ring->vmid_wait = NULL;
ring->me = 0;
@@ -485,8 +476,8 @@ static const struct file_operations
amdgpu_debugfs_ring_fops = {
#endif
-static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
- struct amdgpu_ring *ring)
+int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
+struct amdgpu_ring *ring)
{
#if defined(CONFIG_DEBUG_FS)
struct drm_minor *minor = adev->ddev->primary;
@@ -507,7 +498,7 @@ static int amdgpu_debugfs_ring_init(struct amdgpu_device
*adev,
return 0;
}
-static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring)
+void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring)
{
#if defined(CONFIG_DEBUG_FS)
debugfs_remove(ring->ent);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 5134d0dd6dc2..0d098dafd23c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -329,4 +329,8 @@ static inline void amdgpu_ring_write_multiple(struct
amdgpu_ring *ring,
int amdgpu_ring_test_helper(struct amdgpu_ring *ring);
+int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
+
Re: [PATCH] drm/ttm: replace dma_resv object on deleted BOs v3
Am 13.02.20 um 05:11 schrieb Pan, Xinhui:
2020年2月12日 19:53，Christian König 写道：
Am 12.02.20 um 07:23 schrieb Pan, Xinhui:
2020年2月11日 23:43，Christian König 写道：
When non-imported BOs are resurrected for delayed delete we replace
the dma_resv object to allow for easy reclaiming of the resources.
v2: move that to ttm_bo_individualize_resv
v3: add a comment to explain what's going on
Signed-off-by: Christian König
Reviewed-by: xinhui pan
---
drivers/gpu/drm/ttm/ttm_bo.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index bfc42a9e4fb4..8174603d390f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -393,6 +393,18 @@ static int ttm_bo_individualize_resv(struct
ttm_buffer_object *bo)
r = dma_resv_copy_fences(>base._resv, bo->base.resv);
dma_resv_unlock(>base._resv);
+ if (r)
+ return r;
+
+ if (bo->type != ttm_bo_type_sg) {
+ /* This works because the BO is about to be destroyed and nobody
+* reference it any more. The only tricky case is the trylock on
+* the resv object while holding the lru_lock.
+*/
+ spin_lock(_bo_glob.lru_lock);
+ bo->base.resv = >base._resv;
+ spin_unlock(_bo_glob.lru_lock);
+ }
how about something like that.
the basic idea is to do the bo cleanup work in bo release first and avoid any
race with evict.
As in bo dieing progress, evict also just do bo cleanup work.
If bo is busy, neither bo_release nor evict can do cleanupwork on it. For the
bo release case, we just add bo back to lru list.
So we can clean it up both in workqueue and shrinker as the past way did.
@@ -405,8 +405,9 @@ static int ttm_bo_individualize_resv(struct
ttm_buffer_object *bo)
if (bo->type != ttm_bo_type_sg) {
spin_lock(_bo_glob.lru_lock);
- bo->base.resv = >base._resv;
+ ttm_bo_del_from_lru(bo);
spin_unlock(_bo_glob.lru_lock);
+ bo->base.resv = >base._resv;
}
return r;
@@ -606,10 +607,9 @@ static void ttm_bo_release(struct kref *kref)
* shrinkers, now that they are queued for
* destruction.
*/
- if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
+ if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT)
bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT;
- ttm_bo_move_to_lru_tail(bo, NULL);
- }
+ ttm_bo_add_mem_to_lru(bo, >mem);
kref_init(>kref);
list_add_tail(>ddestroy, >ddestroy);
Yeah, thought about that as well. But this has the major drawback that the
deleted BO moves to the end of the LRU, which is something we don't want.
well, as the bo is busy, looks like it needs time to being idle. putting it to
tail seems fair.
No, see BOs should move to the tail of the LRU whenever they are used.
Freeing up a BO is basically the opposite of using it.
So what would happen on the next memory contention is that the MM would
evict BOs which are still used and only after come to the delete BO
which could have been removed long ago.
I think the real solution to this problem is to go a completely different way
and remove the delayed delete feature from TTM altogether. Instead this should
be part of some DRM domain handler component.
yes, completely agree. As long as we can shrink bos when OOM, the workqueue is
not necessary, The workqueue does not help in a heavy workload case.
Pls see my patches below, I remove the workqueue, and what’s more, we can
clearup the bo without lru lock hold.
That would reduce the lock contention. I run kfdtest and got a good performance
result.
No, that's an approach we had before as well and it also doesn't work
that well.
See the problem is that we can only remove the BO from the LRU after it
has released the memory it references. Otherwise we run into the issue
that some threads can't wait for the memory to be freed any more and run
into an OOM situation.
Regards,
Christian.
In other words it should not matter if a BO is evicted, moved or freed.
Whenever a piece of memory becomes available again we keep around a fence which
marks the end of using this piece of memory.
When then somebody asks for new memory we work through the LRU and test if
using a certain piece of memory makes sense or not. If we find that a BO needs
to be evicted for this we return a reference to the BO in question to the upper
level handling.
If we find that we can do the allocation but only with recently freed up memory
we gather the fences and say you can only use the newly allocated memory after
waiting for those.
HEY! Wait a second! Did I just outlined what a potential replacement to TTM
would look like?
yes, that is a good picture. Looks like we could do more work hen. :)
thanks
xinhui
--git a/drivers/gpu/drm/ttm/ttm_bo.c
