Re: [RFC 1/5] drm: Add HDMI 2.0+ features exposing knob

2017-03-28 Thread Daniel Vetter
On Mon, Mar 27, 2017 at 02:47:08PM +0100, Jose Abreu wrote:
> Hi Daniel,
> 
> 
> On 23-03-2017 11:00, Daniel Vetter wrote:
> > On Thu, Mar 23, 2017 at 10:44:16AM +, Jose Abreu wrote:
> >> Hi Daniel,
> >>
> >>
> >> On 23-03-2017 07:22, Daniel Vetter wrote:
> >>> On Wed, Mar 22, 2017 at 05:35:57PM +, Jose Abreu wrote:
>  We can't expect userspace to have full support for all HDMI 2.0+
>  features. Instead of expecting/waiting for userspace to support
>  the new features add a knob, much like the stereo knob, so that
>  DRM core will only expose the features when asked too.
> 
>  Signed-off-by: Jose Abreu 
>  Cc: Carlos Palminha 
>  Cc: dri-devel@lists.freedesktop.org
>  ---
>   drivers/gpu/drm/drm_ioctl.c | 5 +
>   include/drm/drm_file.h  | 8 
>   include/uapi/drm/drm.h  | 7 +++
>   3 files changed, 20 insertions(+)
> 
>  diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>  index a7c61c2..2430e2e 100644
>  --- a/drivers/gpu/drm/drm_ioctl.c
>  +++ b/drivers/gpu/drm/drm_ioctl.c
>  @@ -318,6 +318,11 @@ static int drm_getcap(struct drm_device *dev, void 
>  *data, struct drm_file *file_
>   file_priv->atomic = req->value;
>   file_priv->universal_planes = req->value;
>   break;
>  +case DRM_CLIENT_CAP_HDMI2:
>  +if (req->value > 1)
>  +return -EINVAL;
>  +file_priv->hdmi2_allowed = req->value;
> >>> Needs some userspace to make use of this (like the -modesetting x.org
> >>> driver).
> >>> -Daniel
> >> I see your point but shouldn't this be merged before actually
> >> starting to work in userspace?
> > No, see
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__dri.freedesktop.org_docs_drm_gpu_drm-2Duapi.html-23open-2Dsource-2Duserspace-2Drequirements=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw=tcc2TQFpF7ats3OIzYgmFrHaeymBrPtqHphO4Ui4AUY=qWySmgpGaOwxC8Jl3A5qZVf4aS2qz3LXshVdv3FehJc=
> >  
> >
> > for reasons behind this and the precise flow.
> >
> > Cheers, Daniel
> 
> Thanks, I will do that. I can leave the flag and just limit the
> export of aspect ratios for now, we can limit more stuff later if
> needed. What do you think?
> 
> Another thing: Is it safe to enable this in the modesetting
> driver, shouldn't the modesetting clients also know about the
> flag or does modesetting handles this correctly? (I'm not very
> familiar with this driver, sorry).

Probably needs an xrandr revision too.
-Daniel

> 
> Best regards,
> Jose Miguel Abreu
> 
> >
> >> Best regards,
> >> Jose Miguel Abreu
> >>
>  +break;
>   default:
>   return -EINVAL;
>   }
>  diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
>  index 5dd27ae..7b97d85 100644
>  --- a/include/drm/drm_file.h
>  +++ b/include/drm/drm_file.h
>  @@ -192,6 +192,14 @@ struct drm_file {
>   unsigned is_master:1;
>   
>   /**
>  + * @hdmi2_allowed:
>  + *
>  + * True if client understands HDMI 2.0+ features like, for 
>  example,
>  + * extended aspect ratios
>  + */
>  +unsigned hdmi2_allowed:1;
>  +
>  +/**
>    * @master:
>    *
>    * Master this node is currently associated with. Only relevant 
>  if
>  diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>  index b2c5284..9e25138 100644
>  --- a/include/uapi/drm/drm.h
>  +++ b/include/uapi/drm/drm.h
>  @@ -678,6 +678,13 @@ struct drm_get_cap {
>    */
>   #define DRM_CLIENT_CAP_ATOMIC   3
>   
>  +/**
>  + * DRM_CLIENT_CAP_HDMI2
>  + *
>  + * If set to 1, the DRM core will expose HDMI 2.0+ features to userspace
>  + */
>  +#define DRM_CLIENT_CAP_HDMI24
>  +
>   /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
>   struct drm_set_client_cap {
>   __u64 capability;
>  -- 
>  1.9.1
> 
> 
>  ___
>  dri-devel mailing list
>  dri-devel@lists.freedesktop.org
>  https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw=4lBpQovdR7O8jeC7mV8SX9xvTypMtV8CBDEDg3bGgzw=h-gqFlO62i53pir0j2lJ-upE_bXQxhn3BK1nhdxoB6Y=
>   
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC 1/5] drm: Add HDMI 2.0+ features exposing knob

2017-03-27 Thread Jose Abreu
Hi Daniel,


On 23-03-2017 11:00, Daniel Vetter wrote:
> On Thu, Mar 23, 2017 at 10:44:16AM +, Jose Abreu wrote:
>> Hi Daniel,
>>
>>
>> On 23-03-2017 07:22, Daniel Vetter wrote:
>>> On Wed, Mar 22, 2017 at 05:35:57PM +, Jose Abreu wrote:
 We can't expect userspace to have full support for all HDMI 2.0+
 features. Instead of expecting/waiting for userspace to support
 the new features add a knob, much like the stereo knob, so that
 DRM core will only expose the features when asked too.

 Signed-off-by: Jose Abreu 
 Cc: Carlos Palminha 
 Cc: dri-devel@lists.freedesktop.org
 ---
  drivers/gpu/drm/drm_ioctl.c | 5 +
  include/drm/drm_file.h  | 8 
  include/uapi/drm/drm.h  | 7 +++
  3 files changed, 20 insertions(+)

 diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
 index a7c61c2..2430e2e 100644
 --- a/drivers/gpu/drm/drm_ioctl.c
 +++ b/drivers/gpu/drm/drm_ioctl.c
 @@ -318,6 +318,11 @@ static int drm_getcap(struct drm_device *dev, void 
 *data, struct drm_file *file_
file_priv->atomic = req->value;
file_priv->universal_planes = req->value;
break;
 +  case DRM_CLIENT_CAP_HDMI2:
 +  if (req->value > 1)
 +  return -EINVAL;
 +  file_priv->hdmi2_allowed = req->value;
>>> Needs some userspace to make use of this (like the -modesetting x.org
>>> driver).
>>> -Daniel
>> I see your point but shouldn't this be merged before actually
>> starting to work in userspace?
> No, see
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__dri.freedesktop.org_docs_drm_gpu_drm-2Duapi.html-23open-2Dsource-2Duserspace-2Drequirements=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw=tcc2TQFpF7ats3OIzYgmFrHaeymBrPtqHphO4Ui4AUY=qWySmgpGaOwxC8Jl3A5qZVf4aS2qz3LXshVdv3FehJc=
>  
>
> for reasons behind this and the precise flow.
>
> Cheers, Daniel

Thanks, I will do that. I can leave the flag and just limit the
export of aspect ratios for now, we can limit more stuff later if
needed. What do you think?

Another thing: Is it safe to enable this in the modesetting
driver, shouldn't the modesetting clients also know about the
flag or does modesetting handles this correctly? (I'm not very
familiar with this driver, sorry).

Best regards,
Jose Miguel Abreu

>
>> Best regards,
>> Jose Miguel Abreu
>>
 +  break;
default:
return -EINVAL;
}
 diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
 index 5dd27ae..7b97d85 100644
 --- a/include/drm/drm_file.h
 +++ b/include/drm/drm_file.h
 @@ -192,6 +192,14 @@ struct drm_file {
unsigned is_master:1;
  
/**
 +   * @hdmi2_allowed:
 +   *
 +   * True if client understands HDMI 2.0+ features like, for example,
 +   * extended aspect ratios
 +   */
 +  unsigned hdmi2_allowed:1;
 +
 +  /**
 * @master:
 *
 * Master this node is currently associated with. Only relevant if
 diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
 index b2c5284..9e25138 100644
 --- a/include/uapi/drm/drm.h
 +++ b/include/uapi/drm/drm.h
 @@ -678,6 +678,13 @@ struct drm_get_cap {
   */
  #define DRM_CLIENT_CAP_ATOMIC 3
  
 +/**
 + * DRM_CLIENT_CAP_HDMI2
 + *
 + * If set to 1, the DRM core will expose HDMI 2.0+ features to userspace
 + */
 +#define DRM_CLIENT_CAP_HDMI2  4
 +
  /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
  struct drm_set_client_cap {
__u64 capability;
 -- 
 1.9.1


 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw=4lBpQovdR7O8jeC7mV8SX9xvTypMtV8CBDEDg3bGgzw=h-gqFlO62i53pir0j2lJ-upE_bXQxhn3BK1nhdxoB6Y=
  

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


Re: [RFC 1/5] drm: Add HDMI 2.0+ features exposing knob

2017-03-23 Thread Jose Abreu
Hi Daniel,


On 23-03-2017 07:22, Daniel Vetter wrote:
> On Wed, Mar 22, 2017 at 05:35:57PM +, Jose Abreu wrote:
>> We can't expect userspace to have full support for all HDMI 2.0+
>> features. Instead of expecting/waiting for userspace to support
>> the new features add a knob, much like the stereo knob, so that
>> DRM core will only expose the features when asked too.
>>
>> Signed-off-by: Jose Abreu 
>> Cc: Carlos Palminha 
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>>  drivers/gpu/drm/drm_ioctl.c | 5 +
>>  include/drm/drm_file.h  | 8 
>>  include/uapi/drm/drm.h  | 7 +++
>>  3 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index a7c61c2..2430e2e 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -318,6 +318,11 @@ static int drm_getcap(struct drm_device *dev, void 
>> *data, struct drm_file *file_
>>  file_priv->atomic = req->value;
>>  file_priv->universal_planes = req->value;
>>  break;
>> +case DRM_CLIENT_CAP_HDMI2:
>> +if (req->value > 1)
>> +return -EINVAL;
>> +file_priv->hdmi2_allowed = req->value;
> Needs some userspace to make use of this (like the -modesetting x.org
> driver).
> -Daniel

I see your point but shouldn't this be merged before actually
starting to work in userspace?

Best regards,
Jose Miguel Abreu

>
>> +break;
>>  default:
>>  return -EINVAL;
>>  }
>> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
>> index 5dd27ae..7b97d85 100644
>> --- a/include/drm/drm_file.h
>> +++ b/include/drm/drm_file.h
>> @@ -192,6 +192,14 @@ struct drm_file {
>>  unsigned is_master:1;
>>  
>>  /**
>> + * @hdmi2_allowed:
>> + *
>> + * True if client understands HDMI 2.0+ features like, for example,
>> + * extended aspect ratios
>> + */
>> +unsigned hdmi2_allowed:1;
>> +
>> +/**
>>   * @master:
>>   *
>>   * Master this node is currently associated with. Only relevant if
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index b2c5284..9e25138 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -678,6 +678,13 @@ struct drm_get_cap {
>>   */
>>  #define DRM_CLIENT_CAP_ATOMIC   3
>>  
>> +/**
>> + * DRM_CLIENT_CAP_HDMI2
>> + *
>> + * If set to 1, the DRM core will expose HDMI 2.0+ features to userspace
>> + */
>> +#define DRM_CLIENT_CAP_HDMI24
>> +
>>  /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
>>  struct drm_set_client_cap {
>>  __u64 capability;
>> -- 
>> 1.9.1
>>
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw=4lBpQovdR7O8jeC7mV8SX9xvTypMtV8CBDEDg3bGgzw=h-gqFlO62i53pir0j2lJ-upE_bXQxhn3BK1nhdxoB6Y=
>>  

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


Re: [RFC 1/5] drm: Add HDMI 2.0+ features exposing knob

2017-03-23 Thread Daniel Vetter
On Thu, Mar 23, 2017 at 10:44:16AM +, Jose Abreu wrote:
> Hi Daniel,
> 
> 
> On 23-03-2017 07:22, Daniel Vetter wrote:
> > On Wed, Mar 22, 2017 at 05:35:57PM +, Jose Abreu wrote:
> >> We can't expect userspace to have full support for all HDMI 2.0+
> >> features. Instead of expecting/waiting for userspace to support
> >> the new features add a knob, much like the stereo knob, so that
> >> DRM core will only expose the features when asked too.
> >>
> >> Signed-off-by: Jose Abreu 
> >> Cc: Carlos Palminha 
> >> Cc: dri-devel@lists.freedesktop.org
> >> ---
> >>  drivers/gpu/drm/drm_ioctl.c | 5 +
> >>  include/drm/drm_file.h  | 8 
> >>  include/uapi/drm/drm.h  | 7 +++
> >>  3 files changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> >> index a7c61c2..2430e2e 100644
> >> --- a/drivers/gpu/drm/drm_ioctl.c
> >> +++ b/drivers/gpu/drm/drm_ioctl.c
> >> @@ -318,6 +318,11 @@ static int drm_getcap(struct drm_device *dev, void 
> >> *data, struct drm_file *file_
> >>file_priv->atomic = req->value;
> >>file_priv->universal_planes = req->value;
> >>break;
> >> +  case DRM_CLIENT_CAP_HDMI2:
> >> +  if (req->value > 1)
> >> +  return -EINVAL;
> >> +  file_priv->hdmi2_allowed = req->value;
> > Needs some userspace to make use of this (like the -modesetting x.org
> > driver).
> > -Daniel
> 
> I see your point but shouldn't this be merged before actually
> starting to work in userspace?

No, see

https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements

for reasons behind this and the precise flow.

Cheers, Daniel

> 
> Best regards,
> Jose Miguel Abreu
> 
> >
> >> +  break;
> >>default:
> >>return -EINVAL;
> >>}
> >> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> >> index 5dd27ae..7b97d85 100644
> >> --- a/include/drm/drm_file.h
> >> +++ b/include/drm/drm_file.h
> >> @@ -192,6 +192,14 @@ struct drm_file {
> >>unsigned is_master:1;
> >>  
> >>/**
> >> +   * @hdmi2_allowed:
> >> +   *
> >> +   * True if client understands HDMI 2.0+ features like, for example,
> >> +   * extended aspect ratios
> >> +   */
> >> +  unsigned hdmi2_allowed:1;
> >> +
> >> +  /**
> >> * @master:
> >> *
> >> * Master this node is currently associated with. Only relevant if
> >> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> >> index b2c5284..9e25138 100644
> >> --- a/include/uapi/drm/drm.h
> >> +++ b/include/uapi/drm/drm.h
> >> @@ -678,6 +678,13 @@ struct drm_get_cap {
> >>   */
> >>  #define DRM_CLIENT_CAP_ATOMIC 3
> >>  
> >> +/**
> >> + * DRM_CLIENT_CAP_HDMI2
> >> + *
> >> + * If set to 1, the DRM core will expose HDMI 2.0+ features to userspace
> >> + */
> >> +#define DRM_CLIENT_CAP_HDMI2  4
> >> +
> >>  /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
> >>  struct drm_set_client_cap {
> >>__u64 capability;
> >> -- 
> >> 1.9.1
> >>
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw=4lBpQovdR7O8jeC7mV8SX9xvTypMtV8CBDEDg3bGgzw=h-gqFlO62i53pir0j2lJ-upE_bXQxhn3BK1nhdxoB6Y=
> >>  
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC 1/5] drm: Add HDMI 2.0+ features exposing knob

2017-03-23 Thread Daniel Vetter
On Wed, Mar 22, 2017 at 05:35:57PM +, Jose Abreu wrote:
> We can't expect userspace to have full support for all HDMI 2.0+
> features. Instead of expecting/waiting for userspace to support
> the new features add a knob, much like the stereo knob, so that
> DRM core will only expose the features when asked too.
> 
> Signed-off-by: Jose Abreu 
> Cc: Carlos Palminha 
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_ioctl.c | 5 +
>  include/drm/drm_file.h  | 8 
>  include/uapi/drm/drm.h  | 7 +++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index a7c61c2..2430e2e 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -318,6 +318,11 @@ static int drm_getcap(struct drm_device *dev, void 
> *data, struct drm_file *file_
>   file_priv->atomic = req->value;
>   file_priv->universal_planes = req->value;
>   break;
> + case DRM_CLIENT_CAP_HDMI2:
> + if (req->value > 1)
> + return -EINVAL;
> + file_priv->hdmi2_allowed = req->value;

Needs some userspace to make use of this (like the -modesetting x.org
driver).
-Daniel

> + break;
>   default:
>   return -EINVAL;
>   }
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 5dd27ae..7b97d85 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -192,6 +192,14 @@ struct drm_file {
>   unsigned is_master:1;
>  
>   /**
> +  * @hdmi2_allowed:
> +  *
> +  * True if client understands HDMI 2.0+ features like, for example,
> +  * extended aspect ratios
> +  */
> + unsigned hdmi2_allowed:1;
> +
> + /**
>* @master:
>*
>* Master this node is currently associated with. Only relevant if
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index b2c5284..9e25138 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -678,6 +678,13 @@ struct drm_get_cap {
>   */
>  #define DRM_CLIENT_CAP_ATOMIC3
>  
> +/**
> + * DRM_CLIENT_CAP_HDMI2
> + *
> + * If set to 1, the DRM core will expose HDMI 2.0+ features to userspace
> + */
> +#define DRM_CLIENT_CAP_HDMI2 4
> +
>  /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
>  struct drm_set_client_cap {
>   __u64 capability;
> -- 
> 1.9.1
> 
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel