Re: [PATCH 1/2] drm/ingenic: Switch IPU plane to type OVERLAY

2021-03-30 Thread Simon Ser
On Tuesday, March 30th, 2021 at 1:53 PM, Paul Cercueil  
wrote:

> > I don't know about this driver but… is this really the same as the
> > previous
> > condition? The previous condition would match two planes, this one
> > seems to
> > match only a single plane. What am I missing?
>
> There are three planes, which we will call here f0, f1, and ipu.
> Previously, the "plane->type == DRM_PLANE_TYPE_PRIMARY" matched f1 and
> ipu. Since ipu is now OVERLAY we have to change the condition or the
> behaviour will be different, as otherwise it would only match the f1
> plane.

Oh okay, I thought f0 was one of the primary planes, but it's not.
Thanks for the explanation.

For the user-space visible change:

Acked-by: Simon Ser 


Re: [PATCH 1/2] drm/ingenic: Switch IPU plane to type OVERLAY

2021-03-30 Thread Simon Ser
> It should have been an OVERLAY from the beginning. The documentation
> stipulates that there should be an unique PRIMARY plane per CRTC.

Thanks for the quick patch! One comment below…

> Fixes: fc1acf317b01 ("drm/ingenic: Add support for the IPU")
> Cc:  # 5.8+
> Signed-off-by: Paul Cercueil 
> ---
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 11 +--
>  drivers/gpu/drm/ingenic/ingenic-ipu.c |  2 +-
>  2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index 29742ec5ab95..09225b770bb8 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -419,7 +419,7 @@ static void ingenic_drm_plane_enable(struct ingenic_drm 
> *priv,
>   unsigned int en_bit;
>
>   if (priv->soc_info->has_osd) {
> - if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> + if (plane != >f0)

I don't know about this driver but… is this really the same as the previous
condition? The previous condition would match two planes, this one seems to
match only a single plane. What am I missing?



Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

2021-03-29 Thread Simon Ser
On Monday, March 29th, 2021 at 5:39 PM, Paul Cercueil  
wrote:

> Ok, I read that as "all drivers should provide AT LEAST one primary
> plane per CRTC", and not as "all drivers should provide ONE AND ONLY
> ONE primary plane per CRTC". My bad.

Yeah, it's a little complicated to document, because it's possible for
a primary plane to be compatible with multiple CRTCs… We tried to
improve this [1] recently.

[1]: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-abstraction


Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

2021-03-29 Thread Simon Ser
On Monday, March 29th, 2021 at 5:32 PM, Paul Cercueil  
wrote:

> Making the second plane an overlay would break the ABI, which is never
> something I'm happy to do; but I'd prefer to do it now than later.

Yeah, I wonder if some user-space depends on this behavior somehow?

> I still have concerns about the user-space being "clever" enough to
> know it can disable the primary plane. Can e.g. wlroots handle that?

wlroots will always pick the first primary plane, and will never use
overlays. The plan is to use libliftoff [1] to make use of overlay
planes. libliftoff should already support the scenario you describe.

I think Weston supports that too.

[1]: https://github.com/emersion/libliftoff


Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

2021-03-29 Thread Simon Ser
On Monday, March 29th, 2021 at 4:07 PM, Maxime Ripard  wrote:

> Since it looks like you have two mutually exclusive planes, just expose
> one and be done with it?

You can expose the other as an overlay. Clever user-space will be able
to figure out that the more advanced plane can be used if the primary
plane is disabled.

But yeah, I don't think exposing two primary planes makes sense. The
"primary" bit is just there for legacy user-space, it's a hint that
it's the best plane to light up for fullscreen content. It has no other
significance than that, and in particular it doesn't mean that it's
incompatible with other primary planes.


Re: [PATCH] drm: DON'T require each CRTC to have a unique primary plane

2021-03-27 Thread Simon Ser
On Saturday, March 27th, 2021 at 12:22 PM, Paul Cercueil  
wrote:

> The ingenic-drm driver has two mutually exclusive primary planes
> already; so the fact that a CRTC must have one and only one primary
> plane is an invalid assumption.

Why does this driver expose two primary planes, if it only has a single
CRTC?


Re: [PATCH] docs: gpu: fix typo

2021-03-08 Thread Simon Ser
R-b me. Pushed, thanks!


Re: [PATCH] drm/amdgpu/display: initialize the variable 'i'

2021-02-23 Thread Simon Ser
On Tuesday, February 23rd, 2021 at 6:42 PM, Alex Deucher 
 wrote:

> yeah, fdo ran out of disk space so I moved to gitlab:
>
> https://gitlab.freedesktop.org/agd5f/linux/-/commits/drm-next

Ah, thanks for the info, my bad!


Re: [PATCH] drm/amdgpu/display: initialize the variable 'i'

2021-02-22 Thread Simon Ser
On Tuesday, February 23rd, 2021 at 12:44 AM, Nathan Chancellor 
 wrote:

> On Mon, Feb 22, 2021 at 11:05:17PM +0000, Simon Ser wrote:
> > On Monday, February 22nd, 2021 at 8:25 PM, Souptick Joarder 
> >  wrote:
> >
> > > >> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:9804:38:
> > > >> warning: variable 'i' is uninitialized when used here
> > > >> [-Wuninitialized]
> > >timing  = >detailed_timings[i];
> > >  ^
> > >drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:9714:7:
> > > note: initialize the variable 'i' to silence this warning
> > >int i;
> > > ^
> > >  = 0
> > >1 warning generated.
> > >
> > > Initialize the variable 'i'.
> >
> > Hm, I see this variable already initialized in the loop:
> >
> > for (i = 0; i < 4; i++) {
> >
> > This is the branch agd5f/drm-next.
>
> That is in the
>
>   if (amdgpu_dm_connector->dc_sink->sink_signal == 
> SIGNAL_TYPE_DISPLAY_PORT
>   || amdgpu_dm_connector->dc_sink->sink_signal == 
> SIGNAL_TYPE_EDP) {
>
> branch not the
>
>   } else if (edid && amdgpu_dm_connector->dc_sink->sink_signal == 
> SIGNAL_TYPE_HDMI_TYPE_A) {
>
> branch, where i is indeed used uninitialized like clang complains about.
>
> I am not at all familiar with the code so I cannot say if this fix is
> the proper one but it is definitely a legitimate issue.

I think you have an outdated branch. In my checkout, i is not used in the first
branch, and is initialized in the second one.

https://cgit.freedesktop.org/~agd5f/linux/tree/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c?h=drm-next#n9700


Re: [PATCH] drm/amdgpu/display: initialize the variable 'i'

2021-02-22 Thread Simon Ser
On Monday, February 22nd, 2021 at 8:25 PM, Souptick Joarder 
 wrote:

> >> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:9804:38:
> >> warning: variable 'i' is uninitialized when used here
> >> [-Wuninitialized]
>timing  = >detailed_timings[i];
>  ^
>drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:9714:7:
> note: initialize the variable 'i' to silence this warning
>int i;
> ^
>  = 0
>1 warning generated.
>
> Initialize the variable 'i'.

Hm, I see this variable already initialized in the loop:

for (i = 0; i < 4; i++) {

This is the branch agd5f/drm-next.

> Reported-by: kernel test robot 
> Signed-off-by: Souptick Joarder 
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index a22a53d..e96d3d9 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -9717,7 +9717,7 @@ static bool parse_hdmi_amd_vsdb(struct 
> amdgpu_dm_connector *aconnector,
>  void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>   struct edid *edid)
>  {
> - int i;
> + int i = 0;
>   struct detailed_timing *timing;
>   struct detailed_non_pixel *data;
>   struct detailed_data_monitor_range *range;
> --
> 1.9.1


Re: DMA-buf and uncached system memory

2021-02-15 Thread Simon Ser
On Monday, February 15th, 2021 at 9:58 AM, Christian König 
 wrote:

> we are currently working an Freesync and direct scan out from system
> memory on AMD APUs in A+A laptops.
>
> On problem we stumbled over is that our display hardware needs to scan
> out from uncached system memory and we currently don't have a way to
> communicate that through DMA-buf.
>
> For our specific use case at hand we are going to implement something
> driver specific, but the question is should we have something more
> generic for this?
>
> After all the system memory access pattern is a PCIe extension and as
> such something generic.

Intel also needs uncached system memory if I'm not mistaken?

Where are the buffers allocated? If GBM, then it needs to allocate memory that
can be scanned out if the USE_SCANOUT flag is set or if a scanout-capable
modifier is picked.

If this is about communicating buffer constraints between different components
of the stack, there were a few proposals about it. The most recent one is [1].

Simon

[1]: https://xdc2020.x.org/event/9/contributions/615/


Re: [PATCH v3] kcmp: Support selection of SYS_kcmp without CHECKPOINT_RESTORE

2021-02-12 Thread Simon Ser
On Friday, February 12th, 2021 at 1:57 PM, Emil Velikov 
 wrote:

> On Fri, 5 Feb 2021 at 22:01, Chris Wilson  wrote:
> >
> > Userspace has discovered the functionality offered by SYS_kcmp and has
> > started to depend upon it. In particular, Mesa uses SYS_kcmp for
> > os_same_file_description() in order to identify when two fd (e.g. device
> > or dmabuf)
>
> As you rightfully point out, SYS_kcmp is a bit of a two edged sword.
> While you mention the CONFIG issue, there is also a portability aspect
> (mesa runs on more than just linux) and as well as sandbox filtering
> of the extra syscall.
>
> Last time I looked, the latter was still an issue and mesa was using
> SYS_kcmp to compare device node fds.
> A far shorter and more portable solution is possible, so let me
> prepare a Mesa patch.

Comparing two DMA-BUFs can be done with their inode number, I think.

Comparing two device FDs is more subtle, because of GEM handle
ref'counting. You sometimes really want to check whether two FDs are
backed by the same file *description*. See [1] for details.

[1]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110


Re: [PATCH] I was wondering why I can't set the resolution to 2560x1080, while in windows 7 I can without a problem. I looked at the radeon driver code and found it doesn't support this resolution. So

2021-02-07 Thread Simon Ser
Please keep the commit message short. You probbly want to send this patch
to amd-...@lists.freedesktop.org instead of dri-devel.


Re: [Linaro-mm-sig] [PATCH v3] dmabuf: Add the capability to expose DMA-BUF stats in sysfs

2021-01-28 Thread Simon Ser
On Thursday, January 28th, 2021 at 1:03 PM, Sumit Semwal 
 wrote:

> Since he didn't comment over Hridya's last clarification about the
> tracepoints to track total GPU memory allocations being orthogonal to
> this series, I assumed he agreed with it.

IIRC he's away this week. (I don't remember when he comes back.)

> Daniel, do you still have objections around adding this patch in?

(Adding him explicitly in CC)


Re: [PATCH 1/5] drm/nouveau/kms/nv50-: Use drm_dbg_kms() in crc.c

2021-01-19 Thread Simon Ser
> Cc: Martin Peres 
> Cc: Jeremy Cline 
> Cc: Simon Ser 
> Signed-off-by: Lyude Paul 

Code LGTM:

Reviewed-by: Simon Ser 



Re: [PATCH 2/3] drm/nouveau/kms/nv50-: Report max cursor size to userspace

2021-01-19 Thread Simon Ser
On Tuesday, January 19th, 2021 at 2:54 AM, Lyude Paul  wrote:

> Cc: Martin Peres 
> Cc: Jeremy Cline 
> Cc: Simon Ser 
> Signed-off-by: Lyude Paul 
> ---
>  drivers/gpu/drm/nouveau/dispnv50/disp.c | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index c6367035970e..5f4f09a601d4 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -2663,6 +2663,14 @@ nv50_display_create(struct drm_device *dev)
>   else
>   nouveau_display(dev)->format_modifiers = disp50xx_modifiers;
>
> + if (disp->disp->object.oclass >= GK104_DISP) {

I can confirm this works fine on GK208B. Tested with wlroots. I don't
have older cards to test, though.

Tested-by: Simon Ser 

> + dev->mode_config.cursor_width = 256;
> + dev->mode_config.cursor_height = 256;
> + } else {
> + dev->mode_config.cursor_width = 64;
> + dev->mode_config.cursor_height = 64;
> + }
> +
>   /* create crtc objects to represent the hw heads */
>   if (disp->disp->object.oclass >= GV100_DISP)
>   crtcs = nvif_rd32(>object, 0x610060) & 0xff;
> --
> 2.29.2



Re: [PATCH 1/3] drivers/nouveau/kms/nv50-: Reject format modifiers for cursor planes

2021-01-19 Thread Simon Ser
On Tuesday, January 19th, 2021 at 2:54 AM, Lyude Paul  wrote:

> Nvidia hardware doesn't actually support using tiling formats with the
> cursor plane, only linear is allowed. In the future, we should write a
> testcase for this.
>
> Fixes: c586f30bf74c ("drm/nouveau/kms: Add format mod prop to 
> base/ovly/nvdisp")
> Cc: James Jones 
> Cc: Martin Peres 
> Cc: Jeremy Cline 
> Cc: Simon Ser 
> Cc:  # v5.8+
> Signed-off-by: Lyude Paul 

Together with [1], this patch allows me to run unpatched modifier-aware
user-space successfully, without a cursor visual glitch. drm_info
correctly reports the new modifier list, and wlroots logs confirm that
a flavor of NVIDIA_BLOCK_LINEAR_2D is used for the primary buffers and
LINEAR is used for cursor buffers.

Code looks good to me as well.

Reviewed-by: Simon Ser 

[1]: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3724


Re: [PATCH v6 15/16] drm: drm_crc: fix a kernel-doc markup

2021-01-14 Thread Simon Ser
On Thursday, January 14th, 2021 at 9:06 AM, Simon Ser  
wrote:

> On Thursday, January 14th, 2021 at 9:04 AM, Mauro Carvalho Chehab 
>  wrote:
>
> > A function has a different name between their prototype
> > and its kernel-doc markup:
> >
> > ../include/drm/drm_crtc.h:1257: warning: expecting prototype for 
> > drm_crtc_alloc_with_planes(). Prototype was for 
> > drmm_crtc_alloc_with_planes() instead
> >
> > Signed-off-by: Mauro Carvalho Chehab 
>
> Acked-by: Simon Ser 

Pushed to drm-misc-next, thanks for the fix!


Re: [PATCH v6 15/16] drm: drm_crc: fix a kernel-doc markup

2021-01-14 Thread Simon Ser
On Thursday, January 14th, 2021 at 9:04 AM, Mauro Carvalho Chehab 
 wrote:

> A function has a different name between their prototype
> and its kernel-doc markup:
>
>   ../include/drm/drm_crtc.h:1257: warning: expecting prototype for 
> drm_crtc_alloc_with_planes(). Prototype was for drmm_crtc_alloc_with_planes() 
> instead
>
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Simon Ser 


Re: [PATCH v8 4/4] NOTFORMERGE: drm/logicvc: Add plane colorkey support

2020-12-23 Thread Simon Ser
nouveau already has something for colorkey:
https://drmdb.emersion.fr/properties/4008636142/colorkey

I know this is marked "not for merge", but it would be nice to discuss
with them and come up with a standardized property.


Re: [PATCH][next] drm/atomic: avoid null pointer dereference on pointer crtc

2020-11-16 Thread Simon Ser
On Monday, November 16, 2020 12:03 PM, Colin King  
wrote:

> From: Colin Ian King colin.k...@canonical.com
>
> Since moving to the new debug helper functions we now have a debug message
> that dereferences crtc to print a kernel debug message when crtc is null
> and so this debug message will now cause a null pointer dereference. Since
> this is a debug message it probably is just simplest to fix this by just
> removing the debug message altogether.

NACK. This removes the log altogether instead of fixing it.

A fix has already been pushed to drm-misc-next: 0003b687ee6d ("drm: fix
oops in drm_atomic_set_crtc_for_connector").


Re: [Intel-gfx] drm_modes: signed integer overflow

2020-10-23 Thread Simon Ser
On Friday, October 23, 2020 5:27 PM, Ville Syrjälä 
 wrote:

> These are two extreme cases:

Thanks!

> > I'm trying to get
> > a fix for my user-space 1, and I'm wondering if int32_t is enough
> > after dividing by mode->htotal.
> > CC Pekka, just FYI (I think Weston has similar code).
>
> What's with those 100LL constants? Are you storing
> clock in Hz units?

We're storing the vertical refresh rate in mHz (milli-Hertz).


Re: [Intel-gfx] drm_modes: signed integer overflow

2020-10-23 Thread Simon Ser
On Thursday, October 22, 2020 12:14 PM, Ville Syrjälä 
 wrote:

> On Wed, Oct 21, 2020 at 08:13:43PM -0700, Randy Dunlap wrote:
>
> > Hi,
> > With linux-next 20201021, when booting up, I am seeing this:
> > [ 0.560896] UBSAN: signed-integer-overflow in 
> > ../drivers/gpu/drm/drm_modes.c:765:20
> > [ 0.560903] 2376000 * 1000 cannot be represented in type 'int'
>
> Dang. Didn't realize these new crazy >8k modes have dotclocks reaching
> almost 6 GHz, which would overflow even u32. I guess we'll switch to
> 64bit maths. Now I wonder how many other places can hit this overflow
> in practice...

Can you provide an example of a full crazy >8k mode? I'm trying to get
a fix for my user-space [1], and I'm wondering if int32_t is enough
after dividing by mode->htotal.

CC Pekka, just FYI (I think Weston has similar code).

[1]: https://github.com/swaywm/wlroots/pull/2450


Re: [PATCH] drm/vkms: add support for gamma_set interface

2020-09-02 Thread Simon Ser
On Tuesday, September 1, 2020 3:26 PM, Daniel Vetter  wrote:

> On Tue, Sep 01, 2020 at 08:57:37AM +0000, Simon Ser wrote:
>
> > On Monday, August 31, 2020 3:48 PM, Ville Syrjälä 
> > ville.syrj...@linux.intel.com wrote:
> >
> > > > > It doesn't seem like this IGT test's goal is to exercise support for
> > > > > gamma LUTs. Does the test just tries to reset the gamma LUT to linear?
> > > > > If so, I think the IGT test should be fixed to ignore "I don't support
> > > > > gamma" errors.
> > > >
> > > > It seems like that IGT test pixel-format is to make gamma lut like 
> > > > below.
> > > > for (i = 0; i < lut_size; i++)
> > > > lut[i] = (i * 0x / (lut_size - 1)) & mask;
> > > > And set this table to drm driver. and test begins. It's the test about 
> > > > pixel
> > > > format. I think you're right. It's not about gamma lut.
> > >
> > > The point of the gamma LUT stuff in the pixel format test is to throw
> > > away a bunch of the lsbs so that the test passes when the result is
> > > "close enough" to the 8bpc RGB reference image. Without it we would
> > > never get a crc match when testing non-8bpc or YCbCr formats.
> >
> > OK, that makes sense. Would it be sensible to:
> >
> > -   Don't set gamma if the pixel format being tested is 8bpc
>
> Hm not sure what 8bpc format you mean here, because we have C8 (needs
> gamma table or doesn't work) and the 8b greyscale one with the R8 one. If
> you ask for legacy 8bpc you get C8.

Why do we need a gamma LUT for C8 and R8? There shouldn't be any
precision loss, right?

> > -   Make the test skip if the pixel format is >8bpc and gamma isn't
> > supported
> >
>
> Yeah the test should skip if gamma isn't there.
> -Daniel
>
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
>
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch




Re: [PATCH] drm/vkms: add support for gamma_set interface

2020-09-01 Thread Simon Ser
On Monday, August 31, 2020 3:48 PM, Ville Syrjälä 
 wrote:

> > > It doesn't seem like this IGT test's goal is to exercise support for
> > > gamma LUTs. Does the test just tries to reset the gamma LUT to linear?
> > > If so, I think the IGT test should be fixed to ignore "I don't support
> > > gamma" errors.
> >
> > It seems like that IGT test pixel-format is to make gamma lut like below.
> > for (i = 0; i < lut_size; i++)
> > lut[i] = (i * 0x / (lut_size - 1)) & mask;
> > And set this table to drm driver. and test begins. It's the test about pixel
> > format. I think you're right. It's not about gamma lut.
>
> The point of the gamma LUT stuff in the pixel format test is to throw
> away a bunch of the lsbs so that the test passes when the result is
> "close enough" to the 8bpc RGB reference image. Without it we would
> never get a crc match when testing non-8bpc or YCbCr formats.

OK, that makes sense. Would it be sensible to:

- Don't set gamma if the pixel format being tested is 8bpc
- Make the test skip if the pixel format is >8bpc and gamma isn't
  supported



Re: [PATCH] drm/vkms: add support for gamma_set interface

2020-08-31 Thread Simon Ser
On Saturday, August 29, 2020 4:06 PM, Sidong Yang  wrote:

> Currently vkms module doesn't support gamma function for userspace. so igt
> subtests in kms_plane(pixel-format-pipe-A-plan) failed for calling
> drmModeCrtcSetGamma().

It doesn't seem like this IGT test's goal is to exercise support for
gamma LUTs. Does the test just tries to reset the gamma LUT to linear?
If so, I think the IGT test should be fixed to ignore "I don't support
gamma" errors.

> This patch set gamma_set interface in vkms_crtc_funcs for
> support gamma function. With initializing crtc, added calls for setting gamma
> size. it pass the test after this patch.
>
> Cc: Daniel Vetter
> Cc: Rodrigo Siqueira 
> Cc: Haneen Mohammed 
>
> Signed-off-by: Sidong Yang 
> ---
>  drivers/gpu/drm/vkms/vkms_crtc.c | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c 
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index ac85e17428f8..643435fb2ee6 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -160,6 +160,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
>   .get_crc_sources= vkms_get_crc_sources,
>   .set_crc_source = vkms_set_crc_source,
>   .verify_crc_source  = vkms_verify_crc_source,
> + .gamma_set  = drm_atomic_helper_legacy_gamma_set,

Why does VKMS need to use a legacy helper?

It seems like this patch just advertises support for gamma LUTs, but
ignores any value set by user-space. If VKMS advertises support for
gamma LUTs, it needs to take the LUT into account when blending planes.

>  };
>
>  static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
> @@ -275,6 +276,13 @@ int vkms_crtc_init(struct drm_device *dev, struct 
> drm_crtc *crtc,
>   return ret;
>   }
>
> + ret = drm_mode_crtc_set_gamma_size(crtc, 256);
> + if (ret) {
> + DRM_ERROR("Failed to set gamma size\n");
> + return ret;
> + }
> + drm_crtc_enable_color_mgmt(crtc, 0, false, 256);
> +
>   drm_crtc_helper_add(crtc, _crtc_helper_funcs);
>
>   spin_lock_init(_out->lock);
> --
> 2.17.1



Re: [PATCH v9 1/6] drm/fourcc: Add modifier definitions for describing Amlogic Video Framebuffer Compression

2020-07-03 Thread Simon Ser
Thanks for the update!

The driver should also disallow importing a AMLOGIC_FBC_LAYOUT_SCATTER
DMA-BUF from another device, but I guess this is clear enough ("not
transferrable between Amlogic SoCs").

>From a user-space PoV:

Acked-by: Simon Ser 


Re: [PATCH v8 1/6] drm/fourcc: Add modifier definitions for describing Amlogic Video Framebuffer Compression

2020-07-02 Thread Simon Ser
On Thursday, July 2, 2020 9:47 AM, Neil Armstrong  
wrote:

> Finally is also adds the Scatter Memory layout, meaning the header contains 
> IOMMU
> references to the compressed frames content to optimize memory access
> and layout.
>
> In this mode, only the header memory address is needed, thus the content
> memory organization is tied to the current producer execution and cannot
> be saved/dumped neither transferrable between Amlogic SoCs supporting this
> modifier.

Still not sure how to handle this one, since this breaks fundamental
assumptions about modifiers.


[tip:core/urgent] objtool: Use '.strtab' if '.shstrtab' doesn't exist, to support ORC tables on Clang

2018-07-14 Thread tip-bot for Simon Ser
Commit-ID:  6d77d3b43ad84a48b502f02dc618e7c36737bdfe
Gitweb: https://git.kernel.org/tip/6d77d3b43ad84a48b502f02dc618e7c36737bdfe
Author: Simon Ser 
AuthorDate: Mon, 9 Jul 2018 11:17:22 -0500
Committer:  Ingo Molnar 
CommitDate: Sat, 14 Jul 2018 14:59:42 +0200

objtool: Use '.strtab' if '.shstrtab' doesn't exist, to support ORC tables on 
Clang

Clang puts its section header names in the '.strtab' section instead of
'.shstrtab', which causes objtool to fail with a "can't find
.shstrtab section" warning when attempting to write ORC metadata to an
object file.

If '.shstrtab' doesn't exist, use '.strtab' instead.

Signed-off-by: Simon Ser 
Signed-off-by: Josh Poimboeuf 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Link: 
http://lkml.kernel.org/r/d1c1c3fe55872be433da7bc5e1860538506229ba.1531153015.git.jpoim...@redhat.com
Signed-off-by: Ingo Molnar 
---
 tools/objtool/elf.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 0d1acb704f64..7ec85d567598 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -519,10 +519,12 @@ struct section *elf_create_section(struct elf *elf, const 
char *name,
sec->sh.sh_flags = SHF_ALLOC;
 
 
-   /* Add section name to .shstrtab */
+   /* Add section name to .shstrtab (or .strtab for Clang) */
shstrtab = find_section_by_name(elf, ".shstrtab");
+   if (!shstrtab)
+   shstrtab = find_section_by_name(elf, ".strtab");
if (!shstrtab) {
-   WARN("can't find .shstrtab section");
+   WARN("can't find .shstrtab or .strtab section");
return NULL;
}
 


[tip:core/urgent] objtool: Use '.strtab' if '.shstrtab' doesn't exist, to support ORC tables on Clang

2018-07-14 Thread tip-bot for Simon Ser
Commit-ID:  6d77d3b43ad84a48b502f02dc618e7c36737bdfe
Gitweb: https://git.kernel.org/tip/6d77d3b43ad84a48b502f02dc618e7c36737bdfe
Author: Simon Ser 
AuthorDate: Mon, 9 Jul 2018 11:17:22 -0500
Committer:  Ingo Molnar 
CommitDate: Sat, 14 Jul 2018 14:59:42 +0200

objtool: Use '.strtab' if '.shstrtab' doesn't exist, to support ORC tables on 
Clang

Clang puts its section header names in the '.strtab' section instead of
'.shstrtab', which causes objtool to fail with a "can't find
.shstrtab section" warning when attempting to write ORC metadata to an
object file.

If '.shstrtab' doesn't exist, use '.strtab' instead.

Signed-off-by: Simon Ser 
Signed-off-by: Josh Poimboeuf 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Link: 
http://lkml.kernel.org/r/d1c1c3fe55872be433da7bc5e1860538506229ba.1531153015.git.jpoim...@redhat.com
Signed-off-by: Ingo Molnar 
---
 tools/objtool/elf.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 0d1acb704f64..7ec85d567598 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -519,10 +519,12 @@ struct section *elf_create_section(struct elf *elf, const 
char *name,
sec->sh.sh_flags = SHF_ALLOC;
 
 
-   /* Add section name to .shstrtab */
+   /* Add section name to .shstrtab (or .strtab for Clang) */
shstrtab = find_section_by_name(elf, ".shstrtab");
+   if (!shstrtab)
+   shstrtab = find_section_by_name(elf, ".strtab");
if (!shstrtab) {
-   WARN("can't find .shstrtab section");
+   WARN("can't find .shstrtab or .strtab section");
return NULL;
}
 


[PATCH] objtool: use .strtab if .shstrtab is not present

2018-06-29 Thread Simon Ser
Executables that are generated by Clang don't have a .shstrtab
section, and store section names in .strtab instead. We can store
section names generated by orc there in this case.

Signed-off-by: Simon Ser 
---
 tools/objtool/elf.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 4e60e105583..6b9705d8b55 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -507,7 +507,10 @@ struct section *elf_create_section(struct elf *elf, const 
char *name,
/* Add section name to .shstrtab */
shstrtab = find_section_by_name(elf, ".shstrtab");
if (!shstrtab) {
-   WARN("can't find .shstrtab section");
+   shstrtab = find_section_by_name(elf, ".strtab");
+   }
+   if (!shstrtab) {
+   WARN("can't find .shstrtab or .strtab section");
return NULL;
}
 
-- 
2.18.0




[PATCH] objtool: use .strtab if .shstrtab is not present

2018-06-29 Thread Simon Ser
Executables that are generated by Clang don't have a .shstrtab
section, and store section names in .strtab instead. We can store
section names generated by orc there in this case.

Signed-off-by: Simon Ser 
---
 tools/objtool/elf.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 4e60e105583..6b9705d8b55 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -507,7 +507,10 @@ struct section *elf_create_section(struct elf *elf, const 
char *name,
/* Add section name to .shstrtab */
shstrtab = find_section_by_name(elf, ".shstrtab");
if (!shstrtab) {
-   WARN("can't find .shstrtab section");
+   shstrtab = find_section_by_name(elf, ".strtab");
+   }
+   if (!shstrtab) {
+   WARN("can't find .shstrtab or .strtab section");
return NULL;
}
 
-- 
2.18.0




[tip:core/urgent] objtool: Fix seg fault with clang-compiled objects

2017-12-30 Thread tip-bot for Simon Ser
Commit-ID:  ce90aaf5cde4ce057b297bb6c955caf16ef00ee6
Gitweb: https://git.kernel.org/tip/ce90aaf5cde4ce057b297bb6c955caf16ef00ee6
Author: Simon Ser <cont...@emersion.fr>
AuthorDate: Sat, 30 Dec 2017 14:43:32 -0600
Committer:  Ingo Molnar <mi...@kernel.org>
CommitDate: Sat, 30 Dec 2017 22:04:17 +0100

objtool: Fix seg fault with clang-compiled objects

Fix a seg fault which happens when an input file provided to 'objtool
orc generate' doesn't have a '.shstrtab' section (for instance, object
files produced by clang don't have this section).

Signed-off-by: Simon Ser <cont...@emersion.fr>
Signed-off-by: Josh Poimboeuf <jpoim...@redhat.com>
Cc: Linus Torvalds <torva...@linux-foundation.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Link: 
http://lkml.kernel.org/r/c0f2231683e9bed40fac1f13ce2c33b8389854bc.1514666459.git.jpoim...@redhat.com
Signed-off-by: Ingo Molnar <mi...@kernel.org>
---
 tools/objtool/orc_gen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index e5ca314..e61fe70 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -165,6 +165,8 @@ int create_orc_sections(struct objtool_file *file)
 
/* create .orc_unwind_ip and .rela.orc_unwind_ip sections */
sec = elf_create_section(file->elf, ".orc_unwind_ip", sizeof(int), idx);
+   if (!sec)
+   return -1;
 
ip_relasec = elf_create_rela_section(file->elf, sec);
if (!ip_relasec)


[tip:core/urgent] objtool: Fix seg fault with clang-compiled objects

2017-12-30 Thread tip-bot for Simon Ser
Commit-ID:  ce90aaf5cde4ce057b297bb6c955caf16ef00ee6
Gitweb: https://git.kernel.org/tip/ce90aaf5cde4ce057b297bb6c955caf16ef00ee6
Author: Simon Ser 
AuthorDate: Sat, 30 Dec 2017 14:43:32 -0600
Committer:  Ingo Molnar 
CommitDate: Sat, 30 Dec 2017 22:04:17 +0100

objtool: Fix seg fault with clang-compiled objects

Fix a seg fault which happens when an input file provided to 'objtool
orc generate' doesn't have a '.shstrtab' section (for instance, object
files produced by clang don't have this section).

Signed-off-by: Simon Ser 
Signed-off-by: Josh Poimboeuf 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Link: 
http://lkml.kernel.org/r/c0f2231683e9bed40fac1f13ce2c33b8389854bc.1514666459.git.jpoim...@redhat.com
Signed-off-by: Ingo Molnar 
---
 tools/objtool/orc_gen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index e5ca314..e61fe70 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -165,6 +165,8 @@ int create_orc_sections(struct objtool_file *file)
 
/* create .orc_unwind_ip and .rela.orc_unwind_ip sections */
sec = elf_create_section(file->elf, ".orc_unwind_ip", sizeof(int), idx);
+   if (!sec)
+   return -1;
 
ip_relasec = elf_create_rela_section(file->elf, sec);
if (!ip_relasec)


[tip:core/urgent] objtool: Fix seg fault caused by missing parameter

2017-12-30 Thread tip-bot for Simon Ser
Commit-ID:  d89e426499cf36b96161bd32970d6783f1fbcb0e
Gitweb: https://git.kernel.org/tip/d89e426499cf36b96161bd32970d6783f1fbcb0e
Author: Simon Ser <cont...@emersion.fr>
AuthorDate: Sat, 30 Dec 2017 14:43:31 -0600
Committer:  Ingo Molnar <mi...@kernel.org>
CommitDate: Sat, 30 Dec 2017 22:04:17 +0100

objtool: Fix seg fault caused by missing parameter

Fix a seg fault when no parameter is provided to 'objtool orc'.

Signed-off-by: Simon Ser <cont...@emersion.fr>
Signed-off-by: Josh Poimboeuf <jpoim...@redhat.com>
Cc: Linus Torvalds <torva...@linux-foundation.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Link: 
http://lkml.kernel.org/r/9172803ec7ebb72535bcd0b7f966ae96d515968e.1514666459.git.jpoim...@redhat.com
Signed-off-by: Ingo Molnar <mi...@kernel.org>
---
 tools/objtool/builtin-orc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/builtin-orc.c b/tools/objtool/builtin-orc.c
index 4c6b5c9..91e8e19 100644
--- a/tools/objtool/builtin-orc.c
+++ b/tools/objtool/builtin-orc.c
@@ -44,6 +44,9 @@ int cmd_orc(int argc, const char **argv)
const char *objname;
 
argc--; argv++;
+   if (argc <= 0)
+   usage_with_options(orc_usage, check_options);
+
if (!strncmp(argv[0], "gen", 3)) {
argc = parse_options(argc, argv, check_options, orc_usage, 0);
if (argc != 1)
@@ -52,7 +55,6 @@ int cmd_orc(int argc, const char **argv)
objname = argv[0];
 
return check(objname, no_fp, no_unreachable, true);
-
}
 
if (!strcmp(argv[0], "dump")) {


[tip:core/urgent] objtool: Fix seg fault caused by missing parameter

2017-12-30 Thread tip-bot for Simon Ser
Commit-ID:  d89e426499cf36b96161bd32970d6783f1fbcb0e
Gitweb: https://git.kernel.org/tip/d89e426499cf36b96161bd32970d6783f1fbcb0e
Author: Simon Ser 
AuthorDate: Sat, 30 Dec 2017 14:43:31 -0600
Committer:  Ingo Molnar 
CommitDate: Sat, 30 Dec 2017 22:04:17 +0100

objtool: Fix seg fault caused by missing parameter

Fix a seg fault when no parameter is provided to 'objtool orc'.

Signed-off-by: Simon Ser 
Signed-off-by: Josh Poimboeuf 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Link: 
http://lkml.kernel.org/r/9172803ec7ebb72535bcd0b7f966ae96d515968e.1514666459.git.jpoim...@redhat.com
Signed-off-by: Ingo Molnar 
---
 tools/objtool/builtin-orc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/builtin-orc.c b/tools/objtool/builtin-orc.c
index 4c6b5c9..91e8e19 100644
--- a/tools/objtool/builtin-orc.c
+++ b/tools/objtool/builtin-orc.c
@@ -44,6 +44,9 @@ int cmd_orc(int argc, const char **argv)
const char *objname;
 
argc--; argv++;
+   if (argc <= 0)
+   usage_with_options(orc_usage, check_options);
+
if (!strncmp(argv[0], "gen", 3)) {
argc = parse_options(argc, argv, check_options, orc_usage, 0);
if (argc != 1)
@@ -52,7 +55,6 @@ int cmd_orc(int argc, const char **argv)
objname = argv[0];
 
return check(objname, no_fp, no_unreachable, true);
-
}
 
if (!strcmp(argv[0], "dump")) {