Nouveau - Dec 2018 - [PATCH v3 1/3] drm/connector: Add generic underscan properties

On Thu, Nov 22, 2018 at 12:23:29PM +0100, Boris Brezillon
wrote:
> @@ -924,6 +978,29 @@ struct drm_connector {
>  	 */
>  	struct drm_property_blob *path_blob_ptr;
>  
> +	/**
> +	 * @underscan_mode_property: Optional connector underscan mode. Used by
> +	 * the driver to scale the output image and compensate an overscan done
> +	 * on the display side.
> +	 */
> +	struct drm_property *underscan_mode_property;
> +
> +	/**
> +	 * @underscan_hborder_property: Optional connector underscan horizontal
> +	 * border (expressed in pixels). Used by the driver to adjust the
> +	 * output image position and compensate an overscan done on the display
> +	 * side.
> +	 */
> +	struct drm_property *underscan_hborder_property;
> +
> +	/**
> +	 * @underscan_hborder_property: Optional connector underscan vertical
> +	 * border (expressed in pixels). Used by the driver to adjust the
> +	 * output image position and compensate an overscan done on the display
> +	 * side.
> +	 */
> +	struct drm_property *underscan_vborder_property;
I'm wondering why we're adding these new props when we already have the
(slightly more flexible) margin properties for TV out. We could just
reuse those AFAICS.
> +
>  #define DRM_CONNECTOR_POLL_HPD (1 << 0)
>  #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
>  #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
> @@ -1180,6 +1257,9 @@ int drm_mode_create_dvi_i_properties(struct
drm_device *dev);
>  int drm_mode_create_tv_properties(struct drm_device *dev,
>  				  unsigned int num_modes,
>  				  const char * const modes[]);
> +int drm_connector_attach_underscan_properties(struct drm_connector
*connector,
> +					      u32 mode_mask, u64 max_hborder,
> +					      u64 max_vborder);
>  int drm_mode_create_scaling_mode_property(struct drm_device *dev);
>  int drm_connector_attach_content_type_property(struct drm_connector *dev);
>  int drm_connector_attach_scaling_mode_property(struct drm_connector
*connector,
> -- 
> 2.17.1
-- 
Ville Syrjälä
Intel

On Mon, 3 Dec 2018 16:40:11 +0200
Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> On Thu, Nov 22, 2018 at 12:23:29PM +0100, Boris Brezillon wrote:
> > @@ -924,6 +978,29 @@ struct drm_connector {
> >  	 */
> >  	struct drm_property_blob *path_blob_ptr;
> >  
> > +	/**
> > +	 * @underscan_mode_property: Optional connector underscan mode. Used
by
> > +	 * the driver to scale the output image and compensate an overscan
done
> > +	 * on the display side.
> > +	 */
> > +	struct drm_property *underscan_mode_property;
> > +
> > +	/**
> > +	 * @underscan_hborder_property: Optional connector underscan
horizontal
> > +	 * border (expressed in pixels). Used by the driver to adjust the
> > +	 * output image position and compensate an overscan done on the
display
> > +	 * side.
> > +	 */
> > +	struct drm_property *underscan_hborder_property;
> > +
> > +	/**
> > +	 * @underscan_hborder_property: Optional connector underscan
vertical
> > +	 * border (expressed in pixels). Used by the driver to adjust the
> > +	 * output image position and compensate an overscan done on the
display
> > +	 * side.
> > +	 */
> > +	struct drm_property *underscan_vborder_property;  
> 
> I'm wondering why we're adding these new props when we already have
the
> (slightly more flexible) margin properties for TV out. We could just
> reuse those AFAICS.
I'm not against the idea, but I can't use
drm_mode_create_tv_properties() directly, as most props created by this
function are not applicable to an HDMI displays. Should I move the
margins props out of the tv_connector_state and provide new helpers to
create those props?
> 
> > +
> >  #define DRM_CONNECTOR_POLL_HPD (1 << 0)
> >  #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
> >  #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
> > @@ -1180,6 +1257,9 @@ int drm_mode_create_dvi_i_properties(struct
drm_device *dev);
> >  int drm_mode_create_tv_properties(struct drm_device *dev,
> >  				  unsigned int num_modes,
> >  				  const char * const modes[]);
> > +int drm_connector_attach_underscan_properties(struct drm_connector
*connector,
> > +					      u32 mode_mask, u64 max_hborder,
> > +					      u64 max_vborder);
> >  int drm_mode_create_scaling_mode_property(struct drm_device *dev);
> >  int drm_connector_attach_content_type_property(struct drm_connector
*dev);
> >  int drm_connector_attach_scaling_mode_property(struct drm_connector
*connector,
> > -- 
> > 2.17.1  
>

Boris Brezillon <boris.brezillon at bootlin.com> writes:
> On Mon, 3 Dec 2018 16:40:11 +0200
> Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
>
>> On Thu, Nov 22, 2018 at 12:23:29PM +0100, Boris Brezillon wrote:
>> > @@ -924,6 +978,29 @@ struct drm_connector {
>> >  	 */
>> >  	struct drm_property_blob *path_blob_ptr;
>> >  
>> > +	/**
>> > +	 * @underscan_mode_property: Optional connector underscan mode.
Used by
>> > +	 * the driver to scale the output image and compensate an
overscan done
>> > +	 * on the display side.
>> > +	 */
>> > +	struct drm_property *underscan_mode_property;
>> > +
>> > +	/**
>> > +	 * @underscan_hborder_property: Optional connector underscan
horizontal
>> > +	 * border (expressed in pixels). Used by the driver to adjust
the
>> > +	 * output image position and compensate an overscan done on the
display
>> > +	 * side.
>> > +	 */
>> > +	struct drm_property *underscan_hborder_property;
>> > +
>> > +	/**
>> > +	 * @underscan_hborder_property: Optional connector underscan
vertical
>> > +	 * border (expressed in pixels). Used by the driver to adjust
the
>> > +	 * output image position and compensate an overscan done on the
display
>> > +	 * side.
>> > +	 */
>> > +	struct drm_property *underscan_vborder_property;  
>> 
>> I'm wondering why we're adding these new props when we already
have the
>> (slightly more flexible) margin properties for TV out. We could just
>> reuse those AFAICS.
>
> I'm not against the idea, but I can't use
> drm_mode_create_tv_properties() directly, as most props created by this
> function are not applicable to an HDMI displays. Should I move the
> margins props out of the tv_connector_state and provide new helpers to
> create those props?
TV margin props look good to me, FWIW.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20181203/b2dfd45b/attachment.sig>