[PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
lanes) and the LCD (CRTC),
> and there is no macro to handle such modules.
Ah, yes, my bad. I thought you were registering a device and a
driver. Still this is a very unusual pattern. Why do you need to split
the two? Can't you just merge them?
> > > > > +int de2_plane_init(struct drm_device *drm, struct lcd *lcd)
> > > > > +{
> > > > > + int ret, possible_crtcs = 1 << lcd->crtc_idx;
> > > > > +
> > > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_PRIMARY_PLANE],
> > > > > + DRM_PLANE_TYPE_PRIMARY, possible_crtcs,
> > > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > > > > + if (ret >= 0)
> > > > > + ret = de2_one_plane_init(drm,
> > > > > &lcd->planes[DE2_CURSOR_PLANE],
> > > > > + DRM_PLANE_TYPE_CURSOR, possible_crtcs,
> > > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > > >
> > > > Nothing looks really special about that cursor plane. Any reasion not
> > > > to make it an overlay?
> > >
> > > As explained above (channel/layer/pipe plane definitions), the cursor
> > > cannot go in a channel lower or equal to the one of the primary plane.
> > > Then, it must be known and, so, have an explicit plane.
> >
> > If you were to make it a plane, you could use atomic_check to check
> > this and make sure this doesn't happen. And you would gain a generic
> > plane that can be used for other purposes if needed.
>
> The function drm_crtc_init_with_planes() offers a cursor plane for free.
> On the other side, having 6 overlay planes is more than the SoCs can
> support.
It's not really for free, it costs you a generic plane that could
definitely be used for something else and cannot anymore because
they've been hardcoded to a cursor.
And having a camera, the VPU or even an application directly output
directly into one of these planes seems a much better use of a generic
plane than a cursor.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/1138769a/attachment-0001.sig>
[Bug 98634] Fedora 32bits kernel 4.8.4-200.fc24.i686+PAE is not able to resume from hibernate
https://bugs.freedesktop.org/show_bug.cgi?id=98634 Bug ID: 98634 Summary: Fedora 32bits kernel 4.8.4-200.fc24.i686+PAE is not able to resume from hibernate Product: DRI Version: XOrg git Hardware: x86 (IA32) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: DRM/other Assignee: dri-devel at lists.freedesktop.org Reporter: humberto.i.perez.rodriguez at intel.com Created attachment 127824 --> https://bugs.freedesktop.org/attachment.cgi?id=127824&action=edit dmesg.log Bug description === Fedora 32bits kernel 4.8.4-200.fc24.i686+PAE is not able to resume from hibernate, after send the platform to S4 / hibernate looks like that the platforms is not able to enter to S4 because the post code remains in "0004" and never shutdown completly how does it happen in Fedora 64bits Note : this bug does not appears in Fedora 64bit with kernel 4.5.5-300-fc24.x86_64 Steps to reproduce === 1. Install Fedora 24 32bits 2. Go to hibernate with either of the following commands : # rtcwake -s 60 -m disk # echo disk > /sys/power/state 3. if you try with the second command try to wake up the platform pushing any key / if you try with the first command wait 60 seconds Expected behavior === The platforms must resume from S4 / hibernate Actual result === After the step 3 the platform does not resume from S4 / hibernate Software configuration Linux OS: Fedora 32Bit Kernel version : 4.8.4-200.fc24.i686+PAE Bios revision : 144.10 Bios release date : 06/27/2016 KSC revision: 1.15 Hardware information Platform: BXT-P Motherboard model : BroxtonP Motherboard type: NOTEBOOK Hand Held Motherboard manufacturer: IntelCorp. CPU family : Other CPU information : 06/5c GPU Card: Intel Corporation Atom/Celeron/Pentium Processor N4200/N3350/E3900 Series Integrated Graphics Controller (rev 0a) (prog-if 00 [VGA controller]) Memory ram : 8 GB Maximum memory ram allowed : 16 GB CPU thread : 4 CPU core: 4 Attachments === dmesg.log, which containg the log when the platforms try to enter to S4 state Additional information === in order to send the platform to S3 (suspend) / S4 (hibernate) you have to enable it in the grub in the following way : 1. $ dmesg | grep -i "swap on" # to know where is your swap partition example : /dev/mapper/fedora-swap 2. add the following parameter in the file /etc/default/grub resume=/dev/mapper/fedora-swap add it at the end of the line : GRUB_CMDLINE_LINUX= 3. update the grub : grub2-mkconfig -o /boot/grub2/grub.cfg this a common issue in Fedora, for more detail information please read the following article : https://fedoraproject.org/wiki/Common_F24_bugs#Hibernation_doesn.27t_work_from_a_standard_install Please correct me if i am placing this issue in the wrong component, thanks i915_error_state = no error state collected cat /sys/class/drm/card0/error = no error state collected -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/a1ac7281/attachment.html>
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
Regards Shashank On 11/7/2016 8:56 PM, Emil Velikov wrote: > On 7 November 2016 at 07:43, Sharma, Shashank > wrote: >> If I was not very clear for the first time, every time we send a patch to >> drm-intel/dri-devel, we do basic testing on Gnome-desktop too (Not only >> Android). >> >> So even these aspect ratio patches were tested with full gnome-desktop, and >> it worked well. >> >> > I guess the part that's not so obvious is that Linux > desktop/distributions provide a very wide permutation of components > and configs used. While on a local (test) setup only a small/limited > set is available. Esp. on Android where things are _very_ tightly > coupled. I agree, Emil. I was only mentioning my testing with Gnome, to confirm that intel-ddx is not broken with Gnome desktop. And I was testing on both Android as well as X. > Obviously nobody likes when we have to carry kernel patches which > workaround "broken" userspace, but it's a kernel policy and we all > have to live with it. That's the main reason people are so > careful/pedantic when it comes to UABI. And as you can see even then > there are bits that we miss :'-( I agree, again. But I was thinking if reverting the patch was the best way, else it would be impossible to add something new in the kernel. I hope experts like you and others can suggest the right way. > > Regards, > Emil
[Intel-gfx] [PATCH] drm/dp: Make space for null terminator in the DP device ID char array
On Fri, 04 Nov 2016, Dhinakaran Pandiyan wrote: > The DP device identification string read from the DPCD registers is 6 > characters long at max. and we store it in a char array of the same length > without space for the NULL terminator. Fix this by increasing the array > size to 7 and initialize it to an empty string. > > Signed-off-by: Dhinakaran Pandiyan > --- > drivers/gpu/drm/drm_dp_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 3e6fe82..3a39312 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -544,7 +544,7 @@ void drm_dp_downstream_debug(struct seq_file *m, >DP_DETAILED_CAP_INFO_AVAILABLE; > int clk; > int bpc; > - char id[6]; > + char id[7] = ""; Or use %*pE in the format string and provide the length. BR, Jani. > int len; > uint8_t rev[2]; > int type = port_cap[0] & DP_DS_PORT_TYPE_MASK; -- Jani Nikula, Intel Open Source Technology Center
[PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe
On 11/7/2016 8:18 PM, Rob Clark wrote:
> On Mon, Nov 7, 2016 at 5:38 AM, Archit Taneja
> wrote:
>>
>>
>> On 11/05/2016 09:55 PM, Rob Clark wrote:
>>>
>>> Split out the hardware pipe specifics from mdp5_plane. To start, the hw
>>> pipes are statically assigned to planes, but next step is to assign the
>>> hw pipes during plane->atomic_check() based on requested caps (scaling,
>>> YUV, etc). And then hw pipe re-assignment if required if required SMP
>>> blocks changes.
>>>
>>> Signed-off-by: Rob Clark
>>> ---
>>> drivers/gpu/drm/msm/Makefile | 1 +
>>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 126
>>> +++---
>>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 7 +-
>>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c | 43 ++
>>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h | 39 +
>>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 66 +++-
>>> 6 files changed, 197 insertions(+), 85 deletions(-)
>>> create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
>>> create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
>>>
>>> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
>>> index fb5be3e..90f66c4 100644
>>> --- a/drivers/gpu/drm/msm/Makefile
>>> +++ b/drivers/gpu/drm/msm/Makefile
>>> @@ -37,6 +37,7 @@ msm-y := \
>>> mdp/mdp5/mdp5_irq.o \
>>> mdp/mdp5/mdp5_mdss.o \
>>> mdp/mdp5/mdp5_kms.o \
>>> + mdp/mdp5/mdp5_pipe.o \
>>> mdp/mdp5/mdp5_plane.o \
>>> mdp/mdp5/mdp5_smp.o \
>>> msm_atomic.o \
>>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>>> index f1288c7..d3d45ed 100644
>>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>>> @@ -119,6 +119,10 @@ static void mdp5_kms_destroy(struct msm_kms *kms)
>>> {
>>> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
>>> struct msm_gem_address_space *aspace = mdp5_kms->aspace;
>>> + int i;
>>> +
>>> + for (i = 0; i < mdp5_kms->num_hwpipes; i++)
>>> + mdp5_pipe_destroy(mdp5_kms->hwpipes[i]);
>>>
>>> if (aspace) {
>>> aspace->mmu->funcs->detach(aspace->mmu,
>>> @@ -323,15 +327,6 @@ static int modeset_init_intf(struct mdp5_kms
>>> *mdp5_kms, int intf_num)
>>>
>>> static int modeset_init(struct mdp5_kms *mdp5_kms)
>>> {
>>> - static const enum mdp5_pipe rgb_planes[] = {
>>> - SSPP_RGB0, SSPP_RGB1, SSPP_RGB2, SSPP_RGB3,
>>> - };
>>> - static const enum mdp5_pipe vig_planes[] = {
>>> - SSPP_VIG0, SSPP_VIG1, SSPP_VIG2, SSPP_VIG3,
>>> - };
>>> - static const enum mdp5_pipe dma_planes[] = {
>>> - SSPP_DMA0, SSPP_DMA1,
>>> - };
>>> struct drm_device *dev = mdp5_kms->dev;
>>> struct msm_drm_private *priv = dev->dev_private;
>>> const struct mdp5_cfg_hw *hw_cfg;
>>> @@ -339,58 +334,34 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
>>>
>>> hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>>
>>> - /* construct CRTCs and their private planes: */
>>> - for (i = 0; i < hw_cfg->pipe_rgb.count; i++) {
>>> + /* Construct planes equaling the number of hw pipes, and CRTCs
>>> +* for the N layer-mixers (LM). The first N planes become primary
>>> +* planes for the CRTCs, with the remainder as overlay planes:
>>> +*/
>>
>>
>> Jfyi, we might need to change this a bit in the future. It'll be better to
>> get the max number of displays connected on our platform via parsing DT,
>> etc, and calculate CRTCs based on that, and not number of layermixers. Maybe
>> add a couple for writeback too. This way, we get the right number of
>> CRTCs, and we don't rely on #LMs, since we can have 2 per crtc
>> in the future.
>
> I *guess* when we get to that stage, we'll dynamically assign LM's
> too, in a similar way as hwpipe. And I suppose we could also put a
> cap on # of crtc's based on # of encoders?
Yeah. Currently, we end up creating 2 encoders for each DSI instance,
i.e, 1 for command mode, and another for video mode. Only one can be
used at a time. If we adjust for that, then I guess # of crtcs should
equal to # of encoders.
Archit
>
> BR,
> -R
>
>> Reviewed-by: Archit Taneja
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
[linux-sunxi] [PATCH v5 4/7] ASoC: sunxi: Add sun8i I2S driver
Hi, On Sun, Nov 06, 2016 at 07:02:48PM +0100, Jean-Francois Moine wrote: > On Sun, 23 Oct 2016 09:33:16 +0800 > Chen-Yu Tsai wrote: > > > On Fri, Oct 21, 2016 at 4:36 PM, Jean-Francois Moine > > wrote: > > > This patch adds I2S support to sun8i SoCs as the A83T and H3. > > > > > > Signed-off-by: Jean-Francois Moine > > > --- > > > Note: This driver is closed to the sun4i-i2s except that: > > > - it handles the H3 > > > > If it's close to sun4i-i2s, you should probably rework that one to support > > the newer SoCs. > > I started to add the H3 into the sun4i-i2s, but I am blocked with > regmap. > Many H3 registers are common with the A10, but some of them have more > or less fields, the fields may be at different offsets. And, finally, > some registers are completely different. > This would not raise any problem, except with regmap which is really > painful. That's weird, because regmap's regmap_field should make that much easier. > As I may understood, regmap is used to simplify suspend/resume, but, is > it useful to save the I2S register on suspend? > Practically, I am streaming some tune on my device. I suspend it for > any reason. The next morning, I resume it. Are you sure I want to > continue to hear the end of the tune? > > I better think that streaming should be simply stopped on suspend. You're mistaken. The code in there is for *runtime* suspend, ie when the device is no longer used, so that case shouldn't even happen at all. (And real suspend isn't supported anyway) > Then, there is no need to save the playing registers, and, here I am, > there is no need to use regmap. > > May I go this way? No, please don't. regmap is also providing very useful features, such as access to all the registers through debugfs, or tracing. What exactly feels painful to you? Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/a27a59e7/attachment.sig>
[Bug 95306] Random Blank(black) screens on "Carrizo"
https://bugs.freedesktop.org/show_bug.cgi?id=95306 --- Comment #29 from Tom --- Linux 4.9rc4 seems to fix the suspend issue, but booting up is still buggy. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/e2279274/attachment.html>
[PATCH] drm/amdgpu: Attach exclusive fence to prime exported bo's. (v3)
On 11/07/2016 08:55 AM, Christian König wrote:
> Am 07.11.2016 um 04:29 schrieb Michel Dänzer:
>> On 07/11/16 11:47 AM, Mario Kleiner wrote:
>>> External clients which import our bo's wait only
>>> for exclusive dmabuf-fences, not on shared ones,
>>> so attach fences on exported buffers as exclusive
>>> ones, if the buffers get imported by an external
>>> client.
>>>
>>> See discussion in thread:
>>> https://lists.freedesktop.org/archives/dri-devel/2016-October/122370.html
>>>
>>>
>>> Tested on Intel iGPU + AMD Tonga dGPU as DRI3/Present
>>> Prime render offload, and with the Tonga standalone as
>>> primary gpu.
>>>
>>> v2: Add a wait for all shared fences before prime export,
>>> as suggested by Christian Koenig.
>>>
>>> v3: - Mark buffer prime_exported in amdgpu_gem_prime_pin,
>>> so we only use the exclusive fence when exporting a
>>> bo to external clients like a separate iGPU, but not
>>> when exporting/importing from/to ourselves as part of
>>> regular DRI3 fd passing.
>>>
>>> - Propagate failure of reservation_object_wait_rcu back
>>> to caller.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95472
>>> (v1) Tested-by: Mike Lothian
>> FWIW, I think the (v1) is usually added at the end of the line, not at
>> the beginning.
Ok.
>>
>> [...]
>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> index 7700dc2..c4494d0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
>>> @@ -81,6 +81,20 @@ int amdgpu_gem_prime_pin(struct drm_gem_object *obj)
>>> {
>>> struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>>> int ret = 0;
>>> +long lret;
>>> +
>>> +/*
>>> + * Wait for all shared fences to complete before we switch to
>>> future
>>> + * use of exclusive fence on this prime_exported bo.
>>> + */
>>> +lret = reservation_object_wait_timeout_rcu(bo->tbo.resv, true,
>>> false,
>>> + MAX_SCHEDULE_TIMEOUT);
>>> +if (unlikely(lret < 0)) {
>>> +DRM_DEBUG_PRIME("Fence wait failed: %li\n", lret);
>>> +return lret;
>>> +}
>>> +
>>> +bo->prime_exported = true;
>> We should probably clear bo->prime_exported in amdgpu_gem_prime_unpin.
>>
>> Also, I think we should set bo->prime_exported (prime_shared?) in
>> amdgpu_gem_prime_import_sg_table as well, so that we'll also set
>> exclusive fences on BOs imported from other GPUs.
> Yes, really good idea.
>
Ok. I guess we don't need to wait for fences there before setting the
flag/counter, as we can't have done anything yet with the bo just
created from the imported sg_table?
> Additional to that are we sure that amdgpu_gem_prime_pin() is only
> called once for each GEM object? Could in theory be that somebody
> exports a BO to another GFX device as well as a V4L device for example.
>
> In this case we would need a counter, but I need to double check that as
> well.
>
If we want to clear the prime_exported flag in amdgpu_gem_prime_unpin()
as an optimization if all clients detach from the buffer, then we need a
prime_shared_counter instead of a prime_exported flag afaics. The dmabuf
api allows multiple clients to import and attach to an exported dmabuf,
each one calling dma_buf_attach which will call amdgpu_gem_prime_pin.
> In general I would protected the flag/counter by the BO being reserved
> to avoid any races. In other word move those lines a bit down after the
> amdgpu_bo_reserve().
Ok.
-mario
>
> Regards,
> Christian.
[PATCH v9 11/11] drm/i915: Add a kerneldoc summary for i915_perf.c
In particular this tries to capture for posterity some of the early challenges we had with using the core perf infrastructure in case we ever want to revisit adapting perf for device metrics. Cc: Chris Wilson Signed-off-by: Robert Bragg Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/i915_perf.c | 163 +++ 1 file changed, 163 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 1a87fe9..9551282 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -24,6 +24,169 @@ * Robert Bragg */ + +/** + * DOC: i915 Perf, streaming API for GPU metrics + * + * Gen graphics supports a large number of performance counters that can help + * driver and application developers understand and optimize their use of the + * GPU. + * + * This i915 perf interface enables userspace to configure and open a file + * descriptor representing a stream of GPU metrics which can then be read() as + * a stream of sample records. + * + * The interface is particularly suited to exposing buffered metrics that are + * captured by DMA from the GPU, unsynchronized with and unrelated to the CPU. + * + * Streams representing a single context are accessible to applications with a + * corresponding drm file descriptor, such that OpenGL can use the interface + * without special privileges. Access to system-wide metrics requires root + * privileges by default, unless changed via the dev.i915.perf_event_paranoid + * sysctl option. + * + * + * The interface was initially inspired by the core Perf infrastructure but + * some notable differences are: + * + * i915 perf file descriptors represent a "stream" instead of an "event"; where + * a perf event primarily corresponds to a single 64bit value, while a stream + * might sample sets of tightly-coupled counters, depending on the + * configuration. For example the Gen OA unit isn't designed to support + * orthogonal configurations of individual counters; it's configured for a set + * of related counters. Samples for an i915 perf stream capturing OA metrics + * will include a set of counter values packed in a compact HW specific format. + * The OA unit supports a number of different packing formats which can be + * selected by the user opening the stream. Perf has support for grouping + * events, but each event in the group is configured, validated and + * authenticated individually with separate system calls. + * + * i915 perf stream configurations are provided as an array of u64 (key,value) + * pairs, instead of a fixed struct with multiple miscellaneous config members, + * interleaved with event-type specific members. + * + * i915 perf doesn't support exposing metrics via an mmap'd circular buffer. + * The supported metrics are being written to memory by the GPU unsynchronized + * with the CPU, using HW specific packing formats for counter sets. Sometimes + * the constraints on HW configuration require reports to be filtered before it + * would be acceptable to expose them to unprivileged applications - to hide + * the metrics of other processes/contexts. For these use cases a read() based + * interface is a good fit, and provides an opportunity to filter data as it + * gets copied from the GPU mapped buffers to userspace buffers. + * + * + * Some notes regarding Linux Perf: + * + * + * The first prototype of this driver was based on the core perf + * infrastructure, and while we did make that mostly work, with some changes to + * perf, we found we were breaking or working around too many assumptions baked + * into perf's currently cpu centric design. + * + * In the end we didn't see a clear benefit to making perf's implementation and + * interface more complex by changing design assumptions while we knew we still + * wouldn't be able to use any existing perf based userspace tools. + * + * Also considering the Gen specific nature of the Observability hardware and + * how userspace will sometimes need to combine i915 perf OA metrics with + * side-band OA data captured via MI_REPORT_PERF_COUNT commands; we're + * expecting the interface to be used by a platform specific userspace such as + * OpenGL or tools. This is to say; we aren't inherently missing out on having + * a standard vendor/architecture agnostic interface by not using perf. + * + * + * For posterity, in case we might re-visit trying to adapt core perf to be + * better suited to exposing i915 metrics these were the main pain points we + * hit: + * + * - The perf based OA PMU driver broke some significant design assumptions: + * + * Existing perf pmus are used for profiling work on a cpu and we were + * introducing the idea of _IS_DEVICE pmus with different security + * implications, the need to fake cpu-related data (such as user/kernel + * registers) to fit with perf's current design, and adding _DEVICE records + * as a way to forward device-specific status records. + *
[PATCH v9 10/11] drm/i915: Add more Haswell OA metric sets
This adds 'compute', 'compute extended', 'memory reads', 'memory writes'
and 'sampler balance' metric sets for Haswell.
The code is auto generated from an XML description of metric sets,
currently maintained in gputop, ref:
https://github.com/rib/gputop
> gputop-data/oa-*.xml
> scripts/i915-perf-kernelgen.py
$ make -C gputop-data -f Makefile.xml
Signed-off-by: Robert Bragg
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/i915_oa_hsw.c | 559 -
1 file changed, 558 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_oa_hsw.c
b/drivers/gpu/drm/i915/i915_oa_hsw.c
index 6af25cf..4ddf756 100644
--- a/drivers/gpu/drm/i915/i915_oa_hsw.c
+++ b/drivers/gpu/drm/i915/i915_oa_hsw.c
@@ -31,9 +31,14 @@
enum metric_set_id {
METRIC_SET_ID_RENDER_BASIC = 1,
+ METRIC_SET_ID_COMPUTE_BASIC,
+ METRIC_SET_ID_COMPUTE_EXTENDED,
+ METRIC_SET_ID_MEMORY_READS,
+ METRIC_SET_ID_MEMORY_WRITES,
+ METRIC_SET_ID_SAMPLER_BALANCE,
};
-int i915_oa_n_builtin_metric_sets_hsw = 1;
+int i915_oa_n_builtin_metric_sets_hsw = 6;
static const struct i915_oa_reg b_counter_config_render_basic[] = {
{ _MMIO(0x2724), 0x0080 },
@@ -112,6 +117,298 @@ get_render_basic_mux_config(struct drm_i915_private
*dev_priv,
return mux_config_render_basic;
}
+static const struct i915_oa_reg b_counter_config_compute_basic[] = {
+ { _MMIO(0x2710), 0x },
+ { _MMIO(0x2714), 0x0080 },
+ { _MMIO(0x2718), 0x },
+ { _MMIO(0x271c), 0x },
+ { _MMIO(0x2720), 0x },
+ { _MMIO(0x2724), 0x0080 },
+ { _MMIO(0x2728), 0x },
+ { _MMIO(0x272c), 0x },
+ { _MMIO(0x2740), 0x },
+ { _MMIO(0x2744), 0x },
+ { _MMIO(0x2748), 0x },
+ { _MMIO(0x274c), 0x },
+ { _MMIO(0x2750), 0x },
+ { _MMIO(0x2754), 0x },
+ { _MMIO(0x2758), 0x },
+ { _MMIO(0x275c), 0x },
+ { _MMIO(0x236c), 0x },
+};
+
+static const struct i915_oa_reg mux_config_compute_basic[] = {
+ { _MMIO(0x253a4), 0x },
+ { _MMIO(0x2681c), 0x01f00800 },
+ { _MMIO(0x26820), 0x1000 },
+ { _MMIO(0x2781c), 0x01f00800 },
+ { _MMIO(0x26520), 0x0007 },
+ { _MMIO(0x265a0), 0x0007 },
+ { _MMIO(0x25380), 0x0010 },
+ { _MMIO(0x2538c), 0x0030 },
+ { _MMIO(0x25384), 0xaa8a },
+ { _MMIO(0x25404), 0x },
+ { _MMIO(0x26800), 0x4202 },
+ { _MMIO(0x26808), 0x00605817 },
+ { _MMIO(0x2680c), 0x10001005 },
+ { _MMIO(0x26804), 0x },
+ { _MMIO(0x27800), 0x0102 },
+ { _MMIO(0x27808), 0x0c0701e0 },
+ { _MMIO(0x2780c), 0x000200a0 },
+ { _MMIO(0x27804), 0x },
+ { _MMIO(0x26484), 0x4400 },
+ { _MMIO(0x26704), 0x4400 },
+ { _MMIO(0x26500), 0x0006 },
+ { _MMIO(0x26510), 0x0001 },
+ { _MMIO(0x26504), 0x8800 },
+ { _MMIO(0x26580), 0x0006 },
+ { _MMIO(0x26590), 0x0020 },
+ { _MMIO(0x26584), 0x },
+ { _MMIO(0x26104), 0x5582 },
+ { _MMIO(0x26184), 0xaa86 },
+ { _MMIO(0x25420), 0x08320c83 },
+ { _MMIO(0x25424), 0x06820c83 },
+ { _MMIO(0x2541c), 0x },
+ { _MMIO(0x25428), 0x0c03 },
+};
+
+static const struct i915_oa_reg *
+get_compute_basic_mux_config(struct drm_i915_private *dev_priv,
+int *len)
+{
+ *len = ARRAY_SIZE(mux_config_compute_basic);
+ return mux_config_compute_basic;
+}
+
+static const struct i915_oa_reg b_counter_config_compute_extended[] = {
+ { _MMIO(0x2724), 0xf080 },
+ { _MMIO(0x2720), 0x },
+ { _MMIO(0x2714), 0xf080 },
+ { _MMIO(0x2710), 0x },
+ { _MMIO(0x2770), 0x0007fe2a },
+ { _MMIO(0x2774), 0xff00 },
+ { _MMIO(0x2778), 0x0007fe6a },
+ { _MMIO(0x277c), 0xff00 },
+ { _MMIO(0x2780), 0x0007fe92 },
+ { _MMIO(0x2784), 0xff00 },
+ { _MMIO(0x2788), 0x0007fea2 },
+ { _MMIO(0x278c), 0xff00 },
+ { _MMIO(0x2790), 0x0007fe32 },
+ { _MMIO(0x2794), 0xff00 },
+ { _MMIO(0x2798), 0x0007fe9a },
+ { _MMIO(0x279c), 0xff00 },
+ { _MMIO(0x27a0), 0x0007ff23 },
+ { _MMIO(0x27a4), 0xff00 },
+ { _MMIO(0x27a8), 0x0007fff3 },
+ { _MMIO(0x27ac), 0xfffe },
+};
+
+static const struct i915_oa_reg mux_config_compute_extended[] = {
+ { _MMIO(0x2681c), 0x3eb00800 },
+ { _MMIO(0x26820), 0x0090 },
+ { _MMIO(0x25384), 0x02aa },
+ { _MMIO(0x25404), 0x03ff },
+ { _MMIO(0x26800), 0x00142284 },
+ { _MMIO(0x26808), 0x0e629062 },
+ { _MMIO(0x2680c), 0x3f6f55cb },
+ { _MMIO(0x26810), 0x0014 },
+ { _MMIO(0x26804), 0x },
+ { _MMIO(0x26104), 0x02a
[PATCH v9 09/11] drm/i915: add dev.i915.oa_max_sample_rate sysctl
The maximum OA sampling frequency is now configurable via a
dev.i915.oa_max_sample_rate sysctl parameter.
Following the precedent set by perf's similar
kernel.perf_event_max_sample_rate the default maximum rate is 10Hz
Signed-off-by: Robert Bragg
---
drivers/gpu/drm/i915/i915_perf.c | 61
1 file changed, 50 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index e51c1d8..1a87fe9 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -82,6 +82,21 @@ static u32 i915_perf_stream_paranoid = true;
#define INVALID_CTX_ID 0x
+/* For sysctl proc_dointvec_minmax of i915_oa_max_sample_rate
+ *
+ * 160ns is the smallest sampling period we can theoretically program the OA
+ * unit with on Haswell, corresponding to 6.25MHz.
+ */
+static int oa_sample_rate_hard_limit = 625;
+
+/* Theoretically we can program the OA unit to sample every 160ns but don't
+ * allow that by default unless root...
+ *
+ * The default threshold of 10Hz is based on perf's similar
+ * kernel.perf_event_max_sample_rate sysctl parameter.
+ */
+static u32 i915_oa_max_sample_rate = 10;
+
/* XXX: beware if future OA HW adds new report formats that the current
* code assumes all reports have a power-of-two size and ~(size - 1) can
* be used as a mask to align the OA tail pointer.
@@ -1314,6 +1329,7 @@ static int read_properties_unlocked(struct
drm_i915_private *dev_priv,
}
for (i = 0; i < n_props; i++) {
+ u64 oa_period, oa_freq_hz;
u64 id, value;
int ret;
@@ -1359,21 +1375,35 @@ static int read_properties_unlocked(struct
drm_i915_private *dev_priv,
return -EINVAL;
}
- /* NB: The exponent represents a period as follows:
-*
-* 80ns * 2^(period_exponent + 1)
-*
-* Theoretically we can program the OA unit to sample
+ /* Theoretically we can program the OA unit to sample
* every 160ns but don't allow that by default unless
* root.
*
-* Referring to perf's
-* kernel.perf_event_max_sample_rate for a precedent
-* (10 by default); with an OA exponent of 6 we get
-* a period of 10.240 microseconds -just under 10Hz
+* On Haswell the period is derived from the exponent
+* as:
+*
+* period = 80ns * 2^(exponent + 1)
+*/
+ BUILD_BUG_ON(sizeof(oa_period) != 8);
+ oa_period = 80ull * (2ull << value);
+
+ /* This check is primarily to ensure that oa_period <=
+* UINT32_MAX (before passing to do_div which only
+* accepts a u32 denominator), but we can also skip
+* checking anything < 1Hz which implicitly can't be
+* limited via an integer oa_max_sample_rate.
*/
- if (value < 6 && !capable(CAP_SYS_ADMIN)) {
- DRM_ERROR("Minimum OA sampling exponent is 6
without root privileges\n");
+ if (oa_period <= NSEC_PER_SEC) {
+ u64 tmp = NSEC_PER_SEC;
+ do_div(tmp, oa_period);
+ oa_freq_hz = tmp;
+ } else
+ oa_freq_hz = 0;
+
+ if (oa_freq_hz > i915_oa_max_sample_rate &&
+ !capable(CAP_SYS_ADMIN)) {
+ DRM_ERROR("OA exponent would exceed the max
sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without root
privileges\n",
+ i915_oa_max_sample_rate);
return -EACCES;
}
@@ -1481,6 +1511,15 @@ static struct ctl_table oa_table[] = {
.extra1 = &zero,
.extra2 = &one,
},
+ {
+.procname = "oa_max_sample_rate",
+.data = &i915_oa_max_sample_rate,
+.maxlen = sizeof(i915_oa_max_sample_rate),
+.mode = 0644,
+.proc_handler = proc_dointvec_minmax,
+.extra1 = &zero,
+.extra2 = &oa_sample_rate_hard_limit,
+},
{}
};
--
2.10.1
[PATCH v9 08/11] drm/i915: Add dev.i915.perf_stream_paranoid sysctl option
Consistent with the kernel.perf_event_paranoid sysctl option that can
allow non-root users to access system wide cpu metrics, this can
optionally allow non-root users to access system wide OA counter metrics
from Gen graphics hardware.
Signed-off-by: Robert Bragg
Reviewed-by: Matthew Auld
Reviewed-by: Sourab Gupta
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_perf.c | 50 +++-
2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 15fba6b..8962bfd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2188,6 +2188,7 @@ struct drm_i915_private {
bool initialized;
struct kobject *metrics_kobj;
+ struct ctl_table_header *sysctl_header;
struct mutex lock;
struct list_head streams;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index c427cd8c..e51c1d8 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -64,6 +64,11 @@
#define POLL_FREQUENCY 200
#define POLL_PERIOD (NSEC_PER_SEC / POLL_FREQUENCY)
+/* for sysctl proc_dointvec_minmax of dev.i915.perf_stream_paranoid */
+static int zero;
+static int one = 1;
+static u32 i915_perf_stream_paranoid = true;
+
/* The maximum exponent the hardware accepts is 63 (essentially it selects one
* of the 64bit timestamp bits to trigger reports from) but there's currently
* no known use case for sampling as infrequently as once per 47 thousand
years.
@@ -1207,7 +1212,13 @@ i915_perf_open_ioctl_locked(struct drm_i915_private
*dev_priv,
}
}
- if (!specific_ctx && !capable(CAP_SYS_ADMIN)) {
+ /* Similar to perf's kernel.perf_paranoid_cpu sysctl option
+* we check a dev.i915.perf_stream_paranoid sysctl option
+* to determine if it's ok to access system wide OA counters
+* without CAP_SYS_ADMIN privileges.
+*/
+ if (!specific_ctx &&
+ i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
DRM_ERROR("Insufficient privileges to open system-wide i915
perf stream\n");
ret = -EACCES;
goto err_ctx;
@@ -1460,6 +1471,39 @@ void i915_perf_unregister(struct drm_i915_private
*dev_priv)
dev_priv->perf.metrics_kobj = NULL;
}
+static struct ctl_table oa_table[] = {
+ {
+.procname = "perf_stream_paranoid",
+.data = &i915_perf_stream_paranoid,
+.maxlen = sizeof(i915_perf_stream_paranoid),
+.mode = 0644,
+.proc_handler = proc_dointvec_minmax,
+.extra1 = &zero,
+.extra2 = &one,
+},
+ {}
+};
+
+static struct ctl_table i915_root[] = {
+ {
+.procname = "i915",
+.maxlen = 0,
+.mode = 0555,
+.child = oa_table,
+},
+ {}
+};
+
+static struct ctl_table dev_root[] = {
+ {
+.procname = "dev",
+.maxlen = 0,
+.mode = 0555,
+.child = i915_root,
+},
+ {}
+};
+
void i915_perf_init(struct drm_i915_private *dev_priv)
{
if (!IS_HASWELL(dev_priv))
@@ -1490,6 +1534,8 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
dev_priv->perf.oa.n_builtin_sets =
i915_oa_n_builtin_metric_sets_hsw;
+ dev_priv->perf.sysctl_header = register_sysctl_table(dev_root);
+
dev_priv->perf.initialized = true;
}
@@ -1498,6 +1544,8 @@ void i915_perf_fini(struct drm_i915_private *dev_priv)
if (!dev_priv->perf.initialized)
return;
+ unregister_sysctl_table(dev_priv->perf.sysctl_header);
+
memset(&dev_priv->perf.oa.ops, 0, sizeof(dev_priv->perf.oa.ops));
dev_priv->perf.initialized = false;
}
--
2.10.1
[PATCH v9 07/11] drm/i915: advertise available metrics via sysfs
Each metric set is given a sysfs entry like:
/sys/class/drm/card0/metrics//id
This allows userspace to enumerate the specific sets that are available
for the current system. The 'id' file contains an unsigned integer that
can be used to open the associated metric set via
DRM_IOCTL_I915_PERF_OPEN. The is a globally unique ID for a
specific OA unit register configuration that can be reliably used by
userspace as a key to lookup corresponding counter meta data and
normalization equations.
The guid registry is currently maintained as part of gputop along with
the XML metric set descriptions and code generation scripts, ref:
https://github.com/rib/gputop
> gputop-data/guids.xml
> scripts/update-guids.py
> gputop-data/oa-*.xml
> scripts/i915-perf-kernelgen.py
$ make -C gputop-data -f Makefile.xml SYSFS=1 WHITELIST=RenderBasic
Signed-off-by: Robert Bragg
Reviewed-by: Matthew Auld
Reviewed-by: Sourab Gupta
---
drivers/gpu/drm/i915/i915_drv.c| 5
drivers/gpu/drm/i915/i915_drv.h| 4 +++
drivers/gpu/drm/i915/i915_oa_hsw.c | 51 +
drivers/gpu/drm/i915/i915_oa_hsw.h | 4 +++
drivers/gpu/drm/i915/i915_perf.c | 52 ++
5 files changed, 116 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 22b4166..4fd8650 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1125,6 +1125,9 @@ static void i915_driver_register(struct drm_i915_private
*dev_priv)
i915_debugfs_register(dev_priv);
i915_guc_register(dev_priv);
i915_setup_sysfs(dev_priv);
+
+ /* Depends on sysfs having been initialized */
+ i915_perf_register(dev_priv);
} else
DRM_ERROR("Failed to register driver for userspace access!\n");
@@ -1161,6 +1164,8 @@ static void i915_driver_unregister(struct
drm_i915_private *dev_priv)
acpi_video_unregister();
intel_opregion_unregister(dev_priv);
+ i915_perf_unregister(dev_priv);
+
i915_teardown_sysfs(dev_priv);
i915_guc_unregister(dev_priv);
i915_debugfs_unregister(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8003120..15fba6b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2187,6 +2187,8 @@ struct drm_i915_private {
struct {
bool initialized;
+ struct kobject *metrics_kobj;
+
struct mutex lock;
struct list_head streams;
@@ -3884,6 +3886,8 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
/* i915_perf.c */
extern void i915_perf_init(struct drm_i915_private *dev_priv);
extern void i915_perf_fini(struct drm_i915_private *dev_priv);
+extern void i915_perf_register(struct drm_i915_private *dev_priv);
+extern void i915_perf_unregister(struct drm_i915_private *dev_priv);
/* i915_suspend.c */
extern int i915_save_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_oa_hsw.c
b/drivers/gpu/drm/i915/i915_oa_hsw.c
index 8906380..6af25cf 100644
--- a/drivers/gpu/drm/i915/i915_oa_hsw.c
+++ b/drivers/gpu/drm/i915/i915_oa_hsw.c
@@ -24,6 +24,8 @@
*
*/
+#include
+
#include "i915_drv.h"
#include "i915_oa_hsw.h"
@@ -142,3 +144,52 @@ int i915_oa_select_metric_set_hsw(struct drm_i915_private
*dev_priv)
return -ENODEV;
}
}
+
+static ssize_t
+show_render_basic_id(struct device *kdev, struct device_attribute *attr, char
*buf)
+{
+ return sprintf(buf, "%d\n", METRIC_SET_ID_RENDER_BASIC);
+}
+
+static struct device_attribute dev_attr_render_basic_id = {
+ .attr = { .name = "id", .mode = 0444 },
+ .show = show_render_basic_id,
+ .store = NULL,
+};
+
+static struct attribute *attrs_render_basic[] = {
+ &dev_attr_render_basic_id.attr,
+ NULL,
+};
+
+static struct attribute_group group_render_basic = {
+ .name = "403d8832-1a27-4aa6-a64e-f5389ce7b212",
+ .attrs = attrs_render_basic,
+};
+
+int
+i915_perf_register_sysfs_hsw(struct drm_i915_private *dev_priv)
+{
+ int mux_len;
+ int ret = 0;
+
+ if (get_render_basic_mux_config(dev_priv, &mux_len)) {
+ ret = sysfs_create_group(dev_priv->perf.metrics_kobj,
&group_render_basic);
+ if (ret)
+ goto error_render_basic;
+ }
+
+ return 0;
+
+error_render_basic:
+ return ret;
+}
+
+void
+i915_perf_unregister_sysfs_hsw(struct drm_i915_private *dev_priv)
+{
+ int mux_len;
+
+ if (get_render_basic_mux_config(dev_priv, &mux_len))
+ sysfs_remove_group(dev_priv->perf.metrics_kobj,
&group_render_basic);
+}
diff --git a/drivers/gpu/drm/i915/i915_oa_hsw.h
b/drivers/gpu/drm/i915/i915_oa_hsw.h
index b618a1f..429a229 100644
--- a/drivers/gpu/drm/i915/i915_oa_hsw.h
+++ b/drivers/gpu/drm/i915/i915_oa_hsw.h
@@ -31,4
[PATCH v9 06/11] drm/i915: Enable i915 perf stream for Haswell OA unit
Gen graphics hardware can be set up to periodically write snapshots of
performance counters into a circular buffer via its Observation
Architecture and this patch exposes that capability to userspace via the
i915 perf interface.
v2:
Make sure to initialize ->specific_ctx_id when opening, without
relying on _pin_notify hook, in case ctx already pinned.
v3:
Revert back to pinning ctx upfront when opening stream, removing
need to hook in to pinning and to update OACONTROL on the fly.
Signed-off-by: Robert Bragg
Signed-off-by: Zhenyu Wang
Cc: Chris Wilson
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/i915_drv.h | 66 ++-
drivers/gpu/drm/i915/i915_perf.c | 1036 +-
drivers/gpu/drm/i915/i915_reg.h | 338 +
include/uapi/drm/i915_drm.h | 71 ++-
4 files changed, 1482 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bdebb66..8003120 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1785,6 +1785,11 @@ struct intel_wm_config {
bool sprites_scaled;
};
+struct i915_oa_format {
+ u32 format;
+ int size;
+};
+
struct i915_oa_reg {
i915_reg_t addr;
u32 value;
@@ -1805,11 +1810,6 @@ struct i915_perf_stream_ops {
*/
void (*disable)(struct i915_perf_stream *stream);
- /* Return: true if any i915 perf records are ready to read()
-* for this stream.
-*/
- bool (*can_read)(struct i915_perf_stream *stream);
-
/* Call poll_wait, passing a wait queue that will be woken
* once there is something ready to read() for the stream
*/
@@ -1819,9 +1819,7 @@ struct i915_perf_stream_ops {
/* For handling a blocking read, wait until there is something
* to ready to read() for the stream. E.g. wait on the same
-* wait queue that would be passed to poll_wait() until
-* ->can_read() returns true (if its safe to call ->can_read()
-* without the i915 perf lock held).
+* wait queue that would be passed to poll_wait().
*/
int (*wait_unlocked)(struct i915_perf_stream *stream);
@@ -1861,11 +1859,28 @@ struct i915_perf_stream {
struct list_head link;
u32 sample_flags;
+ int sample_size;
struct i915_gem_context *ctx;
bool enabled;
- struct i915_perf_stream_ops *ops;
+ const struct i915_perf_stream_ops *ops;
+};
+
+struct i915_oa_ops {
+ void (*init_oa_buffer)(struct drm_i915_private *dev_priv);
+ int (*enable_metric_set)(struct drm_i915_private *dev_priv);
+ void (*disable_metric_set)(struct drm_i915_private *dev_priv);
+ void (*oa_enable)(struct drm_i915_private *dev_priv);
+ void (*oa_disable)(struct drm_i915_private *dev_priv);
+ void (*update_oacontrol)(struct drm_i915_private *dev_priv);
+ void (*update_hw_ctx_id_locked)(struct drm_i915_private *dev_priv,
+ u32 ctx_id);
+ int (*read)(struct i915_perf_stream *stream,
+ char __user *buf,
+ size_t count,
+ size_t *offset);
+ bool (*oa_buffer_is_empty)(struct drm_i915_private *dev_priv);
};
struct drm_i915_private {
@@ -2171,16 +2186,47 @@ struct drm_i915_private {
struct {
bool initialized;
+
struct mutex lock;
struct list_head streams;
+ spinlock_t hook_lock;
+
struct {
- u32 metrics_set;
+ struct i915_perf_stream *exclusive_stream;
+
+ u32 specific_ctx_id;
+ struct i915_vma *pinned_rcs_vma;
+
+ struct hrtimer poll_check_timer;
+ wait_queue_head_t poll_wq;
+ bool pollin;
+
+ bool periodic;
+ int period_exponent;
+ int timestamp_frequency;
+
+ int tail_margin;
+
+ int metrics_set;
const struct i915_oa_reg *mux_regs;
int mux_regs_len;
const struct i915_oa_reg *b_counter_regs;
int b_counter_regs_len;
+
+ struct {
+ struct i915_vma *vma;
+ u8 *vaddr;
+ int format;
+ int format_size;
+ } oa_buffer;
+
+ u32 gen7_latched_oastatus1;
+
+ struct i915_oa_ops ops;
+ const struct i915_oa_format *oa_formats;
+ int n_builtin_sets;
} oa;
} perf;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 777ce65..5465
[PATCH v9 05/11] drm/i915: Add 'render basic' Haswell OA unit config
Adds a static OA unit, MUX + B Counter configuration for basic render
metrics on Haswell. This is auto generated from an XML
description of metric sets, currently maintained in gputop, ref:
https://github.com/rib/gputop
> gputop-data/oa-*.xml
> scripts/i915-perf-kernelgen.py
$ make -C gputop-data -f Makefile.xml SYSFS=0 WHITELIST=RenderBasic
Signed-off-by: Robert Bragg
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/Makefile | 3 +-
drivers/gpu/drm/i915/i915_drv.h| 14
drivers/gpu/drm/i915/i915_oa_hsw.c | 144 +
drivers/gpu/drm/i915/i915_oa_hsw.h | 34 +
4 files changed, 194 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/i915_oa_hsw.c
create mode 100644 drivers/gpu/drm/i915/i915_oa_hsw.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 08b43af..7e9e6d0 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -117,7 +117,8 @@ i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
i915-y += i915_vgpu.o
# perf code
-i915-y += i915_perf.o
+i915-y += i915_perf.o \
+ i915_oa_hsw.o
ifeq ($(CONFIG_DRM_I915_GVT),y)
i915-y += intel_gvt.o
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e4cd322..bdebb66 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1785,6 +1785,11 @@ struct intel_wm_config {
bool sprites_scaled;
};
+struct i915_oa_reg {
+ i915_reg_t addr;
+ u32 value;
+};
+
struct i915_perf_stream;
struct i915_perf_stream_ops {
@@ -2168,6 +2173,15 @@ struct drm_i915_private {
bool initialized;
struct mutex lock;
struct list_head streams;
+
+ struct {
+ u32 metrics_set;
+
+ const struct i915_oa_reg *mux_regs;
+ int mux_regs_len;
+ const struct i915_oa_reg *b_counter_regs;
+ int b_counter_regs_len;
+ } oa;
} perf;
/* Abstract the submission mechanism (legacy ringbuffer or execlists)
away */
diff --git a/drivers/gpu/drm/i915/i915_oa_hsw.c
b/drivers/gpu/drm/i915/i915_oa_hsw.c
new file mode 100644
index 000..8906380
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_oa_hsw.c
@@ -0,0 +1,144 @@
+/*
+ * Autogenerated file, DO NOT EDIT manually!
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "i915_drv.h"
+#include "i915_oa_hsw.h"
+
+enum metric_set_id {
+ METRIC_SET_ID_RENDER_BASIC = 1,
+};
+
+int i915_oa_n_builtin_metric_sets_hsw = 1;
+
+static const struct i915_oa_reg b_counter_config_render_basic[] = {
+ { _MMIO(0x2724), 0x0080 },
+ { _MMIO(0x2720), 0x },
+ { _MMIO(0x2714), 0x0080 },
+ { _MMIO(0x2710), 0x },
+};
+
+static const struct i915_oa_reg mux_config_render_basic[] = {
+ { _MMIO(0x253a4), 0x0160 },
+ { _MMIO(0x25440), 0x0010 },
+ { _MMIO(0x25128), 0x },
+ { _MMIO(0x2691c), 0x0800 },
+ { _MMIO(0x26aa0), 0x0150 },
+ { _MMIO(0x26b9c), 0x6000 },
+ { _MMIO(0x2791c), 0x0800 },
+ { _MMIO(0x27aa0), 0x0150 },
+ { _MMIO(0x27b9c), 0x6000 },
+ { _MMIO(0x2641c), 0x0400 },
+ { _MMIO(0x25380), 0x0010 },
+ { _MMIO(0x2538c), 0x },
+ { _MMIO(0x25384), 0x0800 },
+ { _MMIO(0x25400), 0x0004 },
+ { _MMIO(0x2540c), 0x06029000 },
+ { _MMIO(0x25410), 0x0002 },
+ { _MMIO(0x25404), 0x5c30 },
+ { _MMIO(0x25100), 0x0016 },
+ { _MMIO(0x25110), 0x0400 },
+ { _MMIO(0x25104), 0x },
+ { _MMIO(0x26804), 0x1211 },
+ { _MMIO(0x26884), 0x0100 },
+ { _MMIO(0x26900), 0x000
[PATCH v9 04/11] drm/i915: don't whitelist oacontrol in cmd parser
Being able to program OACONTROL from a non-privileged batch buffer is
not sufficient to be able to configure the OA unit. This was originally
allowed to help enable Mesa to expose OA counters via the
INTEL_performance_query extension, but the current implementation based
on programming OACONTROL via a batch buffer isn't able to report useable
data without a more complete OA unit configuration. Mesa handles the
possibility that writes to OACONTROL may not be allowed and so only
advertises the extension after explicitly testing that a write to
OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
should be ok for userspace.
Removing this simplifies adding a new kernel api for configuring the OA
unit without needing to consider the possibility that userspace might
trample on OACONTROL state which we'd like to start managing within
the kernel instead. In particular running any Mesa based GL application
currently results in clearing OACONTROL when initializing which would
disable the capturing of metrics.
Signed-off-by: Robert Bragg
Reviewed-by: Matthew Auld
Reviewed-by: Sourab Gupta
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 38 ++
1 file changed, 2 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index c9d2ecd..f3453dc 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -450,7 +450,6 @@ static const struct drm_i915_reg_descriptor
gen7_render_regs[] = {
REG64(PS_INVOCATION_COUNT),
REG64(PS_DEPTH_COUNT),
REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
- REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below. */
REG64(MI_PREDICATE_SRC0),
REG64(MI_PREDICATE_SRC1),
REG32(GEN7_3DPRIM_END_OFFSET),
@@ -1060,8 +1059,7 @@ bool intel_engine_needs_cmd_parser(struct intel_engine_cs
*engine)
static bool check_cmd(const struct intel_engine_cs *engine,
const struct drm_i915_cmd_descriptor *desc,
const u32 *cmd, u32 length,
- const bool is_master,
- bool *oacontrol_set)
+ const bool is_master)
{
if (desc->flags & CMD_DESC_SKIP)
return true;
@@ -1099,31 +1097,6 @@ static bool check_cmd(const struct intel_engine_cs
*engine,
}
/*
-* OACONTROL requires some special handling for
-* writes. We want to make sure that any batch which
-* enables OA also disables it before the end of the
-* batch. The goal is to prevent one process from
-* snooping on the perf data from another process. To do
-* that, we need to check the value that will be written
-* to the register. Hence, limit OACONTROL writes to
-* only MI_LOAD_REGISTER_IMM commands.
-*/
- if (reg_addr == i915_mmio_reg_offset(GEN7_OACONTROL)) {
- if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
- DRM_DEBUG_DRIVER("CMD: Rejected LRM to
OACONTROL\n");
- return false;
- }
-
- if (desc->cmd.value == MI_LOAD_REGISTER_REG) {
- DRM_DEBUG_DRIVER("CMD: Rejected LRR to
OACONTROL\n");
- return false;
- }
-
- if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1))
- *oacontrol_set = (cmd[offset + 1] != 0);
- }
-
- /*
* Check the value written to the register against the
* allowed mask/value pair given in the whitelist entry.
*/
@@ -1214,7 +1187,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
u32 *cmd, *batch_end;
struct drm_i915_cmd_descriptor default_desc = noop_desc;
const struct drm_i915_cmd_descriptor *desc = &default_desc;
- bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd() */
bool needs_clflush_after = false;
int ret = 0;
@@ -1270,8 +1242,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
break;
}
- if (!check_cmd(engine, desc, cmd, length, is_master,
- &oacontrol_set)) {
+ if (!check_cmd(engine, desc, cmd, length, is_master)) {
ret = -EACCES;
break;
}
@@ -1279,11 +1250,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
[PATCH v9 03/11] drm/i915: return EACCES for check_cmd() failures
check_cmd() is checking whether a command adheres to certain
restrictions that ensure it's safe to execute within a privileged batch
buffer. Returning false implies a privilege problem, not that the
command is invalid.
The distinction makes the difference between allowing the buffer to be
executed as an unprivileged batch buffer or returning an EINVAL error to
userspace without executing anything.
In a case where userspace may want to test whether it can successfully
write to a register that needs privileges the distinction may be
important and an EINVAL error may be considered fatal.
In particular this is currently true for Mesa, which includes a test for
whether OACONTROL can be written too, but Mesa treats any error when
flushing a batch buffer as fatal, calling exit(1).
As it is currently Mesa can gracefully handle a failure to write to
OACONTROL if the command parser is disabled, but if we were to remove
OACONTROL from the parser's whitelist then the returned EINVAL would
break Mesa applications as they attempt an OACONTROL write.
This bumps the command parser version from 7 to 8, as the change is
visible to userspace.
Signed-off-by: Robert Bragg
Reviewed-by: Matthew Auld
Reviewed-by: Sourab Gupta
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 7719aed..c9d2ecd 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1272,7 +1272,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
if (!check_cmd(engine, desc, cmd, length, is_master,
&oacontrol_set)) {
- ret = -EINVAL;
+ ret = -EACCES;
break;
}
@@ -1333,6 +1333,9 @@ int i915_cmd_parser_get_version(struct drm_i915_private
*dev_priv)
* 5. GPGPU dispatch compute indirect registers.
* 6. TIMESTAMP register and Haswell CS GPR registers
* 7. Allow MI_LOAD_REGISTER_REG between whitelisted registers.
+* 8. Don't report cmd_check() failures as EINVAL errors to userspace;
+*rely on the HW to NOOP disallowed commands as it would without
+*the parser enabled.
*/
- return 7;
+ return 8;
}
--
2.10.1
[PATCH v9 02/11] drm/i915: rename OACONTROL GEN7_OACONTROL
OACONTROL changes quite a bit for gen8, with some bits split out into a
per-context OACTXCONTROL register. Rename now before adding more gen7 OA
registers
Signed-off-by: Robert Bragg
Reviewed-by: Matthew Auld
Reviewed-by: Sourab Gupta
---
drivers/gpu/drm/i915/gvt/handlers.c| 2 +-
drivers/gpu/drm/i915/i915_cmd_parser.c | 4 ++--
drivers/gpu/drm/i915/i915_reg.h| 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
b/drivers/gpu/drm/i915/gvt/handlers.c
index 9ab1f95..4527cb7 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -2177,7 +2177,7 @@ static int init_generic_mmio_info(struct intel_gvt *gvt)
MMIO_DFH(0x1217c, D_ALL, F_CMD_ACCESS, NULL, NULL);
MMIO_F(0x2290, 8, 0, 0, 0, D_HSW_PLUS, NULL, NULL);
- MMIO_D(OACONTROL, D_HSW);
+ MMIO_D(GEN7_OACONTROL, D_HSW);
MMIO_D(0x2b00, D_BDW_PLUS);
MMIO_D(0x2360, D_BDW_PLUS);
MMIO_F(0x5200, 32, 0, 0, 0, D_ALL, NULL, NULL);
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index f5039f4..7719aed 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -450,7 +450,7 @@ static const struct drm_i915_reg_descriptor
gen7_render_regs[] = {
REG64(PS_INVOCATION_COUNT),
REG64(PS_DEPTH_COUNT),
REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
- REG32(OACONTROL), /* Only allowed for LRI and SRM. See below. */
+ REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below. */
REG64(MI_PREDICATE_SRC0),
REG64(MI_PREDICATE_SRC1),
REG32(GEN7_3DPRIM_END_OFFSET),
@@ -1108,7 +1108,7 @@ static bool check_cmd(const struct intel_engine_cs
*engine,
* to the register. Hence, limit OACONTROL writes to
* only MI_LOAD_REGISTER_IMM commands.
*/
- if (reg_addr == i915_mmio_reg_offset(OACONTROL)) {
+ if (reg_addr == i915_mmio_reg_offset(GEN7_OACONTROL)) {
if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
DRM_DEBUG_DRIVER("CMD: Rejected LRM to
OACONTROL\n");
return false;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3361d7f..4623dbb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -615,7 +615,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define HSW_CS_GPR(n) _MMIO(0x2600 + (n) * 8)
#define HSW_CS_GPR_UDW(n) _MMIO(0x2600 + (n) * 8 + 4)
-#define OACONTROL _MMIO(0x2360)
+#define GEN7_OACONTROL _MMIO(0x2360)
#define _GEN7_PIPEA_DE_LOAD_SL 0x70068
#define _GEN7_PIPEB_DE_LOAD_SL 0x71068
--
2.10.1
[PATCH v9 01/11] drm/i915: Add i915 perf infrastructure
Adds base i915 perf infrastructure for Gen performance metrics.
This adds a DRM_IOCTL_I915_PERF_OPEN ioctl that takes an array of uint64
properties to configure a stream of metrics and returns a new fd usable
with standard VFS system calls including read() to read typed and sized
records; ioctl() to enable or disable capture and poll() to wait for
data.
A stream is opened something like:
uint64_t properties[] = {
/* Single context sampling */
DRM_I915_PERF_PROP_CTX_HANDLE,ctx_handle,
/* Include OA reports in samples */
DRM_I915_PERF_PROP_SAMPLE_OA, true,
/* OA unit configuration */
DRM_I915_PERF_PROP_OA_METRICS_SET,metrics_set_id,
DRM_I915_PERF_PROP_OA_FORMAT, report_format,
DRM_I915_PERF_PROP_OA_EXPONENT, period_exponent,
};
struct drm_i915_perf_open_param parm = {
.flags = I915_PERF_FLAG_FD_CLOEXEC |
I915_PERF_FLAG_FD_NONBLOCK |
I915_PERF_FLAG_DISABLED,
.properties_ptr = (uint64_t)properties,
.num_properties = sizeof(properties) / 16,
};
int fd = drmIoctl(drm_fd, DRM_IOCTL_I915_PERF_OPEN, ¶m);
Records read all start with a common { type, size } header with
DRM_I915_PERF_RECORD_SAMPLE being of most interest. Sample records
contain an extensible number of fields and it's the
DRM_I915_PERF_PROP_SAMPLE_xyz properties given when opening that
determine what's included in every sample.
No specific streams are supported yet so any attempt to open a stream
will return an error.
v2:
use i915_gem_context_get() - Chris Wilson
v3:
update read() interface to avoid passing state struct - Chris Wilson
fix some rebase fallout, with i915-perf init/deinit
v4:
s/DRM_IORW/DRM_IOW/ - Emil Velikov
Signed-off-by: Robert Bragg
Reviewed-by: Matthew Auld
Reviewed-by: Sourab Gupta
---
drivers/gpu/drm/i915/Makefile| 3 +
drivers/gpu/drm/i915/i915_drv.c | 4 +
drivers/gpu/drm/i915/i915_drv.h | 91
drivers/gpu/drm/i915/i915_perf.c | 449 +++
include/uapi/drm/i915_drm.h | 67 ++
5 files changed, 614 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_perf.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0857e50..08b43af 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -116,6 +116,9 @@ i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
# virtual gpu code
i915-y += i915_vgpu.o
+# perf code
+i915-y += i915_perf.o
+
ifeq ($(CONFIG_DRM_I915_GVT),y)
i915-y += intel_gvt.o
include $(src)/gvt/Makefile
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0213a30..22b4166 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -844,6 +844,8 @@ static int i915_driver_init_early(struct drm_i915_private
*dev_priv,
intel_detect_preproduction_hw(dev_priv);
+ i915_perf_init(dev_priv);
+
return 0;
err_gvt:
@@ -859,6 +861,7 @@ static int i915_driver_init_early(struct drm_i915_private
*dev_priv,
*/
static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
{
+ i915_perf_fini(dev_priv);
i915_gem_load_cleanup(&dev_priv->drm);
i915_workqueues_cleanup(dev_priv);
}
@@ -2561,6 +2564,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl,
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM,
i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM,
i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl,
DRM_RENDER_ALLOW),
};
static struct drm_driver driver = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4735b417..e4cd322 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1785,6 +1785,84 @@ struct intel_wm_config {
bool sprites_scaled;
};
+struct i915_perf_stream;
+
+struct i915_perf_stream_ops {
+ /* Enables the collection of HW samples, either in response to
+* I915_PERF_IOCTL_ENABLE or implicitly called when stream is
+* opened without I915_PERF_FLAG_DISABLED.
+*/
+ void (*enable)(struct i915_perf_stream *stream);
+
+ /* Disables the collection of HW samples, either in response to
+* I915_PERF_IOCTL_DISABLE or implicitly called before
+* destroying the stream.
+*/
+ void (*disable)(struct i915_perf_stream *stream);
+
+ /* Return: true if any i915 perf records are ready to read()
+* for this stream.
+*/
+ bool (*can_read)(struct i915_perf_stream *stream);
+
+ /* Call poll_wait, passing a wait queue that will be woken
+* once there is something ready to read() for the stream
+*/
+ void (*poll_wait)(stru
[PATCH v9 00/11] Enable i915 perf stream for Haswell OA unit
Rebased and updated with more feedback from Sourab and Matt. In particular the patch that added the oa_min_timer_exponent sysctl parameter has now been replaced with one adding an oa_max_sample_rate parameter in Hz. This way userspace policy won't need to be tailored to different systems when gen9 OA is enabled where the exponents don't represent the same periods as for Haswell. - Robert Robert Bragg (11): drm/i915: Add i915 perf infrastructure drm/i915: rename OACONTROL GEN7_OACONTROL drm/i915: return EACCES for check_cmd() failures drm/i915: don't whitelist oacontrol in cmd parser drm/i915: Add 'render basic' Haswell OA unit config drm/i915: Enable i915 perf stream for Haswell OA unit drm/i915: advertise available metrics via sysfs drm/i915: Add dev.i915.perf_stream_paranoid sysctl option drm/i915: add dev.i915.oa_max_sample_rate sysctl drm/i915: Add more Haswell OA metric sets drm/i915: Add a kerneldoc summary for i915_perf.c drivers/gpu/drm/i915/Makefile |4 + drivers/gpu/drm/i915/gvt/handlers.c|2 +- drivers/gpu/drm/i915/i915_cmd_parser.c | 45 +- drivers/gpu/drm/i915/i915_drv.c|9 + drivers/gpu/drm/i915/i915_drv.h| 156 +++ drivers/gpu/drm/i915/i915_oa_hsw.c | 752 ++ drivers/gpu/drm/i915/i915_oa_hsw.h | 38 + drivers/gpu/drm/i915/i915_perf.c | 1753 drivers/gpu/drm/i915/i915_reg.h| 340 ++- include/uapi/drm/i915_drm.h| 134 +++ 10 files changed, 3193 insertions(+), 40 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_oa_hsw.c create mode 100644 drivers/gpu/drm/i915/i915_oa_hsw.h create mode 100644 drivers/gpu/drm/i915/i915_perf.c -- 2.10.1
v4.9-rc3: radeon oops on shutdown
Hi! On old thinkpad T40p... with v4.9-rc3+ I'm getting radeon oops on shutdown. Seems repeatable. Unable to handle kernel NULL pointer dereference at 78c. IP: .. radeon_connector_unregister ... trace: ? drm_connector_unregister.part.5 drm_connector_unregister_all Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 181 bytes Desc: Digital signature URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/b8cc6823/attachment.sig>
[PATCH v2] drm: move allocation out of drm_get_format_name()
On Mon, 07 Nov 2016, Eric Engestrom wrote:
> On Monday, 2016-11-07 10:10:13 +0200, Jani Nikula wrote:
>> On Mon, 07 Nov 2016, Eric Engestrom wrote:
>> > Fixes: 90844f00049e9f42573fd31d7c32e8fd31d3fd07
>> >
>> > drm: make drm_get_format_name thread-safe
>> >
>> > Signed-off-by: Eric Engestrom
>> > [danvet: Clarify that the returned pointer must be freed with
>> > kfree().]
>> > Signed-off-by: Daniel Vetter
>>
>> The Fixes: format is:
>>
>> Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
>>
>> But is this a fix, really, or just an improvement? What exactly is the
>> bug being fixed? The commit message is not sufficient.
>
> "The function's behaviour was changed in 90844f00049e, without changing
> its signature, causing people to keep using it the old way without
> realising they were now leaking memory.
> Rob Clark also noticed it was also allocating GFP_KERNEL memory in
> atomic contexts, breaking them.
>
> Instead of having to allocate GFP_ATOMIC memory and fixing the callers
> to make them cleanup the memory afterwards, let's change the function's
> signature by having the caller take care of the memory and passing it to
> the function.
> The new parameter is a single-field struct in order to enforce the size
> of its buffer and help callers to correctly manage their memory."
>
> Does this sound good?
It's fine; no need to go overboard. ;)
BR,
Jani.
>
>> > @@ -54,6 +62,6 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
>> > int drm_format_vert_chroma_subsampling(uint32_t format);
>> > int drm_format_plane_width(int width, uint32_t format, int plane);
>> > int drm_format_plane_height(int height, uint32_t format, int plane);
>> > -char *drm_get_format_name(uint32_t format) __malloc;
>> > +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf
>> > *buf);
>>
>> I wonder if it would be better to make that return "const char *". If
>> the user really wants to look under the hood, there's buf->str. *shrug*
>
> Good idea, I'll do that in v3 with the proper commit msg and tags. It'll
> have to wait another day though, -ENOTIME and all.
>
>>
>> With the commit message improved,
>>
>> Reviewed-by: Jani Nikula
>
> Cheers :)
> Eric
--
Jani Nikula, Intel Open Source Technology Center
[PATCH v2 4/4] drm/plane: add inline doc for struct drm_plane
From: Gustavo Padovan
Some of the members of struct drm_plane had extra comments so for these
add inline kernel comment to consolidate all documentation in one place.
Signed-off-by: Gustavo Padovan
---
include/drm/drm_plane.h | 61 +++--
1 file changed, 49 insertions(+), 12 deletions(-)
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 68f6d22..683b170 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -32,11 +32,6 @@ struct drm_crtc;
/**
* struct drm_plane_state - mutable plane state
* @plane: backpointer to the plane
- * @crtc: currently bound CRTC, NULL if disabled
- * @fb: currently bound framebuffer
- * @fence: optional fence to wait for before scanning out @fb
- * @crtc_x: left position of visible portion of plane on crtc
- * @crtc_y: upper position of visible portion of plane on crtc
* @crtc_w: width of visible portion of plane on crtc
* @crtc_h: height of visible portion of plane on crtc
* @src_x: left position of visible portion of plane within
@@ -51,18 +46,56 @@ struct drm_crtc;
* where N is the number of active planes for given crtc
* @src: clipped source coordinates of the plane (in 16.16)
* @dst: clipped destination coordinates of the plane
- * @visible: visibility of the plane
* @state: backpointer to global drm_atomic_state
*/
struct drm_plane_state {
struct drm_plane *plane;
- struct drm_crtc *crtc; /* do not write directly, use
drm_atomic_set_crtc_for_plane() */
- struct drm_framebuffer *fb; /* do not write directly, use
drm_atomic_set_fb_for_plane() */
- struct dma_fence *fence; /* do not write directly, use
drm_atomic_set_fence_for_plane() */
+ /**
+* @crtc:
+*
+* currently bound CRTC, NULL if disabled
+*
+* do not write directly, use drm_atomic_set_crtc_for_plane()
+*/
+ struct drm_crtc *crtc;
+
+ /**
+* @fb:
+*
+* currently bound framebuffer
+*
+* do not write directly, use drm_atomic_set_fb_for_plane()
+*/
+ struct drm_framebuffer *fb;
+
+ /**
+* @fence:
+*
+* optional fence to wait for before scanning out @fb
+*
+* do not write directly, use drm_atomic_set_fence_for_plane()
+*/
+ struct dma_fence *fence;
+
+ /**
+* @crtc_x:
+*
+* left position of visible portion of plane on crtc
+*
+* Signed dest location allows it to be partially off screen.
+*/
+
+ int32_t crtc_x;
+ /**
+* @crtc_y:
+*
+* upper position of visible portion of plane on crtc
+*
+* Signed dest location allows it to be partially off screen.
+*/
+ int32_t crtc_y;
- /* Signed dest location allows it to be partially off screen */
- int32_t crtc_x, crtc_y;
uint32_t crtc_w, crtc_h;
/* Source values are 16.16 fixed point */
@@ -79,7 +112,11 @@ struct drm_plane_state {
/* Clipped coordinates */
struct drm_rect src, dst;
- /*
+ /**
+* @visible:
+*
+* visibility of the plane
+*
* Is the plane actually visible? Can be false even
* if fb!=NULL and crtc!=NULL, due to clipping.
*/
--
2.5.5
[PATCH v2 3/4] drm/msm: use drm_atomic_set_fence_for_plane() to set the fence
From: Gustavo Padovan
drm_atomic_set_fence_for_plane() is smart and won't overwrite
plane_state->fence if the user already set an explicit fence there.
Cc: Rob Clark
Signed-off-by: Gustavo Padovan
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/msm/msm_atomic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index db193f8..4e21e1d 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -217,8 +217,9 @@ int msm_atomic_commit(struct drm_device *dev,
if ((plane->state->fb != plane_state->fb) && plane_state->fb) {
struct drm_gem_object *obj =
msm_framebuffer_bo(plane_state->fb, 0);
struct msm_gem_object *msm_obj = to_msm_bo(obj);
+ struct dma_fence *fence =
reservation_object_get_excl_rcu(msm_obj->resv);
- plane_state->fence =
reservation_object_get_excl_rcu(msm_obj->resv);
+ drm_atomic_set_fence_for_plane(plane_state, fence);
}
}
--
2.5.5
[PATCH v2 2/4] drm/imx: use drm_atomic_set_fence_for_plane() to set the fence
From: Gustavo Padovan drm_atomic_set_fence_for_plane() is smart and won't overwrite plane_state->fence if the user already set an explicit fence there. Cc: Philipp Zabel Signed-off-by: Gustavo Padovan Reviewed-by: Daniel Vetter --- drivers/gpu/drm/imx/imx-drm-core.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 98df09c..07fe955 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -158,6 +158,7 @@ static int imx_drm_atomic_commit(struct drm_device *dev, struct drm_plane_state *plane_state; struct drm_plane *plane; struct dma_buf *dma_buf; + struct dma_fence *fence; int i; /* @@ -170,8 +171,9 @@ static int imx_drm_atomic_commit(struct drm_device *dev, 0)->base.dma_buf; if (!dma_buf) continue; - plane_state->fence = - reservation_object_get_excl_rcu(dma_buf->resv); + fence = reservation_object_get_excl_rcu(dma_buf->resv); + + drm_atomic_set_fence_for_plane(plane_state, fence); } } -- 2.5.5
[PATCH v2 1/4] drm/atomic: add drm_atomic_set_fence_for_plane()
From: Gustavo Padovan
This new function should be used by drivers when setting a implicit
fence for the plane. It abstracts the fact that the user might have
chosen explicit fencing instead.
Signed-off-by: Gustavo Padovan
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/drm_atomic.c | 30 ++
include/drm/drm_atomic.h | 2 ++
include/drm/drm_plane.h | 2 +-
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index c32fb3c..5e73954 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1147,6 +1147,36 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state
*plane_state,
EXPORT_SYMBOL(drm_atomic_set_fb_for_plane);
/**
+ * drm_atomic_set_fence_for_plane - set fence for plane
+ * @plane_state: atomic state object for the plane
+ * @fence: dma_fence to use for the plane
+ *
+ * Helper to setup the plane_state fence in case it is not set yet.
+ * By using this drivers doesn't need to worry if the user choose
+ * implicit or explicit fencing.
+ *
+ * This function will not set the fence to the state if it was set
+ * via explicit fencing interfaces on the atomic ioctl. It will
+ * all drope the reference to the fence as we not storing it
+ * anywhere.
+ *
+ * Otherwise, if plane_state->fence is not set this function we
+ * just set it with the received implict fence.
+ */
+void
+drm_atomic_set_fence_for_plane(struct drm_plane_state *plane_state,
+ struct dma_fence *fence)
+{
+ if (plane_state->fence) {
+ dma_fence_put(fence);
+ return;
+ }
+
+ plane_state->fence = fence;
+}
+EXPORT_SYMBOL(drm_atomic_set_fence_for_plane);
+
+/**
* drm_atomic_set_crtc_for_connector - set crtc for connector
* @conn_state: atomic state object for the connector
* @crtc: crtc to use for the connector
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index fc8af53..2d1e9c9 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -345,6 +345,8 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state
*plane_state,
struct drm_crtc *crtc);
void drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,
struct drm_framebuffer *fb);
+void drm_atomic_set_fence_for_plane(struct drm_plane_state *plane_state,
+ struct dma_fence *fence);
int __must_check
drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
struct drm_crtc *crtc);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index c5e8a0d..68f6d22 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -59,7 +59,7 @@ struct drm_plane_state {
struct drm_crtc *crtc; /* do not write directly, use
drm_atomic_set_crtc_for_plane() */
struct drm_framebuffer *fb; /* do not write directly, use
drm_atomic_set_fb_for_plane() */
- struct dma_fence *fence;
+ struct dma_fence *fence; /* do not write directly, use
drm_atomic_set_fence_for_plane() */
/* Signed dest location allows it to be partially off screen */
int32_t crtc_x, crtc_y;
--
2.5.5
[Intel-gfx] [PATCH] drm/dp: Make space for null terminator in the DP device ID char array
Mika, Can you take a look at this? -DK On Fri, 2016-11-04 at 14:06 -0700, Dhinakaran Pandiyan wrote: > The DP device identification string read from the DPCD registers is 6 > characters long at max. and we store it in a char array of the same length > without space for the NULL terminator. Fix this by increasing the array > size to 7 and initialize it to an empty string. > > Signed-off-by: Dhinakaran Pandiyan > --- > drivers/gpu/drm/drm_dp_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index 3e6fe82..3a39312 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -544,7 +544,7 @@ void drm_dp_downstream_debug(struct seq_file *m, >DP_DETAILED_CAP_INFO_AVAILABLE; > int clk; > int bpc; > - char id[6]; > + char id[7] = ""; > int len; > uint8_t rev[2]; > int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
[Bug 98627] mesa doesn't build since llvm r286062
https://bugs.freedesktop.org/show_bug.cgi?id=98627 --- Comment #1 from Tom Stellard --- This patch fixes gallivm/radeonsi: https://lists.freedesktop.org/archives/mesa-dev/2016-November/134500.html radv will need similar changes. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/9ebb0be4/attachment.html>
[Bug 98505] [Topaz] Regression introduces in 4.8-rc3
https://bugs.freedesktop.org/show_bug.cgi?id=98505 --- Comment #17 from Alex Deucher --- (In reply to Peter Wu from comment #16) > Ok, do you have any certainty about the earliest BIOS date where _PR3 is > used? E.g. if the minimum date is lowered to 2014 without checking for _PR3, > would it be likely to miss out some models? It's always used if it's available. I think 2014 should be ok (should catch most of the early ones), but I'll check with our windows architects. I suspect it will come down to OEM/ODMs. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/643085ed/attachment.html>
v4.9-rc3: radeon oops on shutdown
> -Original Message- > From: Pavel Machek [mailto:pavel at ucw.cz] > Sent: Monday, November 07, 2016 1:40 PM > To: Deucher, Alexander; Koenig, Christian; dri-devel at lists.freedesktop.org; > kernel list > Subject: v4.9-rc3: radeon oops on shutdown > > Hi! > > On old thinkpad T40p... with v4.9-rc3+ I'm getting radeon oops on > shutdown. Seems repeatable. > > Unable to handle kernel NULL pointer dereference at 78c. > IP: .. radeon_connector_unregister > ... > trace: > ? drm_connector_unregister.part.5 > drm_connector_unregister_all > Already fixed: https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-fixes&id=0a6e21056eaa859353945c4b164f3ef574d84271 Alex
[PATCH] drm: panel: simple-panel: get the enable gpio as-is
On Sun, Nov 6, 2016 at 7:09 PM, Icenowy Zheng wrote:
> The enable gpio of simple-panel may be used by a simplefb or other
> driver on the panel's display before the KMS driver get load.
>
> Get the GPIO as-is, so the panel won't be disabled, and the simplefb
> can work.
>
> Signed-off-by: Icenowy Zheng
> ---
> drivers/gpu/drm/panel/panel-simple.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 113db3c..ccee4c1 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -312,7 +312,7 @@ static int panel_simple_probe(struct device *dev, const
> struct panel_desc *desc)
> return PTR_ERR(panel->supply);
>
> panel->enable_gpio = devm_gpiod_get_optional(dev, "enable",
> -GPIOD_OUT_LOW);
> +GPIOD_ASIS);
The GPIO requested as-is might be in input mode. You should change the
gpiod_set_value calls to gpiod_direction_output calls. The later also
allows you to give an initial value. Not sure if it checks for cansleep
like the set_value calls though.
Regards
ChenYu
> if (IS_ERR(panel->enable_gpio)) {
> err = PTR_ERR(panel->enable_gpio);
> dev_err(dev, "failed to request GPIO: %d\n", err);
> --
> 2.10.1
>
[v17 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
On 05/11/16 00:21, Daniel Kurtz wrote: > On Tue, Oct 25, 2016 at 6:23 AM, Matthias Brugger > wrote: >> >> On 10/18/2016 04:37 PM, Enric Balletbo Serra wrote: >> [...] --- /dev/null +++ b/drivers/gpu/drm/bridge/parade-ps8640.c >> [...] + +/* Firmware */ +#define PS_FW_NAME "ps864x_fw.bin" + >>> >>> From where I can download this firmware image? >> >> I suppose this FW bits have to be added to linux-firmware repository first, >> before this patch can be accepted. > > All PS8640 devices should already ship with working firmware. > The firmware update procedure is only used in the unlikely event where > one wants to update the bridge to a different firmware provided by > Parade. > > Why must the lack of firmware really block landing this driver? > > If this is really so, can we just land the functional part of the > driver first, and add the firmware update in a follow-up patch. > After checking other users of request_firmware and check them against linux-firmware I think we don't need the FW in linux-firmware to get the driver merged. Especially as there already is a working FW stored on the device. Regards, Matthias
[Spice-devel] [drm/qxl v2 7/7] qxl: Allow resolution which are not multiple of 8
On 4 November 2016 at 20:41, Christophe Fergeau wrote: > On Thu, Nov 03, 2016 at 06:08:39PM +0100, Gerd Hoffmann wrote: >> > Or maybe other parts of the >> > kernel/userspace rely on this rounding down. >> >> This is where I suspect we could run in trouble. Odd resolutions simply >> don't happen on physical hardware, all usual resolutions are a multiple >> of 8, most of them are even a multiple of 16. >> >> Various image/video formats use 16x16 blocks. >> The qemu vnc server operates on 16x16 blocks too (and we had bugs in the >> past with odd resolutions). >> >> Also scanlines and cachelines align nicely if you don't use odd >> resolutions. >> >> > I unfortunately don't know >> > :( >> >> I don't have definitive answers too, just a gut feeling that this might >> cause trouble. > > I think this might be fine actually, there is already one such > resolution in the kernel, which is 1366x768 (1366 is only a multiple of > 2). There is already a bit of a hack to handle it anyway, see > fixup_mode_1366x768() in drm_edid.c. > >> >> Maybe we should add a module option for this? So there is an easy >> (as-in: doesn't require a kernel rebuild) way out in case it causes >> trouble in certain setups? > > This seems a bit overkill to me, but I can look into it if needed. I think we should try it an see, if we see problems you could enforce the framebuffer would have a stride aligned to whatever value, and just the view into the framebuffer could be whatever. The CVT stuff is just due to how hw is programmed and monitors are described. Dave.
[PATCH] drm/exynos/hdmi: refactor infoframe code
2016ë
11ì 07ì¼ 17:05ì Andrzej Hajda ì´(ê°) ì´ ê¸:
> On 07.11.2016 02:45, Inki Dae wrote:
>>
>> 2016ë
10ì 26ì¼ 21:36ì Andrzej Hajda ì´(ê°) ì´ ê¸:
>>> Use core helpers to generate infoframes and generate vendor frame if
>>> necessary.
>>>
>>> Signed-off-by: Andrzej Hajda
>>> ---
>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 141
>>> ++-
>>> drivers/gpu/drm/exynos/regs-hdmi.h | 2 +
>>> 2 files changed, 42 insertions(+), 101 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> index e8fb6ef..1bb2df7 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> @@ -47,19 +47,6 @@
>>>
>>> #define HOTPLUG_DEBOUNCE_MS1100
>>>
>>> -/* AVI header and aspect ratio */
>>> -#define HDMI_AVI_VERSION 0x02
>>> -#define HDMI_AVI_LENGTH0x0d
>>> -
>>> -/* AUI header info */
>>> -#define HDMI_AUI_VERSION 0x01
>>> -#define HDMI_AUI_LENGTH0x0a
>>> -
>>> -/* AVI active format aspect ratio */
>>> -#define AVI_SAME_AS_PIC_ASPECT_RATIO 0x08
>>> -#define AVI_4_3_CENTER_RATIO 0x09
>>> -#define AVI_16_9_CENTER_RATIO 0x0a
>>> -
>>> enum hdmi_type {
>>> HDMI_TYPE13,
>>> HDMI_TYPE14,
>>> @@ -131,7 +118,6 @@ struct hdmi_context {
>>> booldvi_mode;
>>> struct delayed_work hotplug_work;
>>> struct drm_display_mode current_mode;
>>> - u8 cea_video_id;
>>> const struct hdmi_driver_data *drv_data;
>>>
>>> void __iomem*regs;
>>> @@ -681,6 +667,13 @@ static inline void hdmi_reg_writev(struct hdmi_context
>>> *hdata, u32 reg_id,
>>> }
>>> }
>>>
>>> +static inline void hdmi_reg_write_buf(struct hdmi_context *hdata, u32
>>> reg_id,
>>> + u8 *buf, int size)
>>> +{
>>> + for (reg_id = hdmi_map_reg(hdata, reg_id); size; --size, reg_id += 4)
>>> + writel(*buf++, hdata->regs + reg_id);
>>> +}
>>> +
>>> static inline void hdmi_reg_writemask(struct hdmi_context *hdata,
>>> u32 reg_id, u32 value, u32 mask)
>>> {
>>> @@ -762,93 +755,50 @@ static int hdmi_clk_set_parents(struct hdmi_context
>>> *hdata, bool to_phy)
>>> return ret;
>>> }
>>>
>>> -static u8 hdmi_chksum(struct hdmi_context *hdata,
>>> - u32 start, u8 len, u32 hdr_sum)
>>> -{
>>> - int i;
>>> -
>>> - /* hdr_sum : header0 + header1 + header2
>>> - * start : start address of packet byte1
>>> - * len : packet bytes - 1 */
>>> - for (i = 0; i < len; ++i)
>>> - hdr_sum += 0xff & hdmi_reg_read(hdata, start + i * 4);
>>> -
>>> - /* return 2's complement of 8 bit hdr_sum */
>>> - return (u8)(~(hdr_sum & 0xff) + 1);
>>> -}
>>> -
>>> -static void hdmi_reg_infoframe(struct hdmi_context *hdata,
>>> - union hdmi_infoframe *infoframe)
>>> +static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>>> {
>>> - u32 hdr_sum;
>>> - u8 chksum;
>>> - u8 ar;
>>> + union hdmi_infoframe frm;
>>> + u8 buf[25];
>>> + int ret;
>>>
>>> if (hdata->dvi_mode) {
>>> - hdmi_reg_writeb(hdata, HDMI_VSI_CON,
>>> - HDMI_VSI_CON_DO_NOT_TRANSMIT);
>>> hdmi_reg_writeb(hdata, HDMI_AVI_CON,
>>> HDMI_AVI_CON_DO_NOT_TRANSMIT);
>>> + hdmi_reg_writeb(hdata, HDMI_VSI_CON,
>>> + HDMI_VSI_CON_DO_NOT_TRANSMIT);
>>> hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_NO_TRAN);
>>> return;
>>> }
>>>
>>> - switch (infoframe->any.type) {
>>> - case HDMI_INFOFRAME_TYPE_AVI:
>>> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
>>> + &hdata->current_mode);
>>> + if (ret >= 0)
>> Seems above condition is not clear becuase
>> drm_hdmi_avi_infoframe_from_display_mode function returns 0 or negative
>> value.
>
> So it means 'there is no error', I can change it to '!ret' or 'ret == 0'
> if you prefer, I have just used '>= 0' to be more concise with next
> error check.
As I mentioned, never return bigger than 0. Let's change it to !ret or ret == 0.
>>
>>> + ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
>>> + if (ret > 0) {
>>> hdmi_reg_writeb(hdata, HDMI_AVI_CON, HDMI_AVI_CON_EVERY_VSYNC);
>> I think above code should be called when
>> drm_hdmi_avi_infoframe_from_display_mode function returned 0 and the value
>> from hdmi_avi_infoframe_pack function is more than 1.
>
> Why do you think 'more than 1' is better than 'more than 0' in this
> case? hdmi_avi_infoframe_pack returns length of generated frame or
> negative value in case of error,
> so any positive value is OK, there is no special meaning for '1'.
Because hdmi_avi_infoframe_pack returns 0 or bigger than
[PATCH v2] drm: move allocation out of drm_get_format_name()
On Monday, 2016-11-07 10:10:13 +0200, Jani Nikula wrote:
> On Mon, 07 Nov 2016, Eric Engestrom wrote:
> > Fixes: 90844f00049e9f42573fd31d7c32e8fd31d3fd07
> >
> > drm: make drm_get_format_name thread-safe
> >
> > Signed-off-by: Eric Engestrom
> > [danvet: Clarify that the returned pointer must be freed with
> > kfree().]
> > Signed-off-by: Daniel Vetter
>
> The Fixes: format is:
>
> Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
>
> But is this a fix, really, or just an improvement? What exactly is the
> bug being fixed? The commit message is not sufficient.
"The function's behaviour was changed in 90844f00049e, without changing
its signature, causing people to keep using it the old way without
realising they were now leaking memory.
Rob Clark also noticed it was also allocating GFP_KERNEL memory in
atomic contexts, breaking them.
Instead of having to allocate GFP_ATOMIC memory and fixing the callers
to make them cleanup the memory afterwards, let's change the function's
signature by having the caller take care of the memory and passing it to
the function.
The new parameter is a single-field struct in order to enforce the size
of its buffer and help callers to correctly manage their memory."
Does this sound good?
> > @@ -54,6 +62,6 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
> > int drm_format_vert_chroma_subsampling(uint32_t format);
> > int drm_format_plane_width(int width, uint32_t format, int plane);
> > int drm_format_plane_height(int height, uint32_t format, int plane);
> > -char *drm_get_format_name(uint32_t format) __malloc;
> > +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf
> > *buf);
>
> I wonder if it would be better to make that return "const char *". If
> the user really wants to look under the hood, there's buf->str. *shrug*
Good idea, I'll do that in v3 with the proper commit msg and tags. It'll
have to wait another day though, -ENOTIME and all.
>
> With the commit message improved,
>
> Reviewed-by: Jani Nikula
Cheers :)
Eric
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On 7 November 2016 at 15:48, Sharma, Shashank wrote: > Regards > > Shashank > > > On 11/7/2016 8:56 PM, Emil Velikov wrote: >> >> On 7 November 2016 at 07:43, Sharma, Shashank >> wrote: >>> >>> If I was not very clear for the first time, every time we send a patch to >>> drm-intel/dri-devel, we do basic testing on Gnome-desktop too (Not only >>> Android). >>> >>> So even these aspect ratio patches were tested with full gnome-desktop, >>> and >>> it worked well. >>> >>> >> I guess the part that's not so obvious is that Linux >> desktop/distributions provide a very wide permutation of components >> and configs used. While on a local (test) setup only a small/limited >> set is available. Esp. on Android where things are _very_ tightly >> coupled. > > I agree, Emil. > I was only mentioning my testing with Gnome, to confirm that intel-ddx is > not broken with Gnome desktop. > And I was testing on both Android as well as X. >> >> Obviously nobody likes when we have to carry kernel patches which >> workaround "broken" userspace, but it's a kernel policy and we all >> have to live with it. That's the main reason people are so >> careful/pedantic when it comes to UABI. And as you can see even then >> there are bits that we miss :'-( > > I agree, again. But I was thinking if reverting the patch was the best way, > else it would be impossible to > add something new in the kernel. I hope experts like you and others can > suggest the right way. Afaict, Daniel Vetter and Ville mentioned the best route - "1 week and then revert is the guideline" and "... with a new client cap ..." (grep for DRM_CAP_) respectively. It the former is missing from the documentation, so feel free to point out where you'd expect it to be. Or even better send a patch describing it based on your experience. Things explained form your POV would read better (and be easier to find) for people unfamiliar with the topic. Thanks Emil
[Bug 98627] mesa doesn't build since llvm r286062
or: 'LLVMReadNoneAttribute' undeclared (first use in this function) params, 10, LLVMReadNoneAttribute); ^ ac_nir_to_llvm.c: In function 'visit_interp': ac_nir_to_llvm.c:2817:10: error: 'LLVMReadNoneAttribute' undeclared (first use in this function) LLVMReadNoneAttribute); ^ ac_nir_to_llvm.c: In function 'build_cube_intrinsic': ac_nir_to_llvm.c:3112:23: error: 'LLVMReadNoneAttribute' undeclared (first use in this function) f32, in, 3, LLVMReadNoneAttribute); ^ ac_nir_to_llvm.c: In function 'cube_to_2d_coords': ac_nir_to_llvm.c:3149:21: error: 'LLVMReadNoneAttribute' undeclared (first use in this function) &coords[2], 1, LLVMReadNoneAttribute); ^ ac_nir_to_llvm.c: In function 'visit_tex': ac_nir_to_llvm.c:3376:15: warning: assignment makes pointer from integer without a cast [-Wint-conversion] coords[2] = emit_llvm_intrinsic(ctx, "llvm.rint.f32", ctx->f32, &coords[2], ^ ac_nir_to_llvm.c: In function 'handle_vs_input_decl': ac_nir_to_llvm.c:3754:4: error: 'LLVMReadNoneAttribute' undeclared (first use in this function) LLVMReadNoneAttribute | LLVMNoUnwindAttribute); ^ ac_nir_to_llvm.c:3754:28: error: 'LLVMNoUnwindAttribute' undeclared (first use in this function) LLVMReadNoneAttribute | LLVMNoUnwindAttribute); ^ ac_nir_to_llvm.c: In function 'interp_fs_input': ac_nir_to_llvm.c:3800:9: error: 'LLVMReadNoneAttribute' undeclared (first use in this function) LLVMReadNoneAttribute | LLVMNoUnwindAttribute); ^ ac_nir_to_llvm.c:3800:33: error: 'LLVMNoUnwindAttribute' undeclared (first use in this function) LLVMReadNoneAttribute | LLVMNoUnwindAttribute); ^ ac_nir_to_llvm.c: In function 'si_llvm_init_export_args': ac_nir_to_llvm.c:4059:13: error: 'LLVMReadNoneAttribute' undeclared (first use in this function) LLVMReadNoneAttribute); ^ ac_nir_to_llvm.c: In function 'emit_intrin_1f_param': ac_nir_to_llvm.c:712:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ ac_nir_to_llvm.c: In function 'emit_intrin_2f_param': ac_nir_to_llvm.c:723:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ ac_nir_to_llvm.c: In function 'emit_intrin_3f_param': ac_nir_to_llvm.c:735:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ ac_nir_to_llvm.c: In function 'emit_find_lsb': ac_nir_to_llvm.c:761:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ ac_nir_to_llvm.c: In function 'build_tex_intrinsic': ac_nir_to_llvm.c:1777:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ ac_nir_to_llvm.c: In function 'radv_lower_gather4_integer': ac_nir_to_llvm.c:1709:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ cc1: some warnings being treated as errors make[3]: *** [Makefile:667: ac_nir_to_llvm.lo] Error 1 make[3]: *** Waiting for unfinished jobs make[3]: Leaving directory '/build/mesa-git/src/mesa/src/amd/common' make[2]: *** [Makefile:867: all-recursive] Error 1 make[2]: Leaving directory '/build/mesa-git/src/mesa/src' make[1]: *** [Makefile:658: all] Error 2 make[1]: Leaving directory '/build/mesa-git/src/mesa/src' make: *** [Makefile:651: all-recursive] Error 1 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/0bea6eff/attachment-0001.html>
Enable AMDGPU for CIK by default
On 07/11/16 04:24 PM, Michel Dänzer wrote: > On 07/11/16 03:56 AM, Sandeep wrote: >> Hello, >> >> I was wondering when DRM_AMDGPU_CIK would be turned on by default in the >> upstream kernel (or is this upto individual distros?) >> >> Is there any work left to be done/bugs to be fixed before it can be >> enabled by default? > > There are still some functional regressions, notably amdgpu doesn't > support HDMI/DP audio yet. > > Also, simply enabling DRM_AMDGPU_CIK by default isn't a good solution, > since the radeon driver also still supports the CIK devices, and there's > no good mechanism to choose which driver gets to drive a particular GPU > at runtime. Moreover, amdgpu doesn't work with my Kaveri laptops in my attempts so far â the panel doesn't even light up. Haven't had a chance to dig into what's happening there. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Enable AMDGPU for CIK by default
On 07/11/16 03:56 AM, Sandeep wrote: > Hello, > > I was wondering when DRM_AMDGPU_CIK would be turned on by default in the > upstream kernel (or is this upto individual distros?) > > Is there any work left to be done/bugs to be fixed before it can be > enabled by default? There are still some functional regressions, notably amdgpu doesn't support HDMI/DP audio yet. Also, simply enabling DRM_AMDGPU_CIK by default isn't a good solution, since the radeon driver also still supports the CIK devices, and there's no good mechanism to choose which driver gets to drive a particular GPU at runtime. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
[PATCH 4/4] reservation: revert "wait only with non-zero timeout specified (v3)" v2
From: Christian König
This reverts commit fb8b7d2b9d80e1e71f379e57355936bd2b024be9.
Otherwise signaling might never be activated on the fences. This can
result in infinite waiting with hardware which has unreliable interrupts.
v2: still return one when the timeout is zero and we don't have any fences.
Reviewed-by: Alex Deucher
Signed-off-by: Christian König
Reviewed-by: Chunming Zhou (v1)
Signed-off-by: Alex Deucher
---
drivers/dma-buf/reservation.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 7ed56f3..393817e 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -370,10 +370,7 @@ long reservation_object_wait_timeout_rcu(struct
reservation_object *obj,
{
struct dma_fence *fence;
unsigned seq, shared_count, i = 0;
- long ret = timeout;
-
- if (!timeout)
- return reservation_object_test_signaled_rcu(obj, wait_all);
+ long ret = timeout ? timeout : 1;
retry:
fence = NULL;
--
2.5.5
[PATCH 3/4] drm/ttm: fix ttm_bo_wait
From: Christian König
reservation_object_wait_timeout_rcu() should enable signaling even with a zero
timeout, but ttm_bo_wait() can also be called from atomic context and then it
is not a good idea to do this.
Reviewed-by: Alex Deucher
Signed-off-by: Christian König
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/ttm/ttm_bo.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index f6ff579..d506361 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1611,7 +1611,14 @@ EXPORT_SYMBOL(ttm_bo_unmap_virtual);
int ttm_bo_wait(struct ttm_buffer_object *bo,
bool interruptible, bool no_wait)
{
- long timeout = no_wait ? 0 : 15 * HZ;
+ long timeout = 15 * HZ;
+
+ if (no_wait) {
+ if (reservation_object_test_signaled_rcu(bo->resv, true))
+ return 0;
+ else
+ return -EBUSY;
+ }
timeout = reservation_object_wait_timeout_rcu(bo->resv, true,
interruptible, timeout);
--
2.5.5
[PATCH 2/4] dma-buf/fence: revert "don't wait when specified timeout is zero" (v2)
This reverts commit 847b19a39e4c9b5e74c40f0842c48b41664cb43c. When we don't call the wait function software signaling might never be activated. This can cause infinite polling loops with unreliable interrupt driven hardware. v2: rebase on drm-next Reviewed-by: Alex Deucher Signed-off-by: Christian König Reviewed-by: Chunming Zhou Signed-off-by: Alex Deucher --- drivers/dma-buf/dma-fence.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 6c3f6b4..dd00990 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -161,9 +161,6 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) if (WARN_ON(timeout < 0)) return -EINVAL; - if (timeout == 0) - return dma_fence_is_signaled(fence); - trace_dma_fence_wait_start(fence); ret = fence->ops->wait(fence, intr, timeout); trace_dma_fence_wait_end(fence); -- 2.5.5
[PATCH 1/4] dma-buf/fence: make timeout handling in fence_default_wait consistent (v2)
Kernel functions taking a timeout usually return 1 on success even
when they get a zero timeout.
v2: agd: rebase on drm-next
Reviewed-by: Alex Deucher
Signen-off-by: Christian König
Reviewed-by: Chunming Zhou
Signed-off-by: Alex Deucher
---
These are the same patches Christian sent out previously, just
rebased on the fence naming changes in drm-next.
drivers/dma-buf/dma-fence.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 0c3141e7..6c3f6b4 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -339,18 +339,20 @@ dma_fence_default_wait_cb(struct dma_fence *fence, struct
dma_fence_cb *cb)
* @timeout: [in]timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
*
* Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
- * remaining timeout in jiffies on success.
+ * remaining timeout in jiffies on success. If timeout is zero the value one is
+ * returned if the fence is already signaled for consistency with other
+ * functions taking a jiffies timeout.
*/
signed long
dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
{
struct default_wait_cb cb;
unsigned long flags;
- signed long ret = timeout;
+ signed long ret = timeout ? timeout : 1;
bool was_set;
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
- return timeout;
+ return ret;
spin_lock_irqsave(fence->lock, flags);
--
2.5.5
[PATCH] drm/sun4i: Propagate error to the caller
On Mon, Nov 07, 2016 at 10:19:57AM +0900, Gustavo Padovan wrote: > Hi Christophe, > > 2016-11-04 Christophe JAILLET : > > > If 'sun4i_layers_init()' returns an error, propagate it instead of > > returning -EINVAL unconditionally. > > > > Signed-off-by: Christophe JAILLET > > --- > > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Reviewed-by: Gustavo Padovan Applied, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/310f8f7b/attachment.sig>
[PATCH 0/5] drm/sun4i: Handle TV overscan
ge it. > > The problem is that TVs are at liberty to have this behaviour, so > the overscaned problem is always going to be there, and each driver > should not be dealing with it in their own specific ways. I agree with you, however, without any directions on how to do this, and willingness to merge this, I don't really see why we would work on such a generic implementation in the first place. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/8a156bb1/attachment.sig>
[PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe
On 11/05/2016 09:55 PM, Rob Clark wrote:
> Split out the hardware pipe specifics from mdp5_plane. To start, the hw
> pipes are statically assigned to planes, but next step is to assign the
> hw pipes during plane->atomic_check() based on requested caps (scaling,
> YUV, etc). And then hw pipe re-assignment if required if required SMP
> blocks changes.
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 126
> +++---
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 7 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c | 43 ++
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h | 39 +
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 66 +++-
> 6 files changed, 197 insertions(+), 85 deletions(-)
> create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
> create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index fb5be3e..90f66c4 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -37,6 +37,7 @@ msm-y := \
> mdp/mdp5/mdp5_irq.o \
> mdp/mdp5/mdp5_mdss.o \
> mdp/mdp5/mdp5_kms.o \
> + mdp/mdp5/mdp5_pipe.o \
> mdp/mdp5/mdp5_plane.o \
> mdp/mdp5/mdp5_smp.o \
> msm_atomic.o \
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> index f1288c7..d3d45ed 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> @@ -119,6 +119,10 @@ static void mdp5_kms_destroy(struct msm_kms *kms)
> {
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> struct msm_gem_address_space *aspace = mdp5_kms->aspace;
> + int i;
> +
> + for (i = 0; i < mdp5_kms->num_hwpipes; i++)
> + mdp5_pipe_destroy(mdp5_kms->hwpipes[i]);
>
> if (aspace) {
> aspace->mmu->funcs->detach(aspace->mmu,
> @@ -323,15 +327,6 @@ static int modeset_init_intf(struct mdp5_kms *mdp5_kms,
> int intf_num)
>
> static int modeset_init(struct mdp5_kms *mdp5_kms)
> {
> - static const enum mdp5_pipe rgb_planes[] = {
> - SSPP_RGB0, SSPP_RGB1, SSPP_RGB2, SSPP_RGB3,
> - };
> - static const enum mdp5_pipe vig_planes[] = {
> - SSPP_VIG0, SSPP_VIG1, SSPP_VIG2, SSPP_VIG3,
> - };
> - static const enum mdp5_pipe dma_planes[] = {
> - SSPP_DMA0, SSPP_DMA1,
> - };
> struct drm_device *dev = mdp5_kms->dev;
> struct msm_drm_private *priv = dev->dev_private;
> const struct mdp5_cfg_hw *hw_cfg;
> @@ -339,58 +334,34 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
>
> hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>
> - /* construct CRTCs and their private planes: */
> - for (i = 0; i < hw_cfg->pipe_rgb.count; i++) {
> + /* Construct planes equaling the number of hw pipes, and CRTCs
> + * for the N layer-mixers (LM). The first N planes become primary
> + * planes for the CRTCs, with the remainder as overlay planes:
> + */
Jfyi, we might need to change this a bit in the future. It'll be better to
get the max number of displays connected on our platform via parsing DT,
etc, and calculate CRTCs based on that, and not number of layermixers. Maybe
add a couple for writeback too. This way, we get the right number of
CRTCs, and we don't rely on #LMs, since we can have 2 per crtc
in the future.
Reviewed-by: Archit Taneja
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH] drm/exynos/hdmi: refactor infoframe code
Use core helpers to generate infoframes and generate vendor frame if necessary.
Signed-off-by: Andrzej Hajda
---
- changed 'ret >= 0' checks to '!ret'
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 141 ++-
drivers/gpu/drm/exynos/regs-hdmi.h | 2 +
2 files changed, 42 insertions(+), 101 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index e8fb6ef..e39d557 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -47,19 +47,6 @@
#define HOTPLUG_DEBOUNCE_MS1100
-/* AVI header and aspect ratio */
-#define HDMI_AVI_VERSION 0x02
-#define HDMI_AVI_LENGTH0x0d
-
-/* AUI header info */
-#define HDMI_AUI_VERSION 0x01
-#define HDMI_AUI_LENGTH0x0a
-
-/* AVI active format aspect ratio */
-#define AVI_SAME_AS_PIC_ASPECT_RATIO 0x08
-#define AVI_4_3_CENTER_RATIO 0x09
-#define AVI_16_9_CENTER_RATIO 0x0a
-
enum hdmi_type {
HDMI_TYPE13,
HDMI_TYPE14,
@@ -131,7 +118,6 @@ struct hdmi_context {
booldvi_mode;
struct delayed_work hotplug_work;
struct drm_display_mode current_mode;
- u8 cea_video_id;
const struct hdmi_driver_data *drv_data;
void __iomem*regs;
@@ -681,6 +667,13 @@ static inline void hdmi_reg_writev(struct hdmi_context
*hdata, u32 reg_id,
}
}
+static inline void hdmi_reg_write_buf(struct hdmi_context *hdata, u32 reg_id,
+ u8 *buf, int size)
+{
+ for (reg_id = hdmi_map_reg(hdata, reg_id); size; --size, reg_id += 4)
+ writel(*buf++, hdata->regs + reg_id);
+}
+
static inline void hdmi_reg_writemask(struct hdmi_context *hdata,
u32 reg_id, u32 value, u32 mask)
{
@@ -762,93 +755,50 @@ static int hdmi_clk_set_parents(struct hdmi_context
*hdata, bool to_phy)
return ret;
}
-static u8 hdmi_chksum(struct hdmi_context *hdata,
- u32 start, u8 len, u32 hdr_sum)
-{
- int i;
-
- /* hdr_sum : header0 + header1 + header2
- * start : start address of packet byte1
- * len : packet bytes - 1 */
- for (i = 0; i < len; ++i)
- hdr_sum += 0xff & hdmi_reg_read(hdata, start + i * 4);
-
- /* return 2's complement of 8 bit hdr_sum */
- return (u8)(~(hdr_sum & 0xff) + 1);
-}
-
-static void hdmi_reg_infoframe(struct hdmi_context *hdata,
- union hdmi_infoframe *infoframe)
+static void hdmi_reg_infoframes(struct hdmi_context *hdata)
{
- u32 hdr_sum;
- u8 chksum;
- u8 ar;
+ union hdmi_infoframe frm;
+ u8 buf[25];
+ int ret;
if (hdata->dvi_mode) {
- hdmi_reg_writeb(hdata, HDMI_VSI_CON,
- HDMI_VSI_CON_DO_NOT_TRANSMIT);
hdmi_reg_writeb(hdata, HDMI_AVI_CON,
HDMI_AVI_CON_DO_NOT_TRANSMIT);
+ hdmi_reg_writeb(hdata, HDMI_VSI_CON,
+ HDMI_VSI_CON_DO_NOT_TRANSMIT);
hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_NO_TRAN);
return;
}
- switch (infoframe->any.type) {
- case HDMI_INFOFRAME_TYPE_AVI:
+ ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
+ &hdata->current_mode);
+ if (!ret)
+ ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
+ if (ret > 0) {
hdmi_reg_writeb(hdata, HDMI_AVI_CON, HDMI_AVI_CON_EVERY_VSYNC);
- hdmi_reg_writeb(hdata, HDMI_AVI_HEADER0, infoframe->any.type);
- hdmi_reg_writeb(hdata, HDMI_AVI_HEADER1,
- infoframe->any.version);
- hdmi_reg_writeb(hdata, HDMI_AVI_HEADER2, infoframe->any.length);
- hdr_sum = infoframe->any.type + infoframe->any.version +
- infoframe->any.length;
-
- /* Output format zero hardcoded ,RGB YBCR selection */
- hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(1), 0 << 5 |
- AVI_ACTIVE_FORMAT_VALID |
- AVI_UNDERSCANNED_DISPLAY_VALID);
-
- /*
-* Set the aspect ratio as per the mode, mentioned in
-* Table 9 AVI InfoFrame Data Byte 2 of CEA-861-D Standard
-*/
- ar = hdata->current_mode.picture_aspect_ratio;
- switch (ar) {
- case HDMI_PICTURE_ASPECT_4_3:
- ar |= AVI_4_3_CENTER_RATIO;
- break;
- case HDMI_PICTURE_ASPECT_16_9:
- ar |= AVI_16_9_CENTER_RATIO;
- break;
- case HDMI_PICTURE_ASPECT_NONE:
- defau
[PATCH 4/5] drm/msm/mdp5: dynamically assign hw pipes to planes
Hi,
Minor comments below. LGTM otherwise.
On 11/05/2016 09:56 PM, Rob Clark wrote:
> (re)assign the hw pipes to planes based on required caps, and to handle
> situations where we could not modify an in-use plane (ie. SMP block
> reallocation).
>
> This means all planes advertise the superset of formats and properties.
> Userspace must (as always) use atomic TEST_ONLY step for atomic updates,
> as not all planes may be available for use on every frame.
>
> The mapping of hwpipe to plane is stored in mdp5_state, so that state
> updates are atomically committed in the same way that plane/etc state
> updates are managed. This is needed because the mdp5_plane_state keeps
> a pointer to the hwpipe, and we don't want global state to become out
> of sync with the plane state if an atomic update fails, we hit deadlock/
> backoff scenario, etc. The use of state_lock keeps multiple parallel
> updates which both re-assign hwpipes properly serialized.
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 2 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 4 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 7 +-
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c | 70 +
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h | 10 ++
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 158
> --
> 6 files changed, 171 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> index 6213f51..99958f7 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
> @@ -407,7 +407,7 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc,
> for (i = 0; i < cnt; i++) {
> pstates[i].state->stage = STAGE_BASE + i + base;
> DBG("%s: assign pipe %s on stage=%d", crtc->name,
> - pipe2name(mdp5_plane_pipe(pstates[i].plane)),
> + pstates[i].plane->name,
> pstates[i].state->stage);
> }
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> index ca6dfeb..3542adf 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> @@ -92,7 +92,7 @@ struct mdp5_state *mdp5_get_state(struct drm_atomic_state
> *s)
> return ERR_PTR(-ENOMEM);
>
> /* Copy state: */
> - /* TODO */
> + new_state->hwpipe = mdp5_kms->state->hwpipe;
>
> state->state = new_state;
>
> @@ -377,7 +377,7 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
> struct drm_plane *plane;
> struct drm_crtc *crtc;
>
> - plane = mdp5_plane_init(dev, mdp5_kms->hwpipes[i], primary);
> + plane = mdp5_plane_init(dev, primary);
> if (IS_ERR(plane)) {
> ret = PTR_ERR(plane);
> dev_err(dev->dev, "failed to construct plane %d
> (%d)\n", i, ret);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> index 52914ec..4475090 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> @@ -82,7 +82,7 @@ struct mdp5_kms {
> * For atomic updates which require modifying global state,
> */
> struct mdp5_state {
> - uint32_t dummy;
> + struct mdp5_hw_pipe_state hwpipe;
I was wondering if we could rename the above as hwpipes/hwpipe_state/hwpipe_map,
because it gets a confusing since variables of the type mdp5_hw_pipe are
also named hwpipe.
> };
>
> struct mdp5_state *__must_check
> @@ -94,6 +94,8 @@ mdp5_get_state(struct drm_atomic_state *s);
> struct mdp5_plane_state {
> struct drm_plane_state base;
>
> + struct mdp5_hw_pipe *hwpipe;
> +
> /* aligned with property */
> uint8_t premultiplied;
> uint8_t zpos;
> @@ -232,8 +234,7 @@ uint32_t mdp5_plane_get_flush(struct drm_plane *plane);
> void mdp5_plane_complete_commit(struct drm_plane *plane,
> struct drm_plane_state *state);
> enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane);
> -struct drm_plane *mdp5_plane_init(struct drm_device *dev,
> - struct mdp5_hw_pipe *hwpipe, bool primary);
> +struct drm_plane *mdp5_plane_init(struct drm_device *dev, bool primary);
>
> uint32_t mdp5_crtc_vblank(struct drm_crtc *crtc);
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
> index 7f3e8e50..115de7d 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
> @@ -17,6 +17,76 @@
>
> #include "mdp5_kms.h"
>
> +struct mdp5_hw_pipe *mdp5_pipe_assign(struct drm_atomic_state *s,
> + struct drm_plane *plane, uint32_t caps)
> +{
> + struct msm_drm_private *priv = s->dev->dev_private;
> + struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> +
[PATCH 3/5] drm/msm/mdp5: add skeletal mdp5_state
On 11/05/2016 09:55 PM, Rob Clark wrote:
> Add basic state duplication/apply mechanism. Following commits will
> move actual global hw state into this.
>
> The state_lock allows multiple concurrent updates to proceed as long as
> they don't both try to alter global state. The ww_mutex mechanism will
> trigger backoff in case of deadlock between multiple threads trying to
> update state.
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 43
> +
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 22 +
> 2 files changed, 65 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> index d3d45ed..ca6dfeb 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> @@ -72,6 +72,39 @@ static int mdp5_hw_init(struct msm_kms *kms)
> return 0;
> }
>
> +struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
> +{
> + struct msm_drm_private *priv = s->dev->dev_private;
> + struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> + struct msm_kms_state *state = to_kms_state(s);
> + struct mdp5_state *new_state;
> + int ret;
> +
> + if (state->state)
> + return state->state;
> +
> + ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
> + if (!new_state)
> + return ERR_PTR(-ENOMEM);
> +
> + /* Copy state: */
> + /* TODO */
> +
> + state->state = new_state;
> +
> + return new_state;
> +}
> +
> +static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state
> *state)
> +{
> + struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> + swap(to_kms_state(state)->state, mdp5_kms->state);
> +}
> +
> static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state
> *state)
> {
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> @@ -140,6 +173,7 @@ static const struct mdp_kms_funcs kms_funcs = {
> .irq = mdp5_irq,
> .enable_vblank = mdp5_enable_vblank,
> .disable_vblank = mdp5_disable_vblank,
> + .swap_state = mdp5_swap_state,
> .prepare_commit = mdp5_prepare_commit,
> .complete_commit = mdp5_complete_commit,
> .wait_for_crtc_commit_done = mdp5_wait_for_crtc_commit_done,
> @@ -645,6 +679,8 @@ static void mdp5_destroy(struct platform_device *pdev)
>
> if (mdp5_kms->rpm_enabled)
> pm_runtime_disable(&pdev->dev);
> +
> + kfree(mdp5_kms->state);
> }
>
> static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt,
> @@ -729,6 +765,13 @@ static int mdp5_init(struct platform_device *pdev,
> struct drm_device *dev)
> mdp5_kms->dev = dev;
> mdp5_kms->pdev = pdev;
>
> + drm_modeset_lock_init(&mdp5_kms->state_lock);
> + mdp5_kms->state = kzalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
> + if (!mdp5_kms->state) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
This would probably be better in mdp5_kms_init() since it's intializing kms
stuff, and
not hw resources. Otherwise:
Reviewed-by: Archit Taneja
> mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys", "MDP5");
> if (IS_ERR(mdp5_kms->mmio)) {
> ret = PTR_ERR(mdp5_kms->mmio);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> index 88120c5..52914ec 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
> @@ -27,6 +27,8 @@
> #include "mdp5_pipe.h"
> #include "mdp5_smp.h"
>
> +struct mdp5_state;
> +
> struct mdp5_kms {
> struct mdp_kms base;
>
> @@ -40,6 +42,11 @@ struct mdp5_kms {
> struct mdp5_cfg_handler *cfg;
> uint32_t caps; /* MDP capabilities (MDP_CAP_XXX bits) */
>
> + /**
> + * Global atomic state. Do not access directly, use mdp5_get_state()
> + */
> + struct mdp5_state *state;
> + struct drm_modeset_lock state_lock;
>
> /* mapper-id used to request GEM buffer mapped for scanout: */
> int id;
> @@ -69,6 +76,21 @@ struct mdp5_kms {
> };
> #define to_mdp5_kms(x) container_of(x, struct mdp5_kms, base)
>
> +/* Global atomic state for tracking resources that are shared across
> + * multiple kms objects (planes/crtcs/etc).
> + *
> + * For atomic updates which require modifying global state,
> + */
> +struct mdp5_state {
> + uint32_t dummy;
> +};
> +
> +struct mdp5_state *__must_check
> +mdp5_get_state(struct drm_atomic_state *s);
> +
> +/* Atomic plane state. Subclasses the base drm_plane_state in order to
> + * track assigned hwpipe and hw specific state.
> + */
> struct mdp5_plane_state {
> struct drm_plane_state base;
>
>
--
Qualcomm Innovation
[PATCH v2 2/4] drm/imx: use drm_atomic_set_fence_for_plane() to set the fence
Am Montag, den 07.11.2016, 19:03 +0900 schrieb Gustavo Padovan: > From: Gustavo Padovan > > drm_atomic_set_fence_for_plane() is smart and won't overwrite > plane_state->fence if the user already set an explicit fence there. > > Cc: Philipp Zabel > Signed-off-by: Gustavo Padovan > Reviewed-by: Daniel Vetter Acked-by: Philipp Zabel > --- > drivers/gpu/drm/imx/imx-drm-core.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c > b/drivers/gpu/drm/imx/imx-drm-core.c > index 98df09c..07fe955 100644 > --- a/drivers/gpu/drm/imx/imx-drm-core.c > +++ b/drivers/gpu/drm/imx/imx-drm-core.c > @@ -158,6 +158,7 @@ static int imx_drm_atomic_commit(struct drm_device *dev, > struct drm_plane_state *plane_state; > struct drm_plane *plane; > struct dma_buf *dma_buf; > + struct dma_fence *fence; > int i; > > /* > @@ -170,8 +171,9 @@ static int imx_drm_atomic_commit(struct drm_device *dev, >0)->base.dma_buf; > if (!dma_buf) > continue; > - plane_state->fence = > - reservation_object_get_excl_rcu(dma_buf->resv); > + fence = reservation_object_get_excl_rcu(dma_buf->resv); > + > + drm_atomic_set_fence_for_plane(plane_state, fence); > } > } regards Philipp
[PATCH 0/5] drm/sun4i: Handle TV overscan
On Mon, Nov 07, 2016 at 04:09:09PM +0100, Maxime Ripard wrote: > Hi Russell, > > On Thu, Nov 03, 2016 at 09:54:45AM +, Russell King - ARM Linux wrote: > > > Yes. And that is an XBMC only solution, that doesn't work with the > > > fbdev emulation and is probably doing an additional composition to > > > scale down and center their frames through OpenGL. > > > > Well, it will have to be doing a scaling step anyway. If the video > > frame is a different size to the active area, scaling is required > > no matter what. A 576p SD image needs to be scaled up, and a 1080p > > image would need to be scaled down for a 1080p overscanned display > > with a reduced active area to counter the overscanning - no matter > > how you do it. > > Yes, except that scaling is not always an option. In my particular > example, there's no scaler after the CRTC, which essentially prevents > it from being used in that use case. Which is also why I ended up > reducing the mode reported to the user. I think you completely missed my point. Let me try again. If you expose a reduced mode to the user, you are reporting that (eg) the 1080p-timings mode does not have 1920 pixels horizontally, and 1080 lines. You are instead reporting that it has (eg) 1800 pixels horizontally and maybe 1000 lines. So, when you play back a 1080 video, you are going to have to either: 1. crop the extra 120 pixels horizontally and 80 lines vertically or 2. scale the image. However, this is a completely independent issue to how we go about setting a video mode that is 1800x1000 in the first place. What you're suggesting is that we add code to the kernel to report that your non-EDID, analogue output transforms the standard 1920x1080 timings such that it has a 1800x1000 active area. I'm suggesting instead that you can do the same thing in userspace by specifically adding a mode which has the 1920x1080 standard timings, but with the porches increased and the active area reduced - in exactly the same way that you'd have to do within the kernel to report your active-area-reduced 1800x1000 mode. > > For graphics, userspace could add mode(s) with increased porches and > > reduced active area itself to achieve an underscanned display on a > > timing which the display device always overscans - there's no need to > > do that in the kernel, all the APIs are there to be able to do it > > already. > > > > That means your framebuffer will be smaller, but that's the case > > anyway. > > Yes, that would be a good idea. But it's not always an option for > applications that would rely on the fbdev emulation (like QT's eglfs), > which would then have no way to change whatever default there is (and > the only one able to know how bad it actually is is the user). I guess this is the problem with DRM people wanting to deprecate fbdev... too much stuff currently relies upon it, but DRM on x86 has always had the reduced functionality. I guess there's two solutions here: 1. Either DRMs fbdev gains increased functionality, or 2. The fbdev-only applications/libraries need to be ported over to support DRM natively. This has been a bar for some time to moving over to DRM, and I've heard very little desire on either side to find some sort of compromise on the issue, so I guess things are rather stuck where they are. > > So, I may want my graphics to appear on an overscanned display such > > that I can see the borders, but I may want the overlaid video to use > > the full active area (including overscan) to hide some of the broadcast > > mess at the edges of the screen. > > > > > > Yea, that's quite sad, "analog" has become a dirty word, but really > > > > this has nothing to do with "analog" at all - there are LCD TVs (and > > > > some monitors) out there which take HDMI signals but refuse to > > > > disable overscan no matter what you do to them if you provide them > > > > with a "broadcast" mode - so the analog excuse is very poor. > > > > > > I'd agree with you, but I was also told to not turn that into a > > > generic code and deal with that in my driver. > > > > I guess whoever told you that was wrong. I used to have a cheap TV > > here which took HDMI, and always overscans broadcast modes, and > > underscans PC modes. No amount of fiddling with various settings > > (either in the TV or AVI frames) changes that. > > > > I've run into that with a _monitor_ that Andrew Hutton has, which has > > a HDMI input - exactly the same thing - 1080p, 720p etc are all > > unconditionally overscanned, 1024x768 etc are all unconditionally > > underscanned and there's nothing that can be done to change it. > > > > The problem is that TVs are at liberty to have this behaviour, so > > the overscaned problem is always going to be there, and each driver > > should not be dealing with it in their own specific ways. > > I agree with you, however, without any directions on how to do this, > and willingness to merge this, I don't really see why we would work on > such a gene
[PATCH] drm: panel: simple-panel: get the enable gpio as-is
Am Montag, den 07.11.2016, 14:17 +0100 schrieb Thierry Reding: > On Mon, Nov 07, 2016 at 06:12:43PM +0800, Chen-Yu Tsai wrote: > > On Sun, Nov 6, 2016 at 7:09 PM, Icenowy Zheng wrote: > > > The enable gpio of simple-panel may be used by a simplefb or other > > > driver on the panel's display before the KMS driver get load. > > > > > > Get the GPIO as-is, so the panel won't be disabled, and the simplefb > > > can work. > > > > > > Signed-off-by: Icenowy Zheng > > > --- > > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > > b/drivers/gpu/drm/panel/panel-simple.c > > > index 113db3c..ccee4c1 100644 > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > @@ -312,7 +312,7 @@ static int panel_simple_probe(struct device *dev, > > > const struct panel_desc *desc) > > > return PTR_ERR(panel->supply); > > > > > > panel->enable_gpio = devm_gpiod_get_optional(dev, "enable", > > > -GPIOD_OUT_LOW); > > > +GPIOD_ASIS); > > > > The GPIO requested as-is might be in input mode. You should change the > > gpiod_set_value calls to gpiod_direction_output calls. The later also > > allows you to give an initial value. Not sure if it checks for cansleep > > like the set_value calls though. > > I'd prefer not to add gpiod_direction_output() calls outside of > ->probe(). Instead, could we make this patch be smart about taking over > from an earlier user? Could it read the current direction and value of > the GPIO and not disable it if it had previously been enabled? Seconded, the PWM backlight driver in drivers/video/backlight/pwm_bl.c already does a similar thing. > And even if we go this extra mile there's a possibility that the GPIO > was just left dangling by earlier software (or hardware) and leaving it > on would actually be worse than turning the panel off. Is this something the encoder driver should communicate to the panel? That one will know whether its atomic_reset state is enabled or disabled. regards Philipp
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
On 7 November 2016 at 07:43, Sharma, Shashank wrote: > If I was not very clear for the first time, every time we send a patch to > drm-intel/dri-devel, we do basic testing on Gnome-desktop too (Not only > Android). > > So even these aspect ratio patches were tested with full gnome-desktop, and > it worked well. > > I guess the part that's not so obvious is that Linux desktop/distributions provide a very wide permutation of components and configs used. While on a local (test) setup only a small/limited set is available. Esp. on Android where things are _very_ tightly coupled. Obviously nobody likes when we have to carry kernel patches which workaround "broken" userspace, but it's a kernel policy and we all have to live with it. That's the main reason people are so careful/pedantic when it comes to UABI. And as you can see even then there are bits that we miss :'-( Regards, Emil
[PATCH 0/5] drm/sun4i: Handle TV overscan
Hi Sean, On Thu, Nov 03, 2016 at 03:11:26PM -0600, Sean Paul wrote: > On Thu, Nov 3, 2016 at 3:01 AM, Maxime Ripard > wrote: > > Hi Russell, > > > > On Mon, Oct 31, 2016 at 08:42:34AM +, Russell King - ARM Linux wrote: > >> On Tue, Oct 18, 2016 at 12:03:49PM +0200, Maxime Ripard wrote: > >> > The first one is that this overscanning should be reported by the > >> > connector I guess? but this is really TV specific, so we need one way > >> > to let the user tell how the image is displayed on its side, and we > >> > cannot really autodetect it, and this needs to be done at runtime so > >> > that we can present some shiny interface to let it select which > >> > overscan ratio works for him/her. > >> > >> See xbmc... they go through a nice shiny setup which includes adjusting > >> the visible area. From what I remember, it has pointers on each corner > >> which you can adjust to be just visible on the screen, so xbmc knows > >> how much overscan there is, and xbmc itself reduces down to the user > >> set size. > > > > Yes. And that is an XBMC only solution, that doesn't work with the > > fbdev emulation and is probably doing an additional composition to > > scale down and center their frames through OpenGL. > > > > We might not have a GPU in the system, and we might not even have an > > entire graphic stack on top either, so I don't think fixing at the > > user-space level is a good option (especially since we already have an > > overscan property in DRM). > > > > Hi Maxime, > I took a quick look at the first 2 patches in the series and they look > good at first glance. I have them in my queue to review more > carefully. Yes, the first one is pretty scary. If it can ease your review, I made a bunch of unittests to test that code. It's pretty hacky (basically a copy of some kernel structures and the new logic to parse the command line), but it should test it with a significant number of cases: http://code.bulix.org/4lnlk7-107122?raw It's pretty straightforward to compile, you just have to link against cmocka. > Can you explain why you can't fix this by specifying a new mode with > big porches (as Russell suggested)? I'll reply to his mail. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/f6460e69/attachment-0001.sig>
[Bug 97988] [radeonsi] playing back videos with VDPAU exhibits deinterlacing/anti-aliasing issues not visible with VA-API
https://bugs.freedesktop.org/show_bug.cgi?id=97988 --- Comment #25 from Andy Furniss --- (In reply to Dieter Nützel from comment #23) > (In reply to Andy Furniss from comment #21) > > (In reply to Nicolai Hähnle from comment #20) > > > Created attachment 127812 [details] [review] [review] [review] > > > Mesa part of fix > > > > > > LLVM patch https://reviews.llvm.org/D26348 together with the attached > > > patch > > > for Mesa fix this bug. > > > > Nice, will test later, but is there a actually a nice way to get a patch > > from Phabricator that will apply from top level with git apply? > > > > I usually fail with "download raw diff" and have to search/sort out the > > paths by hand. > > Hello Andy, > > how did you solved this? Turned out to be easier than I remembered from my previous few efforts when (rarely) applying patches for llvm from there. All I needed to do, was wherever there is a +++ or --- followed by a path, add a leading / to the path. I guess there is a cool command line way like Nicolai says - but I would probably end up hosing the whole thing if I tried like that :-) -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/cf827281/attachment.html>
[Bug 97988] [radeonsi] playing back videos with VDPAU exhibits deinterlacing/anti-aliasing issues not visible with VA-API
https://bugs.freedesktop.org/show_bug.cgi?id=97988 --- Comment #24 from Nicolai Hähnle --- Andy, I usually use the arc command line tool for this kind of stuff. Thanks for testing! -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/f304fbce/attachment-0001.html>
[PATCH v3] drm: bridge: add DesignWare HDMI I2S audio support
On Mon, Nov 07, 2016 at 03:50:00AM +, Kuninori Morimoto wrote: > > Hi Russell > > > > + platform = platform_device_register_full(&pdevinfo); > > > + if (IS_ERR_OR_NULL(platform)) > > > + return PTR_ERR(platform); > > > > This is wrong. If platform is NULL, PTR_ERR() will return zero, which > > will be interpreted as success. Please, avoid using IS_ERR_OR_NULL(), > > it leads to exactly this kind of cockup - and it's unnecessary here > > because platform_device_register_full() does not return NULL. > > Thank you for your feedback. > Before sending v4 patch, I would like to confirm. Do you meand this ? > # use IS_ERR() instead of IS_ERR_OR_NULL() > > platform = platform_device_register_full(&pdevinfo); > if (IS_ERR(platform)) > return PTR_ERR(platform); Yes, that's exactly correct. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
Enable AMDGPU for CIK by default
>-Original Message- >From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On Behalf >Of Michel Dänzer >Sent: Monday, November 07, 2016 2:24 AM >To: Sandeep >Cc: dri-devel at lists.freedesktop.org >Subject: Re: Enable AMDGPU for CIK by default > >On 07/11/16 03:56 AM, Sandeep wrote: >> Hello, >> >> I was wondering when DRM_AMDGPU_CIK would be turned on by default >in >> the upstream kernel (or is this upto individual distros?) >> >> Is there any work left to be done/bugs to be fixed before it can be >> enabled by default? > >There are still some functional regressions, notably amdgpu doesn't support >HDMI/DP audio yet. > >Also, simply enabling DRM_AMDGPU_CIK by default isn't a good solution, >since the radeon driver also still supports the CIK devices, and there's no >good mechanism to choose which driver gets to drive a particular GPU at >runtime. Right... we would need corresponding logic to disable CIK support in radeon. Would we need two flags, one for each driver, or could we define a flag at drivers/gpu/drm level which would choose between radeon and amdgpu for CIK hardware ? Even as I type that I don't like it... so two flags I guess. > > >-- >Earthling Michel Dänzer | http://www.amd.com >Libre software enthusiast | Mesa and X developer >___ >dri-devel mailing list >dri-devel at lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 97988] [radeonsi] playing back videos with VDPAU exhibits deinterlacing/anti-aliasing issues not visible with VA-API
https://bugs.freedesktop.org/show_bug.cgi?id=97988 --- Comment #23 from Dieter Nützel --- (In reply to Andy Furniss from comment #21) > (In reply to Nicolai Hähnle from comment #20) > > Created attachment 127812 [details] [review] [review] > > Mesa part of fix > > > > LLVM patch https://reviews.llvm.org/D26348 together with the attached patch > > for Mesa fix this bug. > > Nice, will test later, but is there a actually a nice way to get a patch > from Phabricator that will apply from top level with git apply? > > I usually fail with "download raw diff" and have to search/sort out the > paths by hand. Hello Andy, how did you solved this? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/d332b713/attachment.html>
[PATCH] drm: panel: simple-panel: get the enable gpio as-is
On Mon, Nov 07, 2016 at 06:12:43PM +0800, Chen-Yu Tsai wrote: > On Sun, Nov 6, 2016 at 7:09 PM, Icenowy Zheng wrote: > > The enable gpio of simple-panel may be used by a simplefb or other > > driver on the panel's display before the KMS driver get load. > > > > Get the GPIO as-is, so the panel won't be disabled, and the simplefb > > can work. > > > > Signed-off-by: Icenowy Zheng > > --- > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > b/drivers/gpu/drm/panel/panel-simple.c > > index 113db3c..ccee4c1 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -312,7 +312,7 @@ static int panel_simple_probe(struct device *dev, const > > struct panel_desc *desc) > > return PTR_ERR(panel->supply); > > > > panel->enable_gpio = devm_gpiod_get_optional(dev, "enable", > > -GPIOD_OUT_LOW); > > +GPIOD_ASIS); > > The GPIO requested as-is might be in input mode. You should change the > gpiod_set_value calls to gpiod_direction_output calls. The later also > allows you to give an initial value. Not sure if it checks for cansleep > like the set_value calls though. I'd prefer not to add gpiod_direction_output() calls outside of ->probe(). Instead, could we make this patch be smart about taking over from an earlier user? Could it read the current direction and value of the GPIO and not disable it if it had previously been enabled? And even if we go this extra mile there's a possibility that the GPIO was just left dangling by earlier software (or hardware) and leaving it on would actually be worse than turning the panel off. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/7e0e347f/attachment-0001.sig>
[PATCH v8 02/12] drm/i915: Add i915 perf infrastructure
On Fri, 2016-11-04 at 06:19 -0700, Robert Bragg wrote:
>
>
> On Fri, Nov 4, 2016 at 8:59 AM, sourab gupta
> wrote:
> On Thu, 2016-10-27 at 19:14 -0700, Robert Bragg wrote:
> > Adds base i915 perf infrastructure for Gen performance
> metrics.
> >
> > This adds a DRM_IOCTL_I915_PERF_OPEN ioctl that takes an
> array of uint64
> > properties to configure a stream of metrics and returns a
> new fd usable
> > with standard VFS system calls including read() to read
> typed and sized
> > records; ioctl() to enable or disable capture and poll() to
> wait for
> > data.
> >
> > A stream is opened something like:
> >
> > uint64_t properties[] = {
> > /* Single context sampling */
> > DRM_I915_PERF_PROP_CTX_HANDLE,ctx_handle,
> >
> > /* Include OA reports in samples */
> > DRM_I915_PERF_PROP_SAMPLE_OA, true,
> >
> > /* OA unit configuration */
> > DRM_I915_PERF_PROP_OA_METRICS_SET,metrics_set_id,
> > DRM_I915_PERF_PROP_OA_FORMAT, report_format,
> > DRM_I915_PERF_PROP_OA_EXPONENT, period_exponent,
> >};
> >struct drm_i915_perf_open_param parm = {
> > .flags = I915_PERF_FLAG_FD_CLOEXEC |
> >I915_PERF_FLAG_FD_NONBLOCK |
> >I915_PERF_FLAG_DISABLED,
> > .properties_ptr = (uint64_t)properties,
> > .num_properties = sizeof(properties) / 16,
> >};
> >int fd = drmIoctl(drm_fd, DRM_IOCTL_I915_PERF_OPEN,
> ¶m);
> >
> > Records read all start with a common { type, size } header
> with
> > DRM_I915_PERF_RECORD_SAMPLE being of most interest. Sample
> records
> > contain an extensible number of fields and it's the
> > DRM_I915_PERF_PROP_SAMPLE_xyz properties given when opening
> that
> > determine what's included in every sample.
> >
> > No specific streams are supported yet so any attempt to open
> a stream
> > will return an error.
> >
> > v2:
> > use i915_gem_context_get() - Chris Wilson
> > v3:
> > update read() interface to avoid passing state struct -
> Chris Wilson
> > fix some rebase fallout, with i915-perf init/deinit
> > v4:
> > s/DRM_IORW/DRM_IOW/ - Emil Velikov
> >
> > Signed-off-by: Robert Bragg
> > ---
> > drivers/gpu/drm/i915/Makefile| 3 +
> > drivers/gpu/drm/i915/i915_drv.c | 4 +
> > drivers/gpu/drm/i915/i915_drv.h | 91
> > drivers/gpu/drm/i915/i915_perf.c | 443
> +++
> > include/uapi/drm/i915_drm.h | 67 ++
> > 5 files changed, 608 insertions(+)
> > create mode 100644 drivers/gpu/drm/i915/i915_perf.c
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile
> b/drivers/gpu/drm/i915/Makefile
> > index 6123400..8d4e25f 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -113,6 +113,9 @@ i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) +=
> i915_gpu_error.o
> > # virtual gpu code
> > i915-y += i915_vgpu.o
> >
> > +# perf code
> > +i915-y += i915_perf.o
> > +
> > ifeq ($(CONFIG_DRM_I915_GVT),y)
> > i915-y += intel_gvt.o
> > include $(src)/gvt/Makefile
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> > index af3559d..685c96e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -836,6 +836,8 @@ static int i915_driver_init_early(struct
> drm_i915_private *dev_priv,
> >
> > intel_detect_preproduction_hw(dev_priv);
> >
> > + i915_perf_init(dev_priv);
> > +
> > return 0;
> >
> > err_workqueues:
> > @@ -849,6 +851,7 @@ static int i915_driver_init_early(struct
> drm_i915_private *dev_priv,
> > */
> > static void i915_driver_cleanup_early(struct
> drm_i915_private *dev_priv)
> > {
> > + i915_perf_fini(dev_priv);
> > i915_gem_load_cleanup(&dev_priv->drm);
> > i915_workqueues_cleanup(dev_priv);
> > }
> > @@ -2556,6 +2559,7 @@ static const struct drm_ioctl_desc
> i915_ioctls[] = {
> > DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR,
> i915_gem_userptr_ioctl, DRM_RENDER_ALLOW),
> > DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM,
>
[Bug 97988] [radeonsi] playing back videos with VDPAU exhibits deinterlacing/anti-aliasing issues not visible with VA-API
https://bugs.freedesktop.org/show_bug.cgi?id=97988 --- Comment #22 from Andy Furniss --- (In reply to Andy Furniss from comment #21) > (In reply to Nicolai Hähnle from comment #20) > > Created attachment 127812 [details] [review] [review] > > Mesa part of fix > > > > LLVM patch https://reviews.llvm.org/D26348 together with the attached patch > > for Mesa fix this bug. > > Nice, will test later Working OK for me. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/08980b9d/attachment.html>
[PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
If I was not very clear for the first time, every time we send a patch to drm-intel/dri-devel, we do basic testing on Gnome-desktop too (Not only Android). So even these aspect ratio patches were tested with full gnome-desktop, and it worked well. Regards Shashank On 11/3/2016 9:49 PM, Sharma, Shashank wrote: > > > On 11/3/2016 9:33 PM, Daniel Vetter wrote: >> On Thu, Nov 3, 2016 at 2:49 PM, Ville Syrjälä >> wrote: If you still think you should send this revert, I am removing my NACK. Pls Go ahead. >>> The other option is to not revert and instead slap a fix on top. But >>> that would have to be done reasonably quickly so that the thing is >>> ready in time for 4.10. We're closing in on 4.9-rc4 now so I guess >>> we should still have a few weeks to fix things up. Whether that's >>> enough I don't know. If not, then we should revert. >> 1 week and then revert is the guideline. Please don't bend the rules >> for regressions all the time, it makes things painful for everyone >> else. >> -Daniel > I don't remember breaking it for first time itself. > As I mentioned in the other thread, please add the regression details. > After that, revert it if you want (I saw you have already given > maintainers-ack) > > Regards > Shashank >
[PATCH] drm/tegra: gem: Remove some dead code
On Sun, Oct 30, 2016 at 04:50:29PM +0100, Christophe JAILLET wrote: > 'dma_buf_map_attachment()' can not return NULL, so there is no need to > check for it. > > Signed-off-by: Christophe JAILLET > --- > drivers/gpu/drm/tegra/gem.c | 5 - > 1 file changed, 5 deletions(-) Applied, thanks. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/31d3aa77/attachment.sig>
[PATCH] drm/tegra: sor: No need to free devm_ allocated memory
On Sat, Sep 24, 2016 at 10:07:30PM +0200, Christophe JAILLET wrote: > 'brick' has been allocated by 'devm_kzalloc', so there is no need here to > free it explicitly. > There is only one caller of 'tegra_clk_sor_brick_register()'. This function > is a probe function which correctly checks and handles the return value of > 'tegra_clk_sor_brick_register()'. > > Signed-off-by: Christophe JAILLET > --- > drivers/gpu/drm/tegra/sor.c | 2 -- > 1 file changed, 2 deletions(-) Applied, with a slightly reworded commit message. Thanks, Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/49843cca/attachment.sig>
[PATCH] drm/tegra: fix error handling
On Sun, Jul 03, 2016 at 08:18:57AM +0200, Christophe JAILLET wrote: > This is likely that checking 'gr3d->clk_secondary' instead of 'gr3d->clk' > is expected here. > > > Signed-off-by: Christophe JAILLET > --- > drivers/gpu/drm/tegra/gr3d.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Applied, thanks. Sorry for not noticing this one earlier. Thierry -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/fd9b4f90/attachment.sig>
[PATCH] drm/tegra: dpaux: Fix error handling
On Fri, Oct 28, 2016 at 11:09:45AM +0200, Christophe JAILLET wrote:
> 'devm_pinctrl_register()' returns an error pointer or a valid handle. So
> checking for NULL here is pointless and can never trigger.
>
> Check the returned value with IS_ERR instead and propagate this value as
> done in the other functions which call 'devm_pinctrl_register()'
>
> Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support")
>
> Signed-off-by: Christophe JAILLET
> ---
> drivers/gpu/drm/tegra/dpaux.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Applied, thanks.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/544f81c6/attachment.sig>
[PATCH 5/5] reservation: revert "wait only with non-zero timeout specified (v3)" v2
On Sun, Nov 6, 2016 at 8:07 PM, Gustavo Padovan wrote: > Hi Christian, > > 2016-10-20 Christian König : > >> From: Christian König >> >> This reverts commit fb8b7d2b9d80e1e71f379e57355936bd2b024be9. >> >> Otherwise signaling might never be activated on the fences. This can >> result in infinite waiting with hardware which has unreliable interrupts. >> >> v2: still return one when the timeout is zero and we don't have any fences. >> >> Signed-off-by: Christian König >> Reviewed-by: Chunming Zhou (v1) >> --- >> drivers/dma-buf/reservation.c | 5 + >> 1 file changed, 1 insertion(+), 4 deletions(-) > > Reviewed-by: Gustavo Padovan I've rebased these patches based on the fence renaming in drm-next. Should I pull these in through my tree or should they go in through drm-misc or the dma-buf tree? If the later, I'll send out the rebased patches. Alex
[PATCH 1/2] dma-buf: return index of the first signaled fence (v2)
On Fri, Nov 4, 2016 at 6:03 PM, Sumit Semwal wrote: > Hi Alex, > > Thanks for the patches. > > On 4 November 2016 at 14:16, Alex Deucher wrote: >> From: "monk.liu" >> >> Return the index of the first signaled fence. This information >> is useful in some APIs like Vulkan. >> >> v2: rebase on drm-next (fence -> dma_fence) >> >> Signed-off-by: monk.liu >> Signed-off-by: Alex Deucher >> Cc: Sumit Semwal >> --- >> >> This is the same patch set I send out yesterday, I just >> squashed the amdgpu patches together and rebased everything on >> the fence -> dma_fence renaming. This is used by our VK driver >> and we are planning to use it in mesa as well. >> > > Would you be ok if I apply this and the amdgpu patch both together via > drm-misc, or would you like me to notify you once I merge this for you > to take the amdgpu patch via your tree? I'm fine either ways, but > perhaps drm-misc would be a bit neater. > Either way works for me. Whatever is easier for you. Alex
[PATCH] drm/amdgpu: Attach exclusive fence to prime exported bo's. (v3)
On 07/11/16 11:47 AM, Mario Kleiner wrote:
> External clients which import our bo's wait only
> for exclusive dmabuf-fences, not on shared ones,
> so attach fences on exported buffers as exclusive
> ones, if the buffers get imported by an external
> client.
>
> See discussion in thread:
> https://lists.freedesktop.org/archives/dri-devel/2016-October/122370.html
>
> Tested on Intel iGPU + AMD Tonga dGPU as DRI3/Present
> Prime render offload, and with the Tonga standalone as
> primary gpu.
>
> v2: Add a wait for all shared fences before prime export,
> as suggested by Christian Koenig.
>
> v3: - Mark buffer prime_exported in amdgpu_gem_prime_pin,
> so we only use the exclusive fence when exporting a
> bo to external clients like a separate iGPU, but not
> when exporting/importing from/to ourselves as part of
> regular DRI3 fd passing.
>
> - Propagate failure of reservation_object_wait_rcu back
> to caller.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95472
> (v1) Tested-by: Mike Lothian
FWIW, I think the (v1) is usually added at the end of the line, not at
the beginning.
[...]
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> index 7700dc2..c4494d0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> @@ -81,6 +81,20 @@ int amdgpu_gem_prime_pin(struct drm_gem_object *obj)
> {
> struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
> int ret = 0;
> + long lret;
> +
> + /*
> + * Wait for all shared fences to complete before we switch to future
> + * use of exclusive fence on this prime_exported bo.
> + */
> + lret = reservation_object_wait_timeout_rcu(bo->tbo.resv, true, false,
> +MAX_SCHEDULE_TIMEOUT);
> + if (unlikely(lret < 0)) {
> + DRM_DEBUG_PRIME("Fence wait failed: %li\n", lret);
> + return lret;
> + }
> +
> + bo->prime_exported = true;
We should probably clear bo->prime_exported in amdgpu_gem_prime_unpin.
Also, I think we should set bo->prime_exported (prime_shared?) in
amdgpu_gem_prime_import_sg_table as well, so that we'll also set
exclusive fences on BOs imported from other GPUs.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
[Bug 98578] AMDGPU white glitches in some games
https://bugs.freedesktop.org/show_bug.cgi?id=98578 --- Comment #7 from Nicolai Hähnle --- Thanks for the trace, I can reproduce this and will look into it. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/86771c04/attachment.html>
[PATCH 1/2] mm: add locked parameter to get_user_pages()
On Mon, Oct 31, 2016 at 10:02:27AM +, Lorenzo Stoakes wrote: > This patch adds an int *locked parameter to get_user_pages() to allow > VM_FAULT_RETRY faulting behaviour similar to get_user_pages_[un]locked(). > > It additionally clears the way for get_user_pages_locked() to be removed as > its > sole remaining useful characteristic was to allow for VM_FAULT_RETRY behaviour > when faulting in pages. > > It should not introduce any functional changes, however it does allow for > subsequent changes to get_user_pages() callers to take advantage of > VM_FAULT_RETRY. For the cris-part: Acked-by: Jesper Nilsson > Signed-off-by: Lorenzo Stoakes /^JN - Jesper Nilsson -- Jesper Nilsson -- jesper.nilsson at axis.com
[PATCH v4 0/3] Add initial ZTE VOU DRM/KMS driver
| 91 +++ > > drivers/gpu/drm/zte/zx_vou.c | 661 + > > drivers/gpu/drm/zte/zx_vou.h | 46 ++ > > drivers/gpu/drm/zte/zx_vou_regs.h | 157 + > > 16 files changed, 2372 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/display/zte,vou.txt > > create mode 100644 drivers/gpu/drm/zte/Kconfig > > create mode 100644 drivers/gpu/drm/zte/Makefile > > create mode 100644 drivers/gpu/drm/zte/zx_drm_drv.c > > create mode 100644 drivers/gpu/drm/zte/zx_drm_drv.h > > create mode 100644 drivers/gpu/drm/zte/zx_hdmi.c > > create mode 100644 drivers/gpu/drm/zte/zx_hdmi_regs.h > > create mode 100644 drivers/gpu/drm/zte/zx_plane.c > > create mode 100644 drivers/gpu/drm/zte/zx_plane.h > > create mode 100644 drivers/gpu/drm/zte/zx_plane_regs.h > > create mode 100644 drivers/gpu/drm/zte/zx_vou.c > > create mode 100644 drivers/gpu/drm/zte/zx_vou.h > > create mode 100644 drivers/gpu/drm/zte/zx_vou_regs.h > > > > -- > > 1.9.1 > > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/113f38c4/attachment-0001.html>
[Bug 107381] radeon VCE init error (-110) -- AMD/Intel Mars Hybrid Graphics
https://bugzilla.kernel.org/show_bug.cgi?id=107381 madbiologist changed: What|Removed |Added CC||madbiologist2016 at outlook.co ||m --- Comment #13 from madbiologist --- VCE 1.0 support was added in kernel 4.2. If you are able to build your own kernel with this commit reverted it should fix the issue: https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=a918efab631a5112d9d168700458317ad77f269c -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 97988] [radeonsi] playing back videos with VDPAU exhibits deinterlacing/anti-aliasing issues not visible with VA-API
https://bugs.freedesktop.org/show_bug.cgi?id=97988 --- Comment #21 from Andy Furniss --- (In reply to Nicolai Hähnle from comment #20) > Created attachment 127812 [details] [review] > Mesa part of fix > > LLVM patch https://reviews.llvm.org/D26348 together with the attached patch > for Mesa fix this bug. Nice, will test later, but is there a actually a nice way to get a patch from Phabricator that will apply from top level with git apply? I usually fail with "download raw diff" and have to search/sort out the paths by hand. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/e494912e/attachment.html>
[GIT PULL] ZTE zxdrm driver support for 4.10
Hi Dave, Please consider to pull the following initial ZTE zxdrm driver support for 4.10. The pull request is based on v4.9-rc1. If you need it to be on other base, just let me know, and I will update it. Thanks. Shawn The following changes since commit 1001354ca34179f3db924eb66672442a173147dc: Linux 4.9-rc1 (2016-10-15 12:17:50 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git tags/zxdrm-4.10 for you to fetch changes up to dbb010376768e0ed657b2be679875d275fd9e8c8: MAINTAINERS: add an entry for ZTE ZX DRM driver (2016-11-07 11:02:31 +0800) ZTE zxdrm driver support for 4.10: This is the initial ZTE VOU display controller DRM/KMS driver. There are still some features to be added, like overlay plane, scaling, and more output devices support. But it's already useful with dual CRTCs and HDMI display working. Shawn Guo (3): dt-bindings: add bindings doc for ZTE VOU display controller drm: zte: add initial vou drm driver MAINTAINERS: add an entry for ZTE ZX DRM driver .../devicetree/bindings/display/zte,vou.txt| 84 +++ MAINTAINERS| 7 + drivers/gpu/drm/Kconfig| 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/zte/Kconfig| 8 + drivers/gpu/drm/zte/Makefile | 7 + drivers/gpu/drm/zte/zx_drm_drv.c | 267 + drivers/gpu/drm/zte/zx_drm_drv.h | 36 ++ drivers/gpu/drm/zte/zx_hdmi.c | 624 +++ drivers/gpu/drm/zte/zx_hdmi_regs.h | 56 ++ drivers/gpu/drm/zte/zx_plane.c | 299 ++ drivers/gpu/drm/zte/zx_plane.h | 26 + drivers/gpu/drm/zte/zx_plane_regs.h| 91 +++ drivers/gpu/drm/zte/zx_vou.c | 661 + drivers/gpu/drm/zte/zx_vou.h | 46 ++ drivers/gpu/drm/zte/zx_vou_regs.h | 157 + 16 files changed, 2372 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/zte,vou.txt create mode 100644 drivers/gpu/drm/zte/Kconfig create mode 100644 drivers/gpu/drm/zte/Makefile create mode 100644 drivers/gpu/drm/zte/zx_drm_drv.c create mode 100644 drivers/gpu/drm/zte/zx_drm_drv.h create mode 100644 drivers/gpu/drm/zte/zx_hdmi.c create mode 100644 drivers/gpu/drm/zte/zx_hdmi_regs.h create mode 100644 drivers/gpu/drm/zte/zx_plane.c create mode 100644 drivers/gpu/drm/zte/zx_plane.h create mode 100644 drivers/gpu/drm/zte/zx_plane_regs.h create mode 100644 drivers/gpu/drm/zte/zx_vou.c create mode 100644 drivers/gpu/drm/zte/zx_vou.h create mode 100644 drivers/gpu/drm/zte/zx_vou_regs.h
[Bug 97988] [radeonsi] playing back videos with VDPAU exhibits deinterlacing/anti-aliasing issues not visible with VA-API
https://bugs.freedesktop.org/show_bug.cgi?id=97988 --- Comment #20 from Nicolai Hähnle --- Created attachment 127812 --> https://bugs.freedesktop.org/attachment.cgi?id=127812&action=edit Mesa part of fix LLVM patch https://reviews.llvm.org/D26348 together with the attached patch for Mesa fix this bug. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/394da367/attachment.html>
[PATCH 1/2] mm: add locked parameter to get_user_pages()
On Mon, Nov 07, 2016 at 11:49:18AM +0100, Jesper Nilsson wrote: > For the cris-part: > Acked-by: Jesper Nilsson Thanks very much for that, however just to avoid any confusion, I realised this series was not not the right way forward after discussion with Paolo and rather it makes more sense to keep the API as it is and to update callers where it makes sense to.
[PATCH v2 3/4] drm/msm: use drm_atomic_set_fence_for_plane() to set the fence
On Mon, Nov 7, 2016 at 5:03 AM, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> drm_atomic_set_fence_for_plane() is smart and won't overwrite
> plane_state->fence if the user already set an explicit fence there.
>
> Cc: Rob Clark
> Signed-off-by: Gustavo Padovan
> Reviewed-by: Daniel Vetter
LGTM
Acked-by: Rob Clark
let me know if I should pull this in via msm-next for 4.10.. otherwise
taking it via drm-misc is fine by me
BR,
-R
> ---
> drivers/gpu/drm/msm/msm_atomic.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c
> b/drivers/gpu/drm/msm/msm_atomic.c
> index db193f8..4e21e1d 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -217,8 +217,9 @@ int msm_atomic_commit(struct drm_device *dev,
> if ((plane->state->fb != plane_state->fb) && plane_state->fb)
> {
> struct drm_gem_object *obj =
> msm_framebuffer_bo(plane_state->fb, 0);
> struct msm_gem_object *msm_obj = to_msm_bo(obj);
> + struct dma_fence *fence =
> reservation_object_get_excl_rcu(msm_obj->resv);
>
> - plane_state->fence =
> reservation_object_get_excl_rcu(msm_obj->resv);
> + drm_atomic_set_fence_for_plane(plane_state, fence);
> }
> }
>
> --
> 2.5.5
>
[PATCH] drm/exynos/hdmi: refactor infoframe code
2016ë
10ì 26ì¼ 21:36ì Andrzej Hajda ì´(ê°) ì´ ê¸:
> Use core helpers to generate infoframes and generate vendor frame if
> necessary.
>
> Signed-off-by: Andrzej Hajda
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c | 141
> ++-
> drivers/gpu/drm/exynos/regs-hdmi.h | 2 +
> 2 files changed, 42 insertions(+), 101 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index e8fb6ef..1bb2df7 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -47,19 +47,6 @@
>
> #define HOTPLUG_DEBOUNCE_MS 1100
>
> -/* AVI header and aspect ratio */
> -#define HDMI_AVI_VERSION 0x02
> -#define HDMI_AVI_LENGTH 0x0d
> -
> -/* AUI header info */
> -#define HDMI_AUI_VERSION 0x01
> -#define HDMI_AUI_LENGTH 0x0a
> -
> -/* AVI active format aspect ratio */
> -#define AVI_SAME_AS_PIC_ASPECT_RATIO 0x08
> -#define AVI_4_3_CENTER_RATIO 0x09
> -#define AVI_16_9_CENTER_RATIO0x0a
> -
> enum hdmi_type {
> HDMI_TYPE13,
> HDMI_TYPE14,
> @@ -131,7 +118,6 @@ struct hdmi_context {
> booldvi_mode;
> struct delayed_work hotplug_work;
> struct drm_display_mode current_mode;
> - u8 cea_video_id;
> const struct hdmi_driver_data *drv_data;
>
> void __iomem*regs;
> @@ -681,6 +667,13 @@ static inline void hdmi_reg_writev(struct hdmi_context
> *hdata, u32 reg_id,
> }
> }
>
> +static inline void hdmi_reg_write_buf(struct hdmi_context *hdata, u32 reg_id,
> + u8 *buf, int size)
> +{
> + for (reg_id = hdmi_map_reg(hdata, reg_id); size; --size, reg_id += 4)
> + writel(*buf++, hdata->regs + reg_id);
> +}
> +
> static inline void hdmi_reg_writemask(struct hdmi_context *hdata,
>u32 reg_id, u32 value, u32 mask)
> {
> @@ -762,93 +755,50 @@ static int hdmi_clk_set_parents(struct hdmi_context
> *hdata, bool to_phy)
> return ret;
> }
>
> -static u8 hdmi_chksum(struct hdmi_context *hdata,
> - u32 start, u8 len, u32 hdr_sum)
> -{
> - int i;
> -
> - /* hdr_sum : header0 + header1 + header2
> - * start : start address of packet byte1
> - * len : packet bytes - 1 */
> - for (i = 0; i < len; ++i)
> - hdr_sum += 0xff & hdmi_reg_read(hdata, start + i * 4);
> -
> - /* return 2's complement of 8 bit hdr_sum */
> - return (u8)(~(hdr_sum & 0xff) + 1);
> -}
> -
> -static void hdmi_reg_infoframe(struct hdmi_context *hdata,
> - union hdmi_infoframe *infoframe)
> +static void hdmi_reg_infoframes(struct hdmi_context *hdata)
> {
> - u32 hdr_sum;
> - u8 chksum;
> - u8 ar;
> + union hdmi_infoframe frm;
> + u8 buf[25];
> + int ret;
>
> if (hdata->dvi_mode) {
> - hdmi_reg_writeb(hdata, HDMI_VSI_CON,
> - HDMI_VSI_CON_DO_NOT_TRANSMIT);
> hdmi_reg_writeb(hdata, HDMI_AVI_CON,
> HDMI_AVI_CON_DO_NOT_TRANSMIT);
> + hdmi_reg_writeb(hdata, HDMI_VSI_CON,
> + HDMI_VSI_CON_DO_NOT_TRANSMIT);
> hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_NO_TRAN);
> return;
> }
>
> - switch (infoframe->any.type) {
> - case HDMI_INFOFRAME_TYPE_AVI:
> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> + &hdata->current_mode);
> + if (ret >= 0)
Seems above condition is not clear becuase
drm_hdmi_avi_infoframe_from_display_mode function returns 0 or negative value.
> + ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
> + if (ret > 0) {
> hdmi_reg_writeb(hdata, HDMI_AVI_CON, HDMI_AVI_CON_EVERY_VSYNC);
I think above code should be called when
drm_hdmi_avi_infoframe_from_display_mode function returned 0 and the value from
hdmi_avi_infoframe_pack function is more than 1.
> - hdmi_reg_writeb(hdata, HDMI_AVI_HEADER0, infoframe->any.type);
> - hdmi_reg_writeb(hdata, HDMI_AVI_HEADER1,
> - infoframe->any.version);
> - hdmi_reg_writeb(hdata, HDMI_AVI_HEADER2, infoframe->any.length);
> - hdr_sum = infoframe->any.type + infoframe->any.version +
> - infoframe->any.length;
> -
> - /* Output format zero hardcoded ,RGB YBCR selection */
> - hdmi_reg_writeb(hdata, HDMI_AVI_BYTE(1), 0 << 5 |
> - AVI_ACTIVE_FORMAT_VALID |
> - AVI_UNDERSCANNED_DISPLAY_VALID);
> -
> - /*
> - * Set the aspect ratio as per the mode, mentioned in
> - * Table 9 AVI InfoFrame Data Byte 2 of CEA-861-D S
[PATCH] drm/sun4i: Propagate error to the caller
Hi Christophe, 2016-11-04 Christophe JAILLET : > If 'sun4i_layers_init()' returns an error, propagate it instead of > returning -EINVAL unconditionally. > > Signed-off-by: Christophe JAILLET > --- > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Gustavo Padovan Gustavo
[Spice-devel] [drm/qxl v3 6/7] qxl: Don't notify userspace when monitors config is unchanged
On Mon, Nov 07, 2016 at 04:08:48AM -0500, Frediano Ziglio wrote:
> > @@ -124,9 +157,18 @@ void qxl_display_read_client_monitors_config(struct
> > qxl_device *qdev)
> > {
> >
> > struct drm_device *dev = qdev->ddev;
> > - while (qxl_display_copy_rom_client_monitors_config(qdev)) {
> > + int status;
> > +
> > + status = qxl_display_copy_rom_client_monitors_config(qdev);
> > + while (status == MONITORS_CONFIG_BAD_CRC) {
> > qxl_io_log(qdev, "failed crc check for client_monitors_config,"
> > " retrying\n");
> > + status = qxl_display_copy_rom_client_monitors_config(qdev);
> > + }
> > + if (status == MONITORS_CONFIG_UNCHANGED) {
> > + qxl_io_log(qdev, "config unchanged\n");
> > + DRM_DEBUG("ignoring unchanged client monitors config");
>
> Why log and debug? Looks like a missing debug cleanup.
qxl_io_log is going to be output host-side, DRM_DEBUG is guest-side,
having both was intentional.
Christophe
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/8255cfc1/attachment.sig>
[Bug 92936] Tonga powerplay isssues
https://bugs.freedesktop.org/show_bug.cgi?id=92936 Andy Furniss changed: What|Removed |Added Resolution|FIXED |--- Status|RESOLVED|REOPENED --- Comment #23 from Andy Furniss --- While this bug is a bit messy being a multi bug, there are still issues = I can still lock display as in https://bugs.freedesktop.org/show_bug.cgi?id=92936#c20 I will close this one and open one issue bug(s) when I get some time to re-test everything. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/05757a04/attachment.html>
[drm/qxl v3 0/7] qxl: Various cleanups/fixes
On Mo, 2016-11-07 at 09:00 +0100, Christophe Fergeau wrote: > Hey, > > Same series as v2 except that I removed the use of camel case in patch 6/7. > Now it's only using an anonymous enum + int. Can you please not drop the "PATCH" from $subject? thanks, Gerd
[PATCH v2] drm: move allocation out of drm_get_format_name()
Thomas has already acked the earlier version, but in case you need
it for this one, too.
vmwgfx portion: Acked-by: Sinclair Yeh
On Mon, Nov 07, 2016 at 12:48:09AM +, Eric Engestrom wrote:
> Fixes: 90844f00049e9f42573fd31d7c32e8fd31d3fd07
>
> drm: make drm_get_format_name thread-safe
>
> Signed-off-by: Eric Engestrom
> [danvet: Clarify that the returned pointer must be freed with
> kfree().]
> Signed-off-by: Daniel Vetter
>
> Cc: Rob Clark
> Cc: Christian König
> Suggested-by: Ville Syrjälä
> Signed-off-by: Eric Engestrom
> ---
>
> v2: use single-field struct instead of typedef to let the compiler
> enforce the type (Christian König)
>
> ---
> include/drm/drm_fourcc.h| 10 +-
> drivers/gpu/drm/drm_fourcc.c| 14 +++--
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 7 ++---
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 7 ++---
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 3 +-
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 7 ++---
> drivers/gpu/drm/drm_atomic.c| 7 +++--
> drivers/gpu/drm/drm_crtc.c | 7 +++--
> drivers/gpu/drm/drm_framebuffer.c | 7 +++--
> drivers/gpu/drm/drm_modeset_helper.c| 7 +++--
> drivers/gpu/drm/drm_plane.c | 7 +++--
> drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 7 ++---
> drivers/gpu/drm/i915/i915_debugfs.c | 10 +++---
> drivers/gpu/drm/i915/intel_atomic_plane.c | 8 ++---
> drivers/gpu/drm/i915/intel_display.c| 41
> ++---
> drivers/gpu/drm/radeon/atombios_crtc.c | 14 -
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 +-
> 17 files changed, 80 insertions(+), 86 deletions(-)
>
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index dc0aafa..4b03ca0 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -45,6 +45,14 @@ struct drm_format_info {
> u8 vsub;
> };
>
> +/**
> + * struct drm_format_name_buf - name of a DRM format
> + * @str: string buffer containing the format name
> + */
> +struct drm_format_name_buf {
> + char str[32];
> +};
> +
> const struct drm_format_info *__drm_format_info(u32 format);
> const struct drm_format_info *drm_format_info(u32 format);
> uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
> @@ -54,6 +62,6 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
> int drm_format_vert_chroma_subsampling(uint32_t format);
> int drm_format_plane_width(int width, uint32_t format, int plane);
> int drm_format_plane_height(int height, uint32_t format, int plane);
> -char *drm_get_format_name(uint32_t format) __malloc;
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
>
> #endif /* __DRM_FOURCC_H__ */
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index cbb8b77..99b0b60 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -79,17 +79,13 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t
> depth)
> EXPORT_SYMBOL(drm_mode_legacy_fb_format);
>
> /**
> - * drm_get_format_name - return a string for drm fourcc format
> + * drm_get_format_name - fill a string with a drm fourcc format's name
> * @format: format to compute name of
> + * @buf: caller-supplied buffer
> - *
> - * Note that the buffer returned by this function is owned by the caller
> - * and will need to be freed using kfree().
> */
> -char *drm_get_format_name(uint32_t format)
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf)
> {
> - char *buf = kmalloc(32, GFP_KERNEL);
> -
> - snprintf(buf, 32,
> + snprintf(buf->str, sizeof(buf->str),
>"%c%c%c%c %s-endian (0x%08x)",
>printable_char(format & 0xff),
>printable_char((format >> 8) & 0xff),
> @@ -98,7 +94,7 @@ char *drm_get_format_name(uint32_t format)
>format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
>format);
>
> - return buf;
> + return buf->str;
> }
> EXPORT_SYMBOL(drm_get_format_name);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 199d3f7..2924cdd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2032,7 +2032,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc
> *crtc,
> u32 tmp, viewport_w, viewport_h;
> int r;
> bool bypass_lut = false;
> - char *format_name;
> + struct drm_format_name_buf format_name;
>
> /* no fb bound */
> if (!atomic && !crtc->primary->fb) {
> @@ -2144,9 +2144,8 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc
> *crtc,
> bypass_lut = true;
> break;
> default:
> - format_name = drm_get_format
[PATCH 2/2] drm/amdgpu: add the interface of waiting multiple fences (v4)
Hi Alex,
2016-11-04 Alex Deucher :
> From: Junwei Zhang
>
> v2: agd: rebase and squash in all the previous optimizations and
> changes so everything compiles.
> v3: squash in Slava's 32bit build fix
> v4: rebase on drm-next (fence -> dma_fence),
> squash in Monk's ioctl update patch
>
> Signed-off-by: Junwei Zhang
> Reviewed-by: Monk Liu
> Reviewed-by: Jammy Zhou
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 173
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +-
> include/uapi/drm/amdgpu_drm.h | 28 ++
> 5 files changed, 205 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index dc98ceb..7a94a3c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1212,6 +1212,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void
> *data,
> struct drm_file *filp);
> int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file
> *filp);
> int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, struct drm_file
> *filp);
> +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp);
>
> int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 2728805..2004836 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1130,6 +1130,179 @@ int amdgpu_cs_wait_ioctl(struct drm_device *dev, void
> *data,
> }
>
> /**
> + * amdgpu_cs_get_fence - helper to get fence from drm_amdgpu_fence
> + *
> + * @adev: amdgpu device
> + * @filp: file private
> + * @user: drm_amdgpu_fence copied from user space
> + */
> +static struct dma_fence *amdgpu_cs_get_fence(struct amdgpu_device *adev,
> + struct drm_file *filp,
> + struct drm_amdgpu_fence *user)
> +{
> + struct amdgpu_ring *ring;
> + struct amdgpu_ctx *ctx;
> + struct dma_fence *fence;
> + int r;
> +
> + r = amdgpu_cs_get_ring(adev, user->ip_type, user->ip_instance,
> +user->ring, &ring);
> + if (r)
> + return ERR_PTR(r);
> +
> + ctx = amdgpu_ctx_get(filp->driver_priv, user->ctx_id);
> + if (ctx == NULL)
> + return ERR_PTR(-EINVAL);
> +
> + fence = amdgpu_ctx_get_fence(ctx, ring, user->seq_no);
> + amdgpu_ctx_put(ctx);
> +
> + return fence;
> +}
> +
> +/**
> + * amdgpu_cs_wait_all_fence - wait on all fences to signal
> + *
> + * @adev: amdgpu device
> + * @filp: file private
> + * @wait: wait parameters
> + * @fences: array of drm_amdgpu_fence
> + */
> +static int amdgpu_cs_wait_all_fences(struct amdgpu_device *adev,
> + struct drm_file *filp,
> + union drm_amdgpu_wait_fences *wait,
> + struct drm_amdgpu_fence *fences)
> +{
> + uint32_t fence_count = wait->in.fence_count;
> + unsigned i;
> + long r = 1;
> +
> + for (i = 0; i < fence_count; i++) {
> + struct dma_fence *fence;
> + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns);
> +
> + fence = amdgpu_cs_get_fence(adev, filp, &fences[i]);
> + if (IS_ERR(fence))
> + return PTR_ERR(fence);
> + else if (!fence)
> + continue;
> +
> + r = dma_fence_wait_timeout(fence, true, timeout);
> + if (r < 0)
> + return r;
> +
> + if (r == 0)
> + break;
> + }
> +
> + memset(wait, 0, sizeof(*wait));
> + wait->out.status = (r > 0);
> +
> + return 0;
> +}
> +
> +/**
> + * amdgpu_cs_wait_any_fence - wait on any fence to signal
> + *
> + * @adev: amdgpu device
> + * @filp: file private
> + * @wait: wait parameters
> + * @fences: array of drm_amdgpu_fence
> + */
> +static int amdgpu_cs_wait_any_fence(struct amdgpu_device *adev,
> + struct drm_file *filp,
> + union drm_amdgpu_wait_fences *wait,
> + struct drm_amdgpu_fence *fences)
> +{
> + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns);
> + uint32_t fence_count = wait->in.fence_count;
> + uint32_t first = ~0;
> + struct dma_fence **array;
> + unsigned i;
> + long r;
> +
> + /* Prepare the fence array */
> + array = (struct dma_fence **)kcalloc(fence_count, sizeof(struct
> dma_fence *),
> + GFP_KERNEL);
> +
[PATCH v2] drm: move allocation out of drm_get_format_name()
On Mon, 07 Nov 2016, Eric Engestrom wrote:
> Fixes: 90844f00049e9f42573fd31d7c32e8fd31d3fd07
>
> drm: make drm_get_format_name thread-safe
>
> Signed-off-by: Eric Engestrom
> [danvet: Clarify that the returned pointer must be freed with
> kfree().]
> Signed-off-by: Daniel Vetter
The Fixes: format is:
Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
But is this a fix, really, or just an improvement? What exactly is the
bug being fixed? The commit message is not sufficient.
> Cc: Rob Clark
> Cc: Christian König
> Suggested-by: Ville Syrjälä
> Signed-off-by: Eric Engestrom
> ---
>
> v2: use single-field struct instead of typedef to let the compiler
> enforce the type (Christian König)
Just to make it clear: NAK three times over for v1. This patch is
definitely a better alternative.
>
> ---
> include/drm/drm_fourcc.h| 10 +-
> drivers/gpu/drm/drm_fourcc.c| 14 +++--
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 7 ++---
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 7 ++---
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 3 +-
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 7 ++---
> drivers/gpu/drm/drm_atomic.c| 7 +++--
> drivers/gpu/drm/drm_crtc.c | 7 +++--
> drivers/gpu/drm/drm_framebuffer.c | 7 +++--
> drivers/gpu/drm/drm_modeset_helper.c| 7 +++--
> drivers/gpu/drm/drm_plane.c | 7 +++--
> drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 7 ++---
> drivers/gpu/drm/i915/i915_debugfs.c | 10 +++---
> drivers/gpu/drm/i915/intel_atomic_plane.c | 8 ++---
> drivers/gpu/drm/i915/intel_display.c| 41
> ++---
> drivers/gpu/drm/radeon/atombios_crtc.c | 14 -
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 +-
> 17 files changed, 80 insertions(+), 86 deletions(-)
>
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index dc0aafa..4b03ca0 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -45,6 +45,14 @@ struct drm_format_info {
> u8 vsub;
> };
>
> +/**
> + * struct drm_format_name_buf - name of a DRM format
> + * @str: string buffer containing the format name
> + */
> +struct drm_format_name_buf {
> + char str[32];
> +};
> +
> const struct drm_format_info *__drm_format_info(u32 format);
> const struct drm_format_info *drm_format_info(u32 format);
> uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
> @@ -54,6 +62,6 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
> int drm_format_vert_chroma_subsampling(uint32_t format);
> int drm_format_plane_width(int width, uint32_t format, int plane);
> int drm_format_plane_height(int height, uint32_t format, int plane);
> -char *drm_get_format_name(uint32_t format) __malloc;
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
I wonder if it would be better to make that return "const char *". If
the user really wants to look under the hood, there's buf->str. *shrug*
With the commit message improved,
Reviewed-by: Jani Nikula
>
> #endif /* __DRM_FOURCC_H__ */
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index cbb8b77..99b0b60 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -79,17 +79,13 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t
> depth)
> EXPORT_SYMBOL(drm_mode_legacy_fb_format);
>
> /**
> - * drm_get_format_name - return a string for drm fourcc format
> + * drm_get_format_name - fill a string with a drm fourcc format's name
> * @format: format to compute name of
> + * @buf: caller-supplied buffer
> - *
> - * Note that the buffer returned by this function is owned by the caller
> - * and will need to be freed using kfree().
> */
> -char *drm_get_format_name(uint32_t format)
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf)
> {
> - char *buf = kmalloc(32, GFP_KERNEL);
> -
> - snprintf(buf, 32,
> + snprintf(buf->str, sizeof(buf->str),
>"%c%c%c%c %s-endian (0x%08x)",
>printable_char(format & 0xff),
>printable_char((format >> 8) & 0xff),
> @@ -98,7 +94,7 @@ char *drm_get_format_name(uint32_t format)
>format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
>format);
>
> - return buf;
> + return buf->str;
> }
> EXPORT_SYMBOL(drm_get_format_name);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 199d3f7..2924cdd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2032,7 +2032,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc
> *crtc,
> u32 tmp, viewport_w, viewport_h;
> int r;
> bool bypass_lut =
[PATCH 5/5] reservation: revert "wait only with non-zero timeout specified (v3)" v2
Hi Christian, 2016-10-20 Christian König : > From: Christian König > > This reverts commit fb8b7d2b9d80e1e71f379e57355936bd2b024be9. > > Otherwise signaling might never be activated on the fences. This can > result in infinite waiting with hardware which has unreliable interrupts. > > v2: still return one when the timeout is zero and we don't have any fences. > > Signed-off-by: Christian König > Reviewed-by: Chunming Zhou (v1) > --- > drivers/dma-buf/reservation.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) Reviewed-by: Gustavo Padovan Gustavo
[PATCH 4/5] drm/ttm: fix ttm_bo_wait
Hi Christian, 2016-10-20 Christian König : > From: Christian König > > reservation_object_wait_timeout_rcu() should enable signaling even with a zero > timeout, but ttm_bo_wait() can also be called from atomic context and then it > is not a good idea to do this. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/ttm/ttm_bo.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) Reviewed-by: Gustavo Padovan
[PATCH 3/5] dma-buf/fence: revert "don't wait when specified timeout is zero"
Hi Christian, 2016-10-20 Christian König : > From: Christian König > > This reverts commit 847b19a39e4c9b5e74c40f0842c48b41664cb43c. > > When we don't call the wait function software signaling might never be > activated. This can cause infinite polling loops with unreliable interrupt > driven hardware. > > Signed-off-by: Christian König > Reviewed-by: Chunming Zhou > --- > drivers/dma-buf/fence.c | 3 --- > 1 file changed, 3 deletions(-) Reviewed-by: Gustavo Padovan Gustavo
[PATCH 2/5] dma-buf/fence: make timeout handling in fence_default_wait consistent
Hi Christian, 2016-10-20 Christian König : > From: Christian König > > Kernel functions taking a timeout usually return 1 on success even > when they get a zero timeout. > > Signen-off-by: Christian König > Reviewed-by: Chunming Zhou > --- > drivers/dma-buf/fence.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Gustavo Padovan Gustavo
[PATCH 1/5] drm/msm/mdp5: introduce mdp5_hw_pipe
On Mon, Nov 7, 2016 at 5:38 AM, Archit Taneja wrote:
>
>
> On 11/05/2016 09:55 PM, Rob Clark wrote:
>>
>> Split out the hardware pipe specifics from mdp5_plane. To start, the hw
>> pipes are statically assigned to planes, but next step is to assign the
>> hw pipes during plane->atomic_check() based on requested caps (scaling,
>> YUV, etc). And then hw pipe re-assignment if required if required SMP
>> blocks changes.
>>
>> Signed-off-by: Rob Clark
>> ---
>> drivers/gpu/drm/msm/Makefile | 1 +
>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 126
>> +++---
>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 7 +-
>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c | 43 ++
>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h | 39 +
>> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 66 +++-
>> 6 files changed, 197 insertions(+), 85 deletions(-)
>> create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c
>> create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h
>>
>> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
>> index fb5be3e..90f66c4 100644
>> --- a/drivers/gpu/drm/msm/Makefile
>> +++ b/drivers/gpu/drm/msm/Makefile
>> @@ -37,6 +37,7 @@ msm-y := \
>> mdp/mdp5/mdp5_irq.o \
>> mdp/mdp5/mdp5_mdss.o \
>> mdp/mdp5/mdp5_kms.o \
>> + mdp/mdp5/mdp5_pipe.o \
>> mdp/mdp5/mdp5_plane.o \
>> mdp/mdp5/mdp5_smp.o \
>> msm_atomic.o \
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> index f1288c7..d3d45ed 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
>> @@ -119,6 +119,10 @@ static void mdp5_kms_destroy(struct msm_kms *kms)
>> {
>> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
>> struct msm_gem_address_space *aspace = mdp5_kms->aspace;
>> + int i;
>> +
>> + for (i = 0; i < mdp5_kms->num_hwpipes; i++)
>> + mdp5_pipe_destroy(mdp5_kms->hwpipes[i]);
>>
>> if (aspace) {
>> aspace->mmu->funcs->detach(aspace->mmu,
>> @@ -323,15 +327,6 @@ static int modeset_init_intf(struct mdp5_kms
>> *mdp5_kms, int intf_num)
>>
>> static int modeset_init(struct mdp5_kms *mdp5_kms)
>> {
>> - static const enum mdp5_pipe rgb_planes[] = {
>> - SSPP_RGB0, SSPP_RGB1, SSPP_RGB2, SSPP_RGB3,
>> - };
>> - static const enum mdp5_pipe vig_planes[] = {
>> - SSPP_VIG0, SSPP_VIG1, SSPP_VIG2, SSPP_VIG3,
>> - };
>> - static const enum mdp5_pipe dma_planes[] = {
>> - SSPP_DMA0, SSPP_DMA1,
>> - };
>> struct drm_device *dev = mdp5_kms->dev;
>> struct msm_drm_private *priv = dev->dev_private;
>> const struct mdp5_cfg_hw *hw_cfg;
>> @@ -339,58 +334,34 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
>>
>> hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg);
>>
>> - /* construct CRTCs and their private planes: */
>> - for (i = 0; i < hw_cfg->pipe_rgb.count; i++) {
>> + /* Construct planes equaling the number of hw pipes, and CRTCs
>> +* for the N layer-mixers (LM). The first N planes become primary
>> +* planes for the CRTCs, with the remainder as overlay planes:
>> +*/
>
>
> Jfyi, we might need to change this a bit in the future. It'll be better to
> get the max number of displays connected on our platform via parsing DT,
> etc, and calculate CRTCs based on that, and not number of layermixers. Maybe
> add a couple for writeback too. This way, we get the right number of
> CRTCs, and we don't rely on #LMs, since we can have 2 per crtc
> in the future.
I *guess* when we get to that stage, we'll dynamically assign LM's
too, in a similar way as hwpipe. And I suppose we could also put a
cap on # of crtc's based on # of encoders?
BR,
-R
> Reviewed-by: Archit Taneja
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
[PATCH v2] drm: move allocation out of drm_get_format_name()
On Sun, Nov 6, 2016 at 7:48 PM, Eric Engestrom wrote:
> Fixes: 90844f00049e9f42573fd31d7c32e8fd31d3fd07
>
> drm: make drm_get_format_name thread-safe
>
> Signed-off-by: Eric Engestrom
> [danvet: Clarify that the returned pointer must be freed with
> kfree().]
> Signed-off-by: Daniel Vetter
>
> Cc: Rob Clark
> Cc: Christian König
> Suggested-by: Ville Syrjälä
> Signed-off-by: Eric Engestrom
Thanks,
Acked-by: Rob Clark
> ---
>
> v2: use single-field struct instead of typedef to let the compiler
> enforce the type (Christian König)
>
> ---
> include/drm/drm_fourcc.h| 10 +-
> drivers/gpu/drm/drm_fourcc.c| 14 +++--
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 7 ++---
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 7 ++---
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 3 +-
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 7 ++---
> drivers/gpu/drm/drm_atomic.c| 7 +++--
> drivers/gpu/drm/drm_crtc.c | 7 +++--
> drivers/gpu/drm/drm_framebuffer.c | 7 +++--
> drivers/gpu/drm/drm_modeset_helper.c| 7 +++--
> drivers/gpu/drm/drm_plane.c | 7 +++--
> drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 7 ++---
> drivers/gpu/drm/i915/i915_debugfs.c | 10 +++---
> drivers/gpu/drm/i915/intel_atomic_plane.c | 8 ++---
> drivers/gpu/drm/i915/intel_display.c| 41
> ++---
> drivers/gpu/drm/radeon/atombios_crtc.c | 14 -
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 +-
> 17 files changed, 80 insertions(+), 86 deletions(-)
>
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index dc0aafa..4b03ca0 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -45,6 +45,14 @@ struct drm_format_info {
> u8 vsub;
> };
>
> +/**
> + * struct drm_format_name_buf - name of a DRM format
> + * @str: string buffer containing the format name
> + */
> +struct drm_format_name_buf {
> + char str[32];
> +};
> +
> const struct drm_format_info *__drm_format_info(u32 format);
> const struct drm_format_info *drm_format_info(u32 format);
> uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
> @@ -54,6 +62,6 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
> int drm_format_vert_chroma_subsampling(uint32_t format);
> int drm_format_plane_width(int width, uint32_t format, int plane);
> int drm_format_plane_height(int height, uint32_t format, int plane);
> -char *drm_get_format_name(uint32_t format) __malloc;
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
>
> #endif /* __DRM_FOURCC_H__ */
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index cbb8b77..99b0b60 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -79,17 +79,13 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t
> depth)
> EXPORT_SYMBOL(drm_mode_legacy_fb_format);
>
> /**
> - * drm_get_format_name - return a string for drm fourcc format
> + * drm_get_format_name - fill a string with a drm fourcc format's name
> * @format: format to compute name of
> + * @buf: caller-supplied buffer
> - *
> - * Note that the buffer returned by this function is owned by the caller
> - * and will need to be freed using kfree().
> */
> -char *drm_get_format_name(uint32_t format)
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf)
> {
> - char *buf = kmalloc(32, GFP_KERNEL);
> -
> - snprintf(buf, 32,
> + snprintf(buf->str, sizeof(buf->str),
> "%c%c%c%c %s-endian (0x%08x)",
> printable_char(format & 0xff),
> printable_char((format >> 8) & 0xff),
> @@ -98,7 +94,7 @@ char *drm_get_format_name(uint32_t format)
> format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
> format);
>
> - return buf;
> + return buf->str;
> }
> EXPORT_SYMBOL(drm_get_format_name);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 199d3f7..2924cdd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2032,7 +2032,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc
> *crtc,
> u32 tmp, viewport_w, viewport_h;
> int r;
> bool bypass_lut = false;
> - char *format_name;
> + struct drm_format_name_buf format_name;
>
> /* no fb bound */
> if (!atomic && !crtc->primary->fb) {
> @@ -2144,9 +2144,8 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc
> *crtc,
> bypass_lut = true;
> break;
> default:
> - format_name = drm_get_format_name(target_fb->pixel_format);
> - DRM_ERROR("Unsupported screen f
[Bug 98150] DRM/IMX Kernel 4.8.0 oops when attach a FB with DRM_FORMAT_YUV420
https://bugs.freedesktop.org/show_bug.cgi?id=98150 gnuiyl at gmail.com changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #1 from gnuiyl at gmail.com --- Should be fixed in v4.9-rc4 via commit e73aca5184ad9fc948ba22b4d35dce11db35bb25. . -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161107/bb596270/attachment.html>
[PATCH v4 0/3] Add initial ZTE VOU DRM/KMS driver
Hi David, Daniel, On Mon, Oct 31, 2016 at 05:17:22PM +0800, Shawn Guo wrote: > From: Shawn Guo > > The series adds the initial ZTE VOU display controller DRM/KMS driver. > There are still some features to be added, like overlay plane, scaling, > and more output devices support. But it's already useful with dual > CRTCs and HDMI display working. After a few rounds of reviewing (thanks to all reviewers), it seems that the series is ready for merging, I guess? If so, could you suggest how we proceed? I can prepare a pull request if you like. Thanks. Shawn > Changes for v4: > - Move the hardware setup done in zx_hdmi_hw_init() and clock enable >into .enable hook of drm_encoder_helper_funcs. > - Compare pipe to crtc->index instead of using our own index counter. > - Save the pipe check in zx_vou_enable[disable]_vblank by putting frame >interrupt bit into zx_crtc_bits. > - Use DRM_DEV_* for log messages instead of old dev_* functions > - Return directly in case of -ETIMEDOUT in zx_hdmi_i2c_read to simplify >the code for error condition. > - Shuffle things around to make the crtc and plane state check in >zx_gl_plane_atomic_check a bit clearer > - Move hardware register definitions into headers instead of disturbing >C file reading. > - Save the call to drm_connector_unregister(), so that >drm_connector_cleanup can directly be used as drm_connector_funcs >.destroy hook. > - Move vblank notification from .atomic_begin hook to .atomic_flush, >so that vblank event is sent to user space after planes are committed >rather than before. > > Changes for v3: > - Rebase to v4.9-rc1 > - Update bindings doc to use 'ranges' for address translation between >parent and child devices. > - Call drm_dev_register() last in bind function and drm_dev_unregister() >first in unbind, so that drm_connector_regiser() can be saved from >HDMI driver. > - Instead of using open-coded drm_do_get_edid(), add an I2C adapter for >HDMI DDC read and use drm_get_edid(). > - Improve the plane .atomic_check implementation by calling helper >function drm_plane_helper_check_state(). > - Rename zx_crtc.c to zx_vou.c to avoid the confusion that the file >implements crtc instance. > - Store vou pointer in zx_crtc, so that we do not need to embed the >pointer in zx_drm_private. > - Create zx_readl/zx_writel/zx_writel_mask for register access. > - Define a few macro helpers to ease the register bit setting, like >SYNC_WIDE, BACK_PORCH and FRONT_PORCH. > - Define main/aux channel specific register offset and bits in zx_crtc >to save the use of is_main check > - Sort include headers alphabetically > - Removing encoder pointer out of the structure and constify struct >vou_inf > - Add log message for error conditions > - Make the function calls in teardown path asymmetrical > - A few coding style improvements like defining macro for sub-module >address and changing code to save indentation level > - Add a MAINTAINERS entry for ZTE ZX DRM driver > > Changes for v2: > - Change device tree bindings to kill the virtual display-subsystem >node make VOU the parent node. > > Shawn Guo (3): > dt-bindings: add bindings doc for ZTE VOU display controller > drm: zte: add initial vou drm driver > MAINTAINERS: add an entry for ZTE ZX DRM driver > > .../devicetree/bindings/display/zte,vou.txt| 84 +++ > MAINTAINERS| 7 + > drivers/gpu/drm/Kconfig| 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/zte/Kconfig| 8 + > drivers/gpu/drm/zte/Makefile | 7 + > drivers/gpu/drm/zte/zx_drm_drv.c | 267 + > drivers/gpu/drm/zte/zx_drm_drv.h | 36 ++ > drivers/gpu/drm/zte/zx_hdmi.c | 624 +++ > drivers/gpu/drm/zte/zx_hdmi_regs.h | 56 ++ > drivers/gpu/drm/zte/zx_plane.c | 299 ++ > drivers/gpu/drm/zte/zx_plane.h | 26 + > drivers/gpu/drm/zte/zx_plane_regs.h| 91 +++ > drivers/gpu/drm/zte/zx_vou.c | 661 > + > drivers/gpu/drm/zte/zx_vou.h | 46 ++ > drivers/gpu/drm/zte/zx_vou_regs.h | 157 + > 16 files changed, 2372 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/zte,vou.txt > create mode 100644 drivers/gpu/drm/zte/Kconfig > create mode 100644 drivers/gpu/drm/zte/Makefile > create mode 100644 drivers/gpu/drm/zte/zx_drm_drv.c > create mode 100644 drivers/gpu/drm/zte/zx_drm_drv.h > create mode 100644 drivers/gpu/drm/zte/zx_hdmi.c > create mode 100644 drivers/gpu/drm/zte/zx_hdmi_regs.h > create mode 100644 drivers/gpu/drm/zte/zx_plane.c > create mode 100644 drivers/gpu/drm/zte/zx_plane.h > create mode 100644 drivers
[Spice-devel] [drm/qxl v2 7/7] qxl: Allow resolution which are not multiple of 8
Hi, > I think we should try it an see, Ok, lets try. I'll go pick them up and prepare a pull with this and some virtio-gpu bits, Gerd
[PATCH] drm/exynos/hdmi: refactor infoframe code
On 07.11.2016 02:45, Inki Dae wrote:
>
> 2016ë
10ì 26ì¼ 21:36ì Andrzej Hajda ì´(ê°) ì´ ê¸:
>> Use core helpers to generate infoframes and generate vendor frame if
>> necessary.
>>
>> Signed-off-by: Andrzej Hajda
>> ---
>> drivers/gpu/drm/exynos/exynos_hdmi.c | 141
>> ++-
>> drivers/gpu/drm/exynos/regs-hdmi.h | 2 +
>> 2 files changed, 42 insertions(+), 101 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index e8fb6ef..1bb2df7 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -47,19 +47,6 @@
>>
>> #define HOTPLUG_DEBOUNCE_MS 1100
>>
>> -/* AVI header and aspect ratio */
>> -#define HDMI_AVI_VERSION0x02
>> -#define HDMI_AVI_LENGTH 0x0d
>> -
>> -/* AUI header info */
>> -#define HDMI_AUI_VERSION0x01
>> -#define HDMI_AUI_LENGTH 0x0a
>> -
>> -/* AVI active format aspect ratio */
>> -#define AVI_SAME_AS_PIC_ASPECT_RATIO0x08
>> -#define AVI_4_3_CENTER_RATIO0x09
>> -#define AVI_16_9_CENTER_RATIO 0x0a
>> -
>> enum hdmi_type {
>> HDMI_TYPE13,
>> HDMI_TYPE14,
>> @@ -131,7 +118,6 @@ struct hdmi_context {
>> booldvi_mode;
>> struct delayed_work hotplug_work;
>> struct drm_display_mode current_mode;
>> -u8 cea_video_id;
>> const struct hdmi_driver_data *drv_data;
>>
>> void __iomem*regs;
>> @@ -681,6 +667,13 @@ static inline void hdmi_reg_writev(struct hdmi_context
>> *hdata, u32 reg_id,
>> }
>> }
>>
>> +static inline void hdmi_reg_write_buf(struct hdmi_context *hdata, u32
>> reg_id,
>> + u8 *buf, int size)
>> +{
>> +for (reg_id = hdmi_map_reg(hdata, reg_id); size; --size, reg_id += 4)
>> +writel(*buf++, hdata->regs + reg_id);
>> +}
>> +
>> static inline void hdmi_reg_writemask(struct hdmi_context *hdata,
>> u32 reg_id, u32 value, u32 mask)
>> {
>> @@ -762,93 +755,50 @@ static int hdmi_clk_set_parents(struct hdmi_context
>> *hdata, bool to_phy)
>> return ret;
>> }
>>
>> -static u8 hdmi_chksum(struct hdmi_context *hdata,
>> -u32 start, u8 len, u32 hdr_sum)
>> -{
>> -int i;
>> -
>> -/* hdr_sum : header0 + header1 + header2
>> -* start : start address of packet byte1
>> -* len : packet bytes - 1 */
>> -for (i = 0; i < len; ++i)
>> -hdr_sum += 0xff & hdmi_reg_read(hdata, start + i * 4);
>> -
>> -/* return 2's complement of 8 bit hdr_sum */
>> -return (u8)(~(hdr_sum & 0xff) + 1);
>> -}
>> -
>> -static void hdmi_reg_infoframe(struct hdmi_context *hdata,
>> -union hdmi_infoframe *infoframe)
>> +static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>> {
>> -u32 hdr_sum;
>> -u8 chksum;
>> -u8 ar;
>> +union hdmi_infoframe frm;
>> +u8 buf[25];
>> +int ret;
>>
>> if (hdata->dvi_mode) {
>> -hdmi_reg_writeb(hdata, HDMI_VSI_CON,
>> -HDMI_VSI_CON_DO_NOT_TRANSMIT);
>> hdmi_reg_writeb(hdata, HDMI_AVI_CON,
>> HDMI_AVI_CON_DO_NOT_TRANSMIT);
>> +hdmi_reg_writeb(hdata, HDMI_VSI_CON,
>> +HDMI_VSI_CON_DO_NOT_TRANSMIT);
>> hdmi_reg_writeb(hdata, HDMI_AUI_CON, HDMI_AUI_CON_NO_TRAN);
>> return;
>> }
>>
>> -switch (infoframe->any.type) {
>> -case HDMI_INFOFRAME_TYPE_AVI:
>> +ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
>> +&hdata->current_mode);
>> +if (ret >= 0)
> Seems above condition is not clear becuase
> drm_hdmi_avi_infoframe_from_display_mode function returns 0 or negative value.
So it means 'there is no error', I can change it to '!ret' or 'ret == 0'
if you prefer, I have just used '>= 0' to be more concise with next
error check.
>
>> +ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
>> +if (ret > 0) {
>> hdmi_reg_writeb(hdata, HDMI_AVI_CON, HDMI_AVI_CON_EVERY_VSYNC);
> I think above code should be called when
> drm_hdmi_avi_infoframe_from_display_mode function returned 0 and the value
> from hdmi_avi_infoframe_pack function is more than 1.
Why do you think 'more than 1' is better than 'more than 0' in this
case? hdmi_avi_infoframe_pack returns length of generated frame or
negative value in case of error,
so any positive value is OK, there is no special meaning for '1'.
>
>> -hdmi_reg_writeb(hdata, HDMI_AVI_HEADER0, infoframe->any.type);
>> -hdmi_reg_writeb(hdata, HDMI_AVI_HEADER1,
>> -infoframe->any.version);
>> -hdmi_reg_writeb(hdata, HDMI_AVI_HEADER2, infoframe->any.length);
>> -hdr_
[PATCH 2/2] drm/amdgpu: add the interface of waiting multiple fences (v4)
Am 07.11.2016 um 02:10 schrieb Gustavo Padovan:
> Hi Alex,
>
> 2016-11-04 Alex Deucher :
>
>> From: Junwei Zhang
>>
>> v2: agd: rebase and squash in all the previous optimizations and
>> changes so everything compiles.
>> v3: squash in Slava's 32bit build fix
>> v4: rebase on drm-next (fence -> dma_fence),
>> squash in Monk's ioctl update patch
>>
>> Signed-off-by: Junwei Zhang
>> Reviewed-by: Monk Liu
>> Reviewed-by: Jammy Zhou
>> Signed-off-by: Alex Deucher
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 173
>>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +-
>> include/uapi/drm/amdgpu_drm.h | 28 ++
>> 5 files changed, 205 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index dc98ceb..7a94a3c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1212,6 +1212,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void
>> *data,
>> struct drm_file *filp);
>> int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file
>> *filp);
>> int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, struct
>> drm_file *filp);
>> +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data,
>> +struct drm_file *filp);
>>
>> int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data,
>> struct drm_file *filp);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index 2728805..2004836 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -1130,6 +1130,179 @@ int amdgpu_cs_wait_ioctl(struct drm_device *dev,
>> void *data,
>> }
>>
>> /**
>> + * amdgpu_cs_get_fence - helper to get fence from drm_amdgpu_fence
>> + *
>> + * @adev: amdgpu device
>> + * @filp: file private
>> + * @user: drm_amdgpu_fence copied from user space
>> + */
>> +static struct dma_fence *amdgpu_cs_get_fence(struct amdgpu_device *adev,
>> + struct drm_file *filp,
>> + struct drm_amdgpu_fence *user)
>> +{
>> +struct amdgpu_ring *ring;
>> +struct amdgpu_ctx *ctx;
>> +struct dma_fence *fence;
>> +int r;
>> +
>> +r = amdgpu_cs_get_ring(adev, user->ip_type, user->ip_instance,
>> + user->ring, &ring);
>> +if (r)
>> +return ERR_PTR(r);
>> +
>> +ctx = amdgpu_ctx_get(filp->driver_priv, user->ctx_id);
>> +if (ctx == NULL)
>> +return ERR_PTR(-EINVAL);
>> +
>> +fence = amdgpu_ctx_get_fence(ctx, ring, user->seq_no);
>> +amdgpu_ctx_put(ctx);
>> +
>> +return fence;
>> +}
>> +
>> +/**
>> + * amdgpu_cs_wait_all_fence - wait on all fences to signal
>> + *
>> + * @adev: amdgpu device
>> + * @filp: file private
>> + * @wait: wait parameters
>> + * @fences: array of drm_amdgpu_fence
>> + */
>> +static int amdgpu_cs_wait_all_fences(struct amdgpu_device *adev,
>> + struct drm_file *filp,
>> + union drm_amdgpu_wait_fences *wait,
>> + struct drm_amdgpu_fence *fences)
>> +{
>> +uint32_t fence_count = wait->in.fence_count;
>> +unsigned i;
>> +long r = 1;
>> +
>> +for (i = 0; i < fence_count; i++) {
>> +struct dma_fence *fence;
>> +unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns);
>> +
>> +fence = amdgpu_cs_get_fence(adev, filp, &fences[i]);
>> +if (IS_ERR(fence))
>> +return PTR_ERR(fence);
>> +else if (!fence)
>> +continue;
>> +
>> +r = dma_fence_wait_timeout(fence, true, timeout);
>> +if (r < 0)
>> +return r;
>> +
>> +if (r == 0)
>> +break;
>> +}
>> +
>> +memset(wait, 0, sizeof(*wait));
>> +wait->out.status = (r > 0);
>> +
>> +return 0;
>> +}
>> +
>> +/**
>> + * amdgpu_cs_wait_any_fence - wait on any fence to signal
>> + *
>> + * @adev: amdgpu device
>> + * @filp: file private
>> + * @wait: wait parameters
>> + * @fences: array of drm_amdgpu_fence
>> + */
>> +static int amdgpu_cs_wait_any_fence(struct amdgpu_device *adev,
>> +struct drm_file *filp,
>> +union drm_amdgpu_wait_fences *wait,
>> +struct drm_amdgpu_fence *fences)
>> +{
>> +unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns);
>> +uint32_t fence_count = wait->in.fence_count;
>> +uint32_t first = ~0;
>> +struct dma_fence **array;
>> +unsigned i;
>> +long r;
>> +
>> +/* Prepa
[drm/qxl v3 7/7] qxl: Allow resolution which are not multiple of 8
The use of drm_cvt_mode() in qxl_add_monitors_config_modes() means that the resolutions we are going to present to user-space are going to be rounded down to a multiple of 8. In the QXL arbitrary resolution case, this is not useful. This commit forces the actual width/height that was requested by the client in the drm_display_mode structure rather than keeping the rounded version. Signed-off-by: Christophe Fergeau --- drivers/gpu/drm/qxl/qxl_display.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 518333c..fa99481 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -199,6 +199,9 @@ static int qxl_add_monitors_config_modes(struct drm_connector *connector, mode = drm_cvt_mode(dev, head->width, head->height, 60, false, false, false); mode->type |= DRM_MODE_TYPE_PREFERRED; + mode->hdisplay = head->width; + mode->vdisplay = head->height; + drm_mode_set_name(mode); *pwidth = head->width; *pheight = head->height; drm_mode_probed_add(connector, mode); -- 2.9.3
[drm/qxl v3 6/7] qxl: Don't notify userspace when monitors config is unchanged
When the QXL driver receives a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt,
we currently always notify userspace that there was some hotplug event.
However, gnome-shell/mutter is reacting to this event by attempting a
resolution change, which it does by issueing drmModeRmFB, drmModeAddFB,
and then drmModeSetCrtc. This has the side-effect of causing
qxl_crtc_mode_set() to tell the QXL virtual hardware that a primary
surface was destroyed and created. After going through QEMU and then the
remote SPICE client, a new identical monitors config message will be
sent, resulting in a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt to
be emitted, and the same scenario occurring again.
As destroying/creating the primary surface causes a visible screen
flicker, this makes the guest hard to use (
https://bugzilla.redhat.com/show_bug.cgi?id=1266484 ).
This commit checks if the screen configuration we received is the same
one as the current one, and does not notify userspace about it if that's
the case.
Signed-off-by: Christophe Fergeau
---
drivers/gpu/drm/qxl/qxl_display.c | 62 ---
1 file changed, 52 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 8cf5177..518333c 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -57,11 +57,18 @@ static void qxl_alloc_client_monitors_config(struct
qxl_device *qdev, unsigned c
qdev->client_monitors_config->count = count;
}
+enum {
+ MONITORS_CONFIG_MODIFIED,
+ MONITORS_CONFIG_UNCHANGED,
+ MONITORS_CONFIG_BAD_CRC,
+};
+
static int qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
{
int i;
int num_monitors;
uint32_t crc;
+ int status = MONITORS_CONFIG_UNCHANGED;
num_monitors = qdev->rom->client_monitors_config.count;
crc = crc32(0, (const uint8_t *)&qdev->rom->client_monitors_config,
@@ -70,7 +77,7 @@ static int qxl_display_copy_rom_client_monitors_config(struct
qxl_device *qdev)
qxl_io_log(qdev, "crc mismatch: have %X (%zd) != %X\n", crc,
sizeof(qdev->rom->client_monitors_config),
qdev->rom->client_monitors_config_crc);
- return 1;
+ return MONITORS_CONFIG_BAD_CRC;
}
if (num_monitors > qdev->monitors_config->max_allowed) {
DRM_DEBUG_KMS("client monitors list will be truncated: %d <
%d\n",
@@ -79,6 +86,10 @@ static int
qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
} else {
num_monitors = qdev->rom->client_monitors_config.count;
}
+ if (qdev->client_monitors_config
+ && (num_monitors != qdev->client_monitors_config->count)) {
+ status = MONITORS_CONFIG_MODIFIED;
+ }
qxl_alloc_client_monitors_config(qdev, num_monitors);
/* we copy max from the client but it isn't used */
qdev->client_monitors_config->max_allowed =
@@ -88,17 +99,39 @@ static int
qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
&qdev->rom->client_monitors_config.heads[i];
struct qxl_head *client_head =
&qdev->client_monitors_config->heads[i];
- client_head->x = c_rect->left;
- client_head->y = c_rect->top;
- client_head->width = c_rect->right - c_rect->left;
- client_head->height = c_rect->bottom - c_rect->top;
- client_head->surface_id = 0;
- client_head->id = i;
- client_head->flags = 0;
+ if (client_head->x != c_rect->left) {
+ client_head->x = c_rect->left;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->y != c_rect->top) {
+ client_head->y = c_rect->top;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->width != c_rect->right - c_rect->left) {
+ client_head->width = c_rect->right - c_rect->left;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->height != c_rect->bottom - c_rect->top) {
+ client_head->height = c_rect->bottom - c_rect->top;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->surface_id != 0) {
+ client_head->surface_id = 0;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->id != i) {
+ client_head->id = i;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->flags != 0) {
+ client_head->flags = 0;
+
