On 15.08.2014 16:11, Christian König wrote: > Hi Marco, > > the problem with an CS ioctl flag is that we sometimes don't know how > much SCLK/MCLK boost is needed, for example when we do post processing > in the player using OpenGL and UVD decoding with VDPAU. In this case > VDPAU don't has the slightest idea how high SCLK/MCLK must be and so > can't give that info to the kernel either. > Maybe it's an acceptable workaround to simply disable dynamic UVD state selection in case the UVD states only have a single power level. That will avoid the performance issues on affected systems, while still allowing dynamic UVD states on systems that have a saner DPM table setup. I think it is mosly older systems that suffer from this. Best regards Grigori > Regards, > Christian. > > Am 15.08.2014 um 15:21 schrieb Marco Benatto: >> Hey all, >> >> I also had a talk with Alex yesterday about post-processing issues >> when using dynamic UVD profiles and a chamge on CS ioctl >> including a flag to let user mode driver tell to the kernel which >> performance requirement it wants for post processing. A commom >> point for both discussion is to stablish the default values for these >> profiles, but probably this ioctl change would be more impacting/complex >> to implement than a sysfs entry. >> >> If a sysfs entry is anough for now I can handle the code to create it >> and, with your help, the code to setup the UVD profile requested >> through it. >> >> Is there any suggestion? >> >> Thanks all for your help, >> >> >> On Fri, Aug 15, 2014 at 5:48 AM, Christian König >> <christian.koenig@xxxxxxx <mailto:christian.koenig@xxxxxxx>> wrote: >> >> Hi guys, >> >> to make a long story short every time I watch a movie my laptop >> start to heat up because we always select the standard UVD power >> profile without actually measuring if that is necessary. >> >> Marco came up with a patch that seems to reliable measure the fps >> send down to the kernel and so together with knowing the frame >> size of the video should allow us to select the right UVD power >> profile. >> >> The problem is that Alex (unnoticed by me) completely disabled >> selecting the UVD profiles because of some issues with advanced >> post processing discussed on IRC. The problem seems to be that the >> lower UVD profiles have a to low SCLK/MCLK to handle the 3D load >> that comes with scaling, deinterlacing etc... >> >> I unfortunately don't have time for it, cause this only affects >> the hardware generations R600-SI and not the newest one CIK. So >> could you guys stick together and come up with a solution? >> Something like a sysfs entry that let's us select the minimum UVD >> power level allowed? >> >> I think Marco is happy to come up with a patch, we just need to >> know what's really needed and what should be the default values. >> I'm happy to review everything that comes out of it, just don't >> have time to do it myself. >> >> Happy discussion and thanks in advance, >> Christian. >> >> Am 12.08.2014 um 15:05 schrieb Alex Deucher: >> >> On Tue, Aug 12, 2014 at 6:00 AM, Christian König >> <deathsimple@xxxxxxxxxxx <mailto:deathsimple@xxxxxxxxxxx>> wrote: >> >> Am 11.08.2014 um 16:52 schrieb Alex Deucher: >> >> On Mon, Aug 11, 2014 at 5:08 AM, Christian König >> <deathsimple@xxxxxxxxxxx >> <mailto:deathsimple@xxxxxxxxxxx>> wrote: >> >> Am 07.08.2014 um 21:43 schrieb Alex Deucher: >> >> On Thu, Aug 7, 2014 at 11:32 AM, Christian König >> <deathsimple@xxxxxxxxxxx >> <mailto:deathsimple@xxxxxxxxxxx>> wrote: >> >> Am 07.08.2014 um 16:32 schrieb Alex Deucher: >> >> On Thu, Aug 7, 2014 at 7:33 AM, >> Christian König >> <deathsimple@xxxxxxxxxxx >> <mailto:deathsimple@xxxxxxxxxxx>> >> wrote: >> >> From: Marco A Benatto >> <marco.antonio.780@xxxxxxxxx >> <mailto:marco.antonio.780@xxxxxxxxx>> >> >> Adding a Frames Per Second >> estimation logic on UVD handles >> when it has being used. This >> estimation is per handle basis >> and will help on DPM profile >> calculation. >> >> v2 (chk): fix timestamp type, move >> functions around and >> cleanup code a bit. >> >> Will this really help much? I thought >> the problem was mainly due to >> sclk and mclk for post processing. >> >> >> It should at least handle the UVD side for >> upclocking when you get a >> lot >> of >> streams / fps. And at on my NI the patch >> seems to do exactly that. >> >> Switching sclk and mclk for post >> processing is a different task, and I >> actually have no idea what to do with them. >> >> At this point we always choose the plain UVD >> state anyway so this >> patch would only take effect if we re-enabled >> the dynamic UVD state >> selection. >> >> >> Hui? I thought we already re-enabled the dynamic >> UVD state selection, but >> double checking this I found it disabled again. >> >> What was the problem with that? Looks like I >> somehow missed the >> discussion >> around it. >> >> We did, but after doing so a number of people >> complained about a >> regression on IRC because when apps like xmbc enabled >> post processing, >> performance went down. >> >> >> That's strange, from my experience the different UVD >> performance states only >> affect UVDs dclk/vclk, not sclk/mclk. I need to get the >> DPM dumps to >> confirms this. >> >> The sclks and mclks are usually different as well, especially >> on APUs. >> I can send you some examples. >> >> You not off hand remember who complained on IRC? Finding >> something in the >> IRC logs is like searching for a needle in a haystack. >> >> I don't remember off hand. I think zgreg was involved in some >> of the >> discussions. >> >> Alex >> >> Thanks, >> Christian. >> >> >> Alex >> >> >> Christian. >> >> >> For the post processing, we probably need a >> hint we can >> pass to the driver in the CS ioctl to denote >> what state we need. >> Although if we did that, this could would >> largely be moot. That said, >> newer asics support dynamic UVD clocks so we >> really only need >> something like that for older asics and I >> guess VCE. >> >> Alex >> >> Christian. >> >> >> Alex >> >> Signed-off-by: Marco A Benatto >> <marco.antonio.780@xxxxxxxxx >> <mailto:marco.antonio.780@xxxxxxxxx>> >> Signed-off-by: Christian König >> <christian.koenig@xxxxxxx >> <mailto:christian.koenig@xxxxxxx>> >> --- >> >> drivers/gpu/drm/radeon/radeon.h >> | 10 ++++++ >> >> drivers/gpu/drm/radeon/radeon_uvd.c >> | 64 >> +++++++++++++++++++++++++++++++++---- >> 2 files changed, 68 >> insertions(+), 6 deletions(-) >> >> diff --git >> a/drivers/gpu/drm/radeon/radeon.h >> b/drivers/gpu/drm/radeon/radeon.h >> index 9e1732e..e92f6cb 100644 >> --- a/drivers/gpu/drm/radeon/radeon.h >> +++ b/drivers/gpu/drm/radeon/radeon.h >> @@ -1617,6 +1617,15 @@ int >> radeon_pm_get_type_index(struct >> radeon_device >> *rdev, >> #define >> RADEON_UVD_STACK_SIZE (1024*1024) >> #define RADEON_UVD_HEAP_SIZE >> (1024*1024) >> >> +#define RADEON_UVD_FPS_EVENTS_MAX 8 >> +#define RADEON_UVD_DEFAULT_FPS 60 >> + >> +struct radeon_uvd_fps { >> + uint64_t timestamp; >> + uint8_t event_index; >> + uint8_t >> events[RADEON_UVD_FPS_EVENTS_MAX]; >> +}; >> + >> struct radeon_uvd { >> struct radeon_bo >> *vcpu_bo; >> void >> *cpu_addr; >> @@ -1626,6 +1635,7 @@ struct >> radeon_uvd { >> struct drm_file >> *filp[RADEON_MAX_UVD_HANDLES]; >> unsigned >> img_size[RADEON_MAX_UVD_HANDLES]; >> struct delayed_work >> idle_work; >> + struct radeon_uvd_fps >> fps_info[RADEON_MAX_UVD_HANDLES]; >> }; >> >> int radeon_uvd_init(struct >> radeon_device *rdev); >> diff --git >> a/drivers/gpu/drm/radeon/radeon_uvd.c >> b/drivers/gpu/drm/radeon/radeon_uvd.c >> index 6bf55ec..ef5667a 100644 >> --- >> a/drivers/gpu/drm/radeon/radeon_uvd.c >> +++ >> b/drivers/gpu/drm/radeon/radeon_uvd.c >> @@ -237,6 +237,51 @@ void >> radeon_uvd_force_into_uvd_segment(struct >> radeon_bo *rbo) >> rbo->placement.lpfn = >> (256 * 1024 * 1024) >> PAGE_SHIFT; >> } >> >> +static void >> radeon_uvd_fps_clear_events(struct >> radeon_device *rdev, >> int >> idx) >> +{ >> + struct radeon_uvd_fps *fps >> = &rdev->uvd.fps_info[idx]; >> + unsigned i; >> + >> + fps->timestamp = jiffies_64; >> + fps->event_index = 0; >> + for (i = 0; i < >> RADEON_UVD_FPS_EVENTS_MAX; i++) >> + fps->events[i] = 0; >> +} >> + >> +static void >> radeon_uvd_fps_note_event(struct >> radeon_device *rdev, >> int >> idx) >> +{ >> + struct radeon_uvd_fps *fps >> = &rdev->uvd.fps_info[idx]; >> + uint64_t timestamp = >> jiffies_64; >> + unsigned rate = 0; >> + >> + uint8_t index = >> fps->event_index++; >> + fps->event_index %= >> RADEON_UVD_FPS_EVENTS_MAX; >> + >> + rate = div64_u64(HZ, >> max(timestamp - fps->timestamp, >> 1ULL)); >> + >> + fps->timestamp = timestamp; >> + fps->events[index] = >> min(rate, 120u); >> +} >> + >> +static unsigned >> radeon_uvd_estimate_fps(struct >> radeon_device *rdev, >> int >> idx) >> +{ >> + struct radeon_uvd_fps *fps >> = &rdev->uvd.fps_info[idx]; >> + unsigned i, valid = 0, >> count = 0; >> + >> + for (i = 0; i < >> RADEON_UVD_FPS_EVENTS_MAX; i++) { >> + /* We should >> ignore zero values */ >> + if (fps->events[i] >> != 0) { >> + count += >> fps->events[i]; >> + valid++; >> + } >> + } >> + >> + if (valid > 0) >> + return count / valid; >> + else >> + return >> RADEON_UVD_DEFAULT_FPS; >> +} >> + >> void >> radeon_uvd_free_handles(struct >> radeon_device *rdev, struct >> drm_file *filp) >> { >> int i, r; >> @@ -419,8 +464,10 @@ static int >> radeon_uvd_cs_msg(struct >> radeon_cs_parser >> *p, struct radeon_bo *bo, >> >> /* create or decode, >> validate the handle */ >> for (i = 0; i < >> RADEON_MAX_UVD_HANDLES; ++i) { >> - if >> (atomic_read(&p->rdev->uvd.handles[i]) >> == handle) >> + if >> (atomic_read(&p->rdev->uvd.handles[i]) >> == handle) >> { >> + >> radeon_uvd_fps_note_event(p->rdev, i); >> return 0; >> + } >> } >> >> /* handle not found >> try to alloc a new one */ >> @@ -428,6 +475,7 @@ static int >> radeon_uvd_cs_msg(struct >> radeon_cs_parser >> *p, struct radeon_bo *bo, >> if >> (!atomic_cmpxchg(&p->rdev->uvd.handles[i], >> 0, >> handle)) { >> >> p->rdev->uvd.filp[i] = p->filp; >> >> p->rdev->uvd.img_size[i] = img_size; >> + >> radeon_uvd_fps_clear_events(p->rdev, >> i); >> return 0; >> } >> } >> @@ -763,7 +811,7 @@ int >> radeon_uvd_get_destroy_msg(struct >> radeon_device >> *rdev, int ring, >> static void >> radeon_uvd_count_handles(struct >> radeon_device *rdev, >> >> unsigned *sd, unsigned *hd) >> { >> - unsigned i; >> + unsigned i, fps_rate = 0; >> >> *sd = 0; >> *hd = 0; >> @@ -772,10 +820,13 @@ static void >> radeon_uvd_count_handles(struct >> radeon_device *rdev, >> if >> (!atomic_read(&rdev->uvd.handles[i])) >> continue; >> >> - if >> (rdev->uvd.img_size[i] >= 720*576) >> - ++(*hd); >> - else >> - ++(*sd); >> + fps_rate = >> radeon_uvd_estimate_fps(rdev, i); >> + >> + if >> (rdev->uvd.img_size[i] >= 720*576) { >> + (*hd) += >> fps_rate > 30 ? 1 : 2; >> + } else { >> + (*sd) += >> fps_rate > 30 ? 1 : 2; >> + } >> } >> } >> >> @@ -805,6 +856,7 @@ void >> radeon_uvd_note_usage(struct >> radeon_device >> *rdev) >> set_clocks &= >> schedule_delayed_work(&rdev->uvd.idle_work, >> >> msecs_to_jiffies(UVD_IDLE_TIMEOUT_MS)); >> >> + >> if >> ((rdev->pm.pm_method == >> PM_METHOD_DPM) && >> rdev->pm.dpm_enabled) { >> unsigned hd = >> 0, sd = 0; >> >> radeon_uvd_count_handles(rdev, >> &sd, &hd); >> -- >> 1.9.1 >> >> >> >> >> >> -- >> Marco Antonio Benatto >> Linux user ID:#506236 >
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
