Re: [RFC PATCH v2 1/2] drm: Add generic colorkey properties
call
> * drm_atomic_normalize_zpos() to update this before it can be trusted.
> + * @colorkey.mode: color key mode
> + * @colorkey.min: color key range minimum. The value is stored in
> AXYZ16161616
> + * format, where A is the alpha value and X, Y and Z correspond to the
> + * color components of the plane's pixel format (usually RGB or YUV)
> + * @colorkey.max: color key range maximum (in AXYZ16161616 format)
> + * @colorkey.mask: color key mask value (in AXYZ16161616 format)
> + * @colorkey.format: color key min/max/mask values pixel format (in
> + * DRM_FORMAT_AXYZ16161616 form)
> + * @colorkey.inverted_match: color key min-max matching range is inverted
> + * @colorkey.replacement_mask: color key replacement mask value (in
> + * AXYZ16161616 format)
> + * @colorkey.replacement_value: color key replacement value (in
> + * AXYZ16161616 format)
> + * @colorkey.replacement_format: color key replacement value / mask
> + * pixel format (in DRM_FORMAT_AXYZ16161616 form)
> * @src: clipped source coordinates of the plane (in 16.16)
> * @dst: clipped destination coordinates of the plane
> * @state: backpointer to global drm_atomic_state
> @@ -124,6 +175,19 @@ struct drm_plane_state {
> unsigned int zpos;
> unsigned int normalized_zpos;
>
> + /* Plane colorkey */
> + struct {
> + enum drm_plane_colorkey_mode mode;
> + u64 min;
> + u64 max;
> + u64 mask;
> + u32 format;
> + bool inverted_match;
> + u64 replacement_mask;
> + u64 replacement_value;
> + u32 replacement_format;
> + } colorkey;
> +
> /**
>* @color_encoding:
>*
> @@ -510,6 +574,7 @@ enum drm_plane_type {
> * @alpha_property: alpha property for this plane
> * @zpos_property: zpos property for this plane
> * @rotation_property: rotation property for this plane
> + * @colorkey: colorkey properties for this plane
> * @helper_private: mid-layer private data
> */
> struct drm_plane {
> @@ -587,6 +652,18 @@ struct drm_plane {
> struct drm_property *zpos_property;
> struct drm_property *rotation_property;
>
> + struct {
> + struct drm_property *min_property;
> + struct drm_property *max_property;
> + struct drm_property *mode_property;
> + struct drm_property *mask_property;
> + struct drm_property *format_property;
> + struct drm_property *inverted_match_property;
> + struct drm_property *replacement_mask_property;
> + struct drm_property *replacement_value_property;
> + struct drm_property *replacement_format_property;
> + } colorkey;
> +
> /**
>* @color_encoding_property:
>*
> --
> 2.17.0
--
Ville Syrjälä
Intel
Re: [PATCH v2 2/5] drm/i915: hdmi: add CEC notifier to intel_hdmi
drm_connector_cleanup(connector);
> kfree(connector);
> @@ -2358,6 +2366,11 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
> u32 temp = I915_READ(PEG_BAND_GAP_DATA);
> I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
> }
> +
> + intel_hdmi->notifier = cec_notifier_get_conn(dev->dev,
> + port_identifier(port));
> + if (!intel_hdmi->notifier)
> + DRM_DEBUG_KMS("CEC notifier get failed\n");
> }
>
> void intel_hdmi_init(struct drm_i915_private *dev_priv,
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH v2 2/5] drm/i915: hdmi: add CEC notifier to intel_hdmi
On Wed, May 16, 2018 at 09:40:17AM +0200, Neil Armstrong wrote:
> On 16/05/2018 09:31, Neil Armstrong wrote:
> > Hi Ville,
> >
> > On 15/05/2018 17:35, Ville Syrjälä wrote:
> >> On Tue, May 15, 2018 at 04:42:19PM +0200, Neil Armstrong wrote:
> >>> This patchs adds the cec_notifier feature to the intel_hdmi part
> >>> of the i915 DRM driver. It uses the HDMI DRM connector name to
> >>> differentiate
> >>> between each HDMI ports.
> >>> The changes will allow the i915 HDMI code to notify EDID and HPD changes
> >>> to an eventual CEC adapter.
> >>>
> >>> Signed-off-by: Neil Armstrong
> >>> ---
> >>> drivers/gpu/drm/i915/Kconfig | 1 +
> >>> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> >>> drivers/gpu/drm/i915/intel_hdmi.c | 12
> >>> 3 files changed, 15 insertions(+)
> >>>
> >
> > [..]
> >
> >>> }
> >>>
> >>> @@ -2031,6 +2037,8 @@ static void chv_hdmi_pre_enable(struct
> >>> intel_encoder *encoder,
> >>>
> >>> static void intel_hdmi_destroy(struct drm_connector *connector)
> >>> {
> >>> + if (intel_attached_hdmi(connector)->notifier)
> >>> + cec_notifier_put(intel_attached_hdmi(connector)->notifier);
> >>> kfree(to_intel_connector(connector)->detect_edid);
> >>> drm_connector_cleanup(connector);
> >>> kfree(connector);
> >>> @@ -2358,6 +2366,10 @@ void intel_hdmi_init_connector(struct
> >>> intel_digital_port *intel_dig_port,
> >>> u32 temp = I915_READ(PEG_BAND_GAP_DATA);
> >>> I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
> >>> }
> >>> +
> >>> + intel_hdmi->notifier = cec_notifier_get_conn(dev->dev, connector->name);
> >>
> >> What are we doing with the connector name here? Those are not actually
> >> guaranteed to be stable, and any change in the connector probe order
> >> may cause the name to change.
> >
> > The i915 driver can expose multiple HDMI connectors, but each connected will
> > have a different EDID and CEC physical address, so we will need a different
> > notifier
> > for each connector.
> >
> > The connector name is DRM dependent, but user-space actually uses this name
> > for
> > operations, so I supposed it was kind of stable.
> >
> > Anyway, is there another stable id/name/whatever that can be used to make a
> > name to
> > distinguish the HDMI connectors ?
>
> Would "HDMI %c", port_name(port) be OK to use ?
Yeah, something like seems like a reasonable approach. That does mean
we have to be a little careful with changing enum port or port_name()
in the future, but I guess we could just add a small function to
generated the name/id specifically for this thing.
We're also going to have to think what to do with enum port and
port_name() on ICL+ though. There we won't just have A-F but also
TC1-TC. Hmm. Looks like what we have for those ports in our
codebase ATM is a bit wonky since we haven't even changed
port_name() to give us the TC type names.
Also we might not want "HDMI" in the identifier since the physical
port is not HDMI specific. There are different things we could call
these but I think we could just go with a generic "Port A-F" and
"Port TC1-TC" approach. I think something like that should work
fine for current and upcoming hardware. And in theory that could
then be used for other things as well which need a stable
identifier.
Oh, and now I recall that input subsystem at least has some kind
of "physical path" property on devices. So I guess what we're
dicussing is a somewhat similar idea. I think input drivers
generally include the pci/usb/etc. device in the path as well.
Even though we currently only ever have the one pci device it
would seem like decent idea to include that information in our
identifiers as well. Or what do you think?
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH v2 2/5] drm/i915: hdmi: add CEC notifier to intel_hdmi
On Tue, May 15, 2018 at 04:42:19PM +0200, Neil Armstrong wrote:
> This patchs adds the cec_notifier feature to the intel_hdmi part
> of the i915 DRM driver. It uses the HDMI DRM connector name to differentiate
> between each HDMI ports.
> The changes will allow the i915 HDMI code to notify EDID and HPD changes
> to an eventual CEC adapter.
>
> Signed-off-by: Neil Armstrong
> ---
> drivers/gpu/drm/i915/Kconfig | 1 +
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_hdmi.c | 12
> 3 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index dfd9588..2d65d56 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -23,6 +23,7 @@ config DRM_I915
> select SYNC_FILE
> select IOSF_MBI
> select CRC32
> + select CEC_CORE if CEC_NOTIFIER
> help
> Choose this option if you have a system that has "Intel Graphics
> Media Accelerator" or "HD Graphics" integrated graphics,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index d436858..b50e51b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -39,6 +39,7 @@
> #include
> #include
> #include
> +#include
>
> /**
> * __wait_for - magic wait macro
> @@ -1001,6 +1002,7 @@ struct intel_hdmi {
> bool has_audio;
> bool rgb_quant_range_selectable;
> struct intel_connector *attached_connector;
> + struct cec_notifier *notifier;
> };
>
> struct intel_dp_mst_encoder;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 1baef4a..e98103d 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1868,6 +1868,8 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> connected = true;
> }
>
> + cec_notifier_set_phys_addr_from_edid(intel_hdmi->notifier, edid);
> +
> return connected;
> }
>
> @@ -1876,6 +1878,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool
> force)
> {
> enum drm_connector_status status;
> struct drm_i915_private *dev_priv = to_i915(connector->dev);
> + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> connector->base.id, connector->name);
> @@ -1891,6 +1894,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool
> force)
>
> intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
>
> + if (status != connector_status_connected)
> + cec_notifier_phys_addr_invalidate(intel_hdmi->notifier);
> +
> return status;
> }
>
> @@ -2031,6 +2037,8 @@ static void chv_hdmi_pre_enable(struct intel_encoder
> *encoder,
>
> static void intel_hdmi_destroy(struct drm_connector *connector)
> {
> + if (intel_attached_hdmi(connector)->notifier)
> + cec_notifier_put(intel_attached_hdmi(connector)->notifier);
> kfree(to_intel_connector(connector)->detect_edid);
> drm_connector_cleanup(connector);
> kfree(connector);
> @@ -2358,6 +2366,10 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
> u32 temp = I915_READ(PEG_BAND_GAP_DATA);
> I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
> }
> +
> + intel_hdmi->notifier = cec_notifier_get_conn(dev->dev, connector->name);
What are we doing with the connector name here? Those are not actually
guaranteed to be stable, and any change in the connector probe order
may cause the name to change.
> + if (!intel_hdmi->notifier)
> + DRM_DEBUG_KMS("CEC notifier get failed\n");
> }
>
> void intel_hdmi_init(struct drm_i915_private *dev_priv,
> --
> 2.7.4
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
Re: [PATCH/RFC 1/4] drm: Add colorkey properties
> + if (!prop)
> + return -ENOMEM;
> +
> + if (replace) {
> + prop = CREATE_COLORKEY_PROP(plane, value, range, 0, U64_MAX);
> + if (!prop)
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_create_colorkey_properties);
> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
> index 17606026590b..1d4c965dd5e2 100644
> --- a/include/drm/drm_blend.h
> +++ b/include/drm/drm_blend.h
> @@ -49,4 +49,8 @@ int drm_plane_create_zpos_immutable_property(struct
> drm_plane *plane,
>unsigned int zpos);
> int drm_atomic_normalize_zpos(struct drm_device *dev,
> struct drm_atomic_state *state);
> +
> +int drm_plane_create_colorkey_properties(struct drm_plane *plane,
> + const struct drm_prop_enum_list *modes,
> + unsigned int num_modes, bool replace);
> #endif
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 8185e3468a23..a5804a4ea5c3 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -52,6 +52,13 @@ struct drm_modeset_acquire_ctx;
> * where N is the number of active planes for given crtc. Note that
> * the driver must call drm_atomic_normalize_zpos() to update this before
> * it can be trusted.
> + * @colorkey.mode: color key mode. 0 disabled color keying, other values are
> + * driver-specific.
> + * @colorkey.min: color key range minimum. The value is stored in
> AXYZ16161616
> + * format, where A is the alpha value and X, Y and Z correspond to the
> + * color components of the plane's pixel format (usually RGB or YUV).
> + * @colorkey.max: color key range maximum (in AXYZ16161616 format)
> + * @colorkey.value: color key replacement value (in in AXYZ16161616 format)
> * @src: clipped source coordinates of the plane (in 16.16)
> * @dst: clipped destination coordinates of the plane
> * @state: backpointer to global drm_atomic_state
> @@ -112,6 +119,14 @@ struct drm_plane_state {
> unsigned int zpos;
> unsigned int normalized_zpos;
>
> + /* Plane colorkey */
> + struct {
> + unsigned int mode;
> + u64 min;
> + u64 max;
> + u64 value;
> + } colorkey;
> +
> /* Clipped coordinates */
> struct drm_rect src, dst;
>
> @@ -481,9 +496,13 @@ enum drm_plane_type {
> * @funcs: helper functions
> * @properties: property tracking for this plane
> * @type: type of plane (overlay, primary, cursor)
> + * @helper_private: mid-layer private data
> * @zpos_property: zpos property for this plane
> * @rotation_property: rotation property for this plane
> - * @helper_private: mid-layer private data
> + * @colorkey.mode_property: color key mode property
> + * @colorkey.min_property: color key range minimum property
> + * @colorkey.max_property: color key range maximum property
> + * @colorkey.value_property: color key replacement value property
> */
> struct drm_plane {
> struct drm_device *dev;
> @@ -558,6 +577,13 @@ struct drm_plane {
>
> struct drm_property *zpos_property;
> struct drm_property *rotation_property;
> +
> + struct {
> + struct drm_property *mode_property;
> + struct drm_property *min_property;
> + struct drm_property *max_property;
> + struct drm_property *value_property;
> + } colorkey;
> };
>
> #define obj_to_plane(x) container_of(x, struct drm_plane, base)
> --
> Regards,
>
> Laurent Pinchart
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
On Fri, Jan 12, 2018 at 06:14:53PM +0100, Hans Verkuil wrote: > On 01/12/2018 05:30 PM, Ville Syrjälä wrote: > > On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote: > >> Hi Ville, > >> > >> For some strange reason your email disappeared from the Cc list. Perhaps > >> it's the > >> ä that confuses something somewhere. > >> > >> So I'll just forward this directly to you. > >> > >> Can you please take a look? This patch series has been in limbo for too > >> long. > > > > IIRC last I looked we still had some ragistration race to deal with. > > Was that fixed? > > That was fixed in v5. > > > > > Also I think we got stuck on leaving the zombie device lingering around > > when the display is disconnected. I couldn't understand why that is > > at all useful since you anyway remove the device eventually. > > It's not a zombie device. If you disconnect and reconnect the display then the > application using the CEC device will see the display disappear and reappear > as expected. > > It helps if you think of the normal situation (as is present in most ARM SoCs) > where CEC is integral to the HDMI transmitter. I.e. it is not functionality > that > can be removed. So the cec device is always there and an application opens the > device and can use it, regardless of whether a display is connected or not. > > If a display is detected, the EDID will be read and the CEC physical address > is > set. The application is informed of that through an event and the CEC adapter > can be used. If the HPD disappears the physical address is reset to f.f.f.f > and > again the application is informed. And in fact it still has to be able to use > the CEC adapter even if there is no HPD since some displays turn off the HPD > when > in standby, but CEC can still be used to power them up again. Hmm. So you're saying there are DP devices out there that release HPD but still respond to DPCD accesses? That sounds... wrong. In general I don't think we can assume that a device has retained its state if it has deasserted HPD, thus we have to reconfigure everything again anyway. > > Now consider a future Intel NUC with an HDMI connector on the backplane and > working DP CEC-Tunneling-over-AUX support (e.g. the Megachips MCDP2900): the > CEC support is always there (it's built in), but only becomes visible to the > kernel when you connect a display. You don't want the cec device to disappear > whenever you unplug the display, that makes no sense. Applications would > loose the CEC configuration and have to close and reopen (when it reappears) > the cec device for no good reason since it is built in. If the application can't remember its settings across a disconnect it sounds broken anwyay. This would anyway happen when you disconenct the entire dongle. > > The same situation is valid when using a USB-C to HDMI adapter: disconnecting > or reconnecting a display should not lead to the removal of the CEC device. > Only when an adapter with different CEC capabilities is detected is there a > need to actually unregister the CEC device. > > All this is really a workaround of the fact that when the HPD disappears the > DP-to-HDMI adapter (either external or built-in) also disappears from the > topology, even though it is physically still there. The dongles I've seen do not disappear. The downstream hpd is signalled with short hpd pulses + SINK_COUNT instead. But I've not actually seen a dongle that implements the BRANCH_DEVICE_CTRL DPCD register, so not quite sure what those would actually do. The spec does say they should default to using long hpd for downstream hpd handling. > If there was a way to > detect the adapter when there is no display connected, then this workaround > wouldn't be needed. > > This situation is specific to DisplayPort, this is the only case where the > HDMI connector disappears in a puff of smoke when you disconnect the HDMI > cable, even though the actual physical connector is obviously still there. > > Regards, > > Hans > > > > > Adding the lists back to cc so I don't have to repeat myself there... > > > >> > >> Regards, > >> > >>Hans > >> > >> > >> Forwarded Message > >> Subject: Re: [PATCHv5 0/3] drm/i915: add DisplayPort > >> CEC-Tunneling-over-AUX support > >> Date: Tue, 9 Jan 2018 13:46:44 +0100 > >> From: Hans Verkuil > >> To: linux-media@vger.kernel.org > >> CC: Daniel Vetter , Carlos Santa > >> , dri-de...@lists.freedesktop.org > >> > >&g
Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
to HDMI adapters at all). > >>> > >>> Hopefully if this gets merged there will be an incentive for vendors > >>> to make adapters where this actually works. It is a very nice feature > >>> for HTPC boxes. > >>> > >>> The main reason why this v5 is delayed by 2 months is due to the fact > >>> that I needed some dedicated time to investigate what happens when an > >>> MST hub is in use. It turns out that this is not working. There is no > >>> mechanism defined in the DisplayPort standard to transport the CEC > >>> interrupt back up the MST chain. I was actually able to send a CEC > >>> message but the interrupt that tells when the transmit finished is > >>> unavailable. > >>> > >>> I attempted to implement this via polling, but I got weird errors > >>> and was not able to read the DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1 > >>> register. I decided to give up on this for now and just disable CEC > >>> for DP-to-HDMI adapters after an MST hub. I plan to revisit this > >>> later since it would be neat to make this work as well. Although it > >>> might not be possible at all. > >>> > >>> If anyone is interested, work-in-progress for this is here: > >>> > >>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=dp-cec-mst > >>> > >>> Note that I removed the Tested-by tag from Carlos Santa due to the > >>> almost complete rework of the third patch. Carlos, can you test this > >>> again? > >>> > >>> Regards, > >>> > >>> Hans > >>> > >>> Changes since v4: > >>> > >>> - Updated comment at the start of drm_dp_cec.c > >>> - Add edid pointer to drm_dp_cec_configure_adapter > >>> - Reworked the last patch (adding CEC to i915) based on Ville's comments > >>> and my MST testing: > >>> - register/unregister CEC in intel_dp_connector_register/unregister > >>> - add comment and check if connector is registered in long_pulse > >>> - unregister CEC if an MST 'connector' is detected. > >>> > >>> ___ > >>> dri-devel mailing list > >>> dri-de...@lists.freedesktop.org > >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >>> > >> > >> ___ > >> dri-devel mailing list > >> dri-de...@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >> > > > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC
Re: [PATCH 1/2] drivers/video/hdmi: allow for larger-than-needed vendor IF
On Mon, Nov 20, 2017 at 04:02:14PM +0100, Hans Verkuil wrote: > On 11/20/2017 03:51 PM, Ville Syrjälä wrote: > > On Mon, Nov 20, 2017 at 02:41:28PM +0100, Hans Verkuil wrote: > >> From: Hans Verkuil > >> > >> Some devices (Windows Intel driver!) send a Vendor InfoFrame that > >> uses a payload length of 0x1b instead of the length of 5 or 6 > >> that the unpack code expects. The InfoFrame is padded with 0 by > >> the source. > > > > So it doesn't put any 3D_Metadata stuff in there? We don't see to > > have code to parse/generate any of that. > > I can't remember if it puts any 3D stuff in there. Let me know if you > want me to check this later this week. Would be nice to know. I guess we should really add code to parse/generate that stuff too, otherwise we're going to be lying when we unpack an infoframe with that stuff present. -- Ville Syrjälä Intel OTC
Re: [PATCH 10/10] video/hdmi: Pass buffer size to infoframe unpack functions
On Mon, Nov 20, 2017 at 02:36:20PM +0100, Hans Verkuil wrote:
> On 11/13/2017 06:04 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä
> > @@ -1163,7 +1176,7 @@ static int hdmi_audio_infoframe_unpack(struct
> > hdmi_audio_infoframe *frame,
> > */
> > static int
> > hdmi_vendor_any_infoframe_unpack(union hdmi_vendor_any_infoframe *frame,
> > -const void *buffer)
> > +const void *buffer, size_t size)
> > {
> > const u8 *ptr = buffer;
> > size_t length;
> > @@ -1171,6 +1184,9 @@ hdmi_vendor_any_infoframe_unpack(union
> > hdmi_vendor_any_infoframe *frame,
> > u8 hdmi_video_format;
> > struct hdmi_vendor_infoframe *hvf = &frame->hdmi;
> >
> > + if (size < HDMI_INFOFRAME_HEADER_SIZE)
> > + return -EINVAL;
> > +
>
> This check is not needed since that is already done in
> hdmi_infoframe_unpack().
Hmm. True. Somehow I was expecting that this function would have been
exported on its own, but it's static so clearly I was mistaken.
The pack functions are individually exported, which is where I got
this idea probably.
>
> > if (ptr[0] != HDMI_INFOFRAME_TYPE_VENDOR ||
> > ptr[1] != 1 ||
> > (ptr[2] != 4 && ptr[2] != 5 && ptr[2] != 6))
> > @@ -1178,6 +1194,9 @@ hdmi_vendor_any_infoframe_unpack(union
> > hdmi_vendor_any_infoframe *frame,
> >
> > length = ptr[2];
> >
> > + if (size < HDMI_INFOFRAME_HEADER_SIZE + length)
> > + return -EINVAL;
> > +
> > if (hdmi_infoframe_checksum(buffer,
> > HDMI_INFOFRAME_HEADER_SIZE + length) != 0)
> > return -EINVAL;
--
Ville Syrjälä
Intel OTC
Re: [PATCH 1/2] drivers/video/hdmi: allow for larger-than-needed vendor IF
On Mon, Nov 20, 2017 at 02:41:28PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Some devices (Windows Intel driver!) send a Vendor InfoFrame that
> uses a payload length of 0x1b instead of the length of 5 or 6
> that the unpack code expects. The InfoFrame is padded with 0 by
> the source.
So it doesn't put any 3D_Metadata stuff in there? We don't see to
have code to parse/generate any of that.
Sadly the spec doesn't seem to forbid sending an overly long infoframe
as long it's padded with 0. Would have been nicer for extending it if
that sort of thing was forbidden. But I guess everything can be solved
with flags. Not that I expect anyone to extend it anymore now that
HDMI 2.0 has specified a totally new infoframe.
>
> The current code thinks anything other than 5 or 6 is an error,
> but larger values are allowed by the specification. So support
> that here as well.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/video/hdmi.c | 3 +--
> include/linux/hdmi.h | 1 +
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index 1cf907ecded4..61f803f75a47 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -1164,8 +1164,7 @@ hdmi_vendor_any_infoframe_unpack(union
> hdmi_vendor_any_infoframe *frame,
> struct hdmi_vendor_infoframe *hvf = &frame->hdmi;
>
> if (ptr[0] != HDMI_INFOFRAME_TYPE_VENDOR ||
> - ptr[1] != 1 ||
> - (ptr[2] != 5 && ptr[2] != 6))
> + ptr[1] != 1 || ptr[2] < 5 || ptr[2] > HDMI_VENDOR_INFOFRAME_SIZE)
> return -EINVAL;
>
> length = ptr[2];
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index d271ff23984f..14d3531a0eda 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -40,6 +40,7 @@ enum hdmi_infoframe_type {
> #define HDMI_AVI_INFOFRAME_SIZE13
> #define HDMI_SPD_INFOFRAME_SIZE25
> #define HDMI_AUDIO_INFOFRAME_SIZE 10
> +#define HDMI_VENDOR_INFOFRAME_SIZE 31
>
> #define HDMI_INFOFRAME_SIZE(type)\
> (HDMI_INFOFRAME_HEADER_SIZE + HDMI_ ## type ## _INFOFRAME_SIZE)
> --
> 2.14.1
--
Ville Syrjälä
Intel OTC
Re: [PATCH 01/10] video/hdmi: Allow "empty" HDMI infoframes
On Thu, Nov 16, 2017 at 08:06:18PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 11/13/2017 10:34 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > HDMI 2.0 Appendix F suggest that we should keep sending the infoframe
> > when switching from 3D to 2D mode, even if the infoframe isn't strictly
> > necessary (ie. not needed to transmit the VIC or stereo information).
> > This is a workaround against some sinks that fail to realize that they
> > should switch from 3D to 2D mode when the source stop transmitting
> > the infoframe.
> >
> > v2: Handle unpack() as well
> > Pull the length calculation into a helper
> >
> > Cc: Shashank Sharma
> > Cc: Andrzej Hajda
> > Cc: Thierry Reding
> > Cc: Hans Verkuil
> > Cc: linux-media@vger.kernel.org
> > Reviewed-by: Andrzej Hajda #v1
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/video/hdmi.c | 51
> > +++
> > 1 file changed, 31 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> > index 1cf907ecded4..111a0ab6280a 100644
> > --- a/drivers/video/hdmi.c
> > +++ b/drivers/video/hdmi.c
> > @@ -321,6 +321,17 @@ int hdmi_vendor_infoframe_init(struct
> > hdmi_vendor_infoframe *frame)
> > }
> > EXPORT_SYMBOL(hdmi_vendor_infoframe_init);
> >
> > +static int hdmi_vendor_infoframe_length(const struct hdmi_vendor_infoframe
> > *frame)
> > +{
> > + /* for side by side (half) we also need to provide 3D_Ext_Data */
> > + if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
> > + return 6;
> > + else if (frame->vic != 0 || frame->s3d_struct !=
> > HDMI_3D_STRUCTURE_INVALID)
> > + return 5;
> > + else
> > + return 4;
> > +}
> > +
> > /**
> >* hdmi_vendor_infoframe_pack() - write a HDMI vendor infoframe to binary
> > buffer
> >* @frame: HDMI infoframe
> > @@ -341,19 +352,11 @@ ssize_t hdmi_vendor_infoframe_pack(struct
> > hdmi_vendor_infoframe *frame,
> > u8 *ptr = buffer;
> > size_t length;
> >
> > - /* empty info frame */
> > - if (frame->vic == 0 && frame->s3d_struct == HDMI_3D_STRUCTURE_INVALID)
> > - return -EINVAL;
> > -
> > /* only one of those can be supplied */
> > if (frame->vic != 0 && frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID)
> > return -EINVAL;
> >
> > - /* for side by side (half) we also need to provide 3D_Ext_Data */
> > - if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
> > - frame->length = 6;
> > - else
> > - frame->length = 5;
> > + frame->length = hdmi_vendor_infoframe_length(frame);
> >
> > length = HDMI_INFOFRAME_HEADER_SIZE + frame->length;
> >
> > @@ -372,14 +375,16 @@ ssize_t hdmi_vendor_infoframe_pack(struct
> > hdmi_vendor_infoframe *frame,
> > ptr[5] = 0x0c;
> > ptr[6] = 0x00;
> >
> > - if (frame->vic) {
> > - ptr[7] = 0x1 << 5; /* video format */
> > - ptr[8] = frame->vic;
> > - } else {
> > + if (frame->s3d_struct != HDMI_3D_STRUCTURE_INVALID) {
> > ptr[7] = 0x2 << 5; /* video format */
> > ptr[8] = (frame->s3d_struct & 0xf) << 4;
> > if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
> > ptr[9] = (frame->s3d_ext_data & 0xf) << 4;
> > + } else if (frame->vic) {
> Please correct me if I dint understand this properly, but for a HDMI 2.0
> sink + 3D transmission, wouldn't I be sending
> HDMI 2.0 VIC = 94 as well as some valid s3d flag (like side by side) ?
That vic will be in the AVI infoframe. Here we're concerned about the
VIC in the HDMI vendor infoframe.
>
> - Shashank
> > + ptr[7] = 0x1 << 5; /* video format */
> > + ptr[8] = frame->vic;
> > + } else {
> > + ptr[7] = 0x0 << 5; /* video format */
> > }
> >
> > hdmi_infoframe_set_checksum(buffer, length);
> > @@ -1165,7 +1170,7 @@ hdmi_vendor_any_infoframe_unpack(union
> > hdmi_vendor_any_infoframe *frame,
> >
> > if (ptr[0] != HDMI_INFOFRAME_TYPE_VENDOR ||
> > ptr[1] != 1 ||
> > - (ptr[2] != 5 && ptr[2] != 6))
> > + (ptr
Re: [PATCH 0/4] dma-buf: Silence dma_fence __rcu sparse warnings
On Tue, Nov 07, 2017 at 01:37:10PM +0530, Sumit Semwal wrote: > Hi Ville, > > On 3 November 2017 at 13:18, Christian König wrote: > > Patch #4 is Reviewed-by: Christian König . > > > > The rest is Acked-by: Christian König . > > > > Regards, > > Christian. > > > > > > Am 02.11.2017 um 21:03 schrieb Ville Syrjala: > >> > >> From: Ville Syrjälä > >> > >> When building drm+i915 I get around 150 lines of sparse noise from > >> dma_fence __rcu warnings. This series eliminates all of that. > >> > >> The first two patches were already posted by Chris, but there wasn't > >> any real reaction, so I figured I'd repost with a wider Cc list. > >> > >> As for the other two patches, I'm no expert on dma_fence and I didn't > >> spend a lot of time looking at it so I can't be sure I annotated all > >> the accesses correctly. But I figured someone will scream at me if > >> I got it wrong ;) > >> > >> Cc: Dave Airlie > >> Cc: Jason Ekstrand > >> Cc: linaro-mm-...@lists.linaro.org > >> Cc: linux-media@vger.kernel.org > >> Cc: Alex Deucher > >> Cc: Christian König > >> Cc: Sumit Semwal > >> Cc: Chris Wilson > >> > >> Chris Wilson (2): > >>drm/syncobj: Mark up the fence as an RCU protected pointer > >>dma-buf/fence: Sparse wants __rcu on the object itself > >> > >> Ville Syrjälä (2): > >>drm/syncobj: Use proper methods for accessing rcu protected pointers > >>dma-buf: Use rcu_assign_pointer() to set rcu protected pointers > > For patches 2 (with Daniel's minor comment) and 4, please feel free to add my > Acked-by: Sumit Semwal > >> > >> drivers/dma-buf/reservation.c | 2 +- > >> drivers/gpu/drm/drm_syncobj.c | 11 +++ > >> include/drm/drm_syncobj.h | 2 +- > >> include/linux/dma-fence.h | 2 +- > >> 4 files changed, 10 insertions(+), 7 deletions(-) > >> > > > > Best, > Sumit. -- Ville Syrjälä Intel OTC
Re: [PATCHv4 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
On Mon, Sep 18, 2017 at 05:26:43PM +0200, Hans Verkuil wrote:
> On 09/18/2017 04:36 PM, Ville Syrjälä wrote:
> > On Mon, Sep 18, 2017 at 04:07:41PM +0200, Hans Verkuil wrote:
> >> Hi Ville,
> >>
> >> On 09/18/2017 03:05 PM, Ville Syrjälä wrote:
> >>> On Sat, Sep 16, 2017 at 04:17:26PM +0200, Hans Verkuil wrote:
> >>>> From: Hans Verkuil
> >>>>
> >>>> Implement support for this DisplayPort feature.
> >>>>
> >>>> The cec device is created whenever it detects an adapter that
> >>>> has this feature. It is only removed when a new adapter is connected
> >>>> that does not support this. If a new adapter is connected that has
> >>>> different properties than the previous one, then the old cec device is
> >>>> unregistered and a new one is registered to replace the old one.
> >>>>
> >>>> Signed-off-by: Hans Verkuil
> >>>> Tested-by: Carlos Santa
> >>>> ---
> >>>> drivers/gpu/drm/i915/intel_dp.c | 18 ++
> >>>> 1 file changed, 14 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >>>> b/drivers/gpu/drm/i915/intel_dp.c
> >>>> index 64fa774c855b..fdb853d2c458 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_dp.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >>>> @@ -32,6 +32,7 @@
> >>>> #include
> >>>> #include
> >>>> #include
> >>>> +#include
> >>>> #include
> >>>> #include
> >>>> #include
> >>>> @@ -1449,6 +1450,7 @@ static void intel_aux_reg_init(struct intel_dp
> >>>> *intel_dp)
> >>>> static void
> >>>> intel_dp_aux_fini(struct intel_dp *intel_dp)
> >>>> {
> >>>> +cec_unregister_adapter(intel_dp->aux.cec_adap);
> >>>> kfree(intel_dp->aux.name);
> >>>> }
> >>>>
> >>>> @@ -4587,6 +4589,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
> >>>> intel_connector->detect_edid = edid;
> >>>>
> >>>> intel_dp->has_audio = drm_detect_monitor_audio(edid);
> >>>> +cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
> >>>> }
> >>>>
> >>>> static void
> >>>> @@ -4596,6 +4599,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> >>>>
> >>>> kfree(intel_connector->detect_edid);
> >>>> intel_connector->detect_edid = NULL;
> >>>> +cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
> >>>>
> >>>> intel_dp->has_audio = false;
> >>>> }
> >>>> @@ -4616,13 +4620,17 @@ intel_dp_long_pulse(struct intel_connector
> >>>> *intel_connector)
> >>>> intel_display_power_get(to_i915(dev),
> >>>> intel_dp->aux_power_domain);
> >>>>
> >>>> /* Can't disconnect eDP, but you can close the lid... */
> >>>> -if (is_edp(intel_dp))
> >>>> +if (is_edp(intel_dp)) {
> >>>> status = edp_detect(intel_dp);
> >>>> -else if (intel_digital_port_connected(to_i915(dev),
> >>>> - dp_to_dig_port(intel_dp)))
> >>>> +} else if (intel_digital_port_connected(to_i915(dev),
> >>>> +
> >>>> dp_to_dig_port(intel_dp))) {
> >>>> status = intel_dp_detect_dpcd(intel_dp);
> >>>> -else
> >>>> +if (status == connector_status_connected)
> >>>> +drm_dp_cec_configure_adapter(&intel_dp->aux,
> >>>> + intel_dp->aux.name, dev->dev);
> >>>
> >>> This is cluttering up the code a bit. Maybe do this call somewhere
> >>> around the intel_dp_configure_mst() call instead since that seems to be
> >>> the place where we start to do changes to externally visible state.
> >>>
> >>> Actually, do we want to register cec adapters for MST devices?
> >>>
> >>> And shouldn't we call this regardless of the co
Re: [PATCHv4 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
On Mon, Sep 18, 2017 at 04:07:41PM +0200, Hans Verkuil wrote:
> Hi Ville,
>
> On 09/18/2017 03:05 PM, Ville Syrjälä wrote:
> > On Sat, Sep 16, 2017 at 04:17:26PM +0200, Hans Verkuil wrote:
> >> From: Hans Verkuil
> >>
> >> Implement support for this DisplayPort feature.
> >>
> >> The cec device is created whenever it detects an adapter that
> >> has this feature. It is only removed when a new adapter is connected
> >> that does not support this. If a new adapter is connected that has
> >> different properties than the previous one, then the old cec device is
> >> unregistered and a new one is registered to replace the old one.
> >>
> >> Signed-off-by: Hans Verkuil
> >> Tested-by: Carlos Santa
> >> ---
> >> drivers/gpu/drm/i915/intel_dp.c | 18 ++
> >> 1 file changed, 14 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >> b/drivers/gpu/drm/i915/intel_dp.c
> >> index 64fa774c855b..fdb853d2c458 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -32,6 +32,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >> #include
> >> #include
> >> #include
> >> @@ -1449,6 +1450,7 @@ static void intel_aux_reg_init(struct intel_dp
> >> *intel_dp)
> >> static void
> >> intel_dp_aux_fini(struct intel_dp *intel_dp)
> >> {
> >> + cec_unregister_adapter(intel_dp->aux.cec_adap);
> >>kfree(intel_dp->aux.name);
> >> }
> >>
> >> @@ -4587,6 +4589,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
> >>intel_connector->detect_edid = edid;
> >>
> >>intel_dp->has_audio = drm_detect_monitor_audio(edid);
> >> + cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
> >> }
> >>
> >> static void
> >> @@ -4596,6 +4599,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> >>
> >>kfree(intel_connector->detect_edid);
> >>intel_connector->detect_edid = NULL;
> >> + cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
> >>
> >>intel_dp->has_audio = false;
> >> }
> >> @@ -4616,13 +4620,17 @@ intel_dp_long_pulse(struct intel_connector
> >> *intel_connector)
> >>intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
> >>
> >>/* Can't disconnect eDP, but you can close the lid... */
> >> - if (is_edp(intel_dp))
> >> + if (is_edp(intel_dp)) {
> >>status = edp_detect(intel_dp);
> >> - else if (intel_digital_port_connected(to_i915(dev),
> >> -dp_to_dig_port(intel_dp)))
> >> + } else if (intel_digital_port_connected(to_i915(dev),
> >> + dp_to_dig_port(intel_dp))) {
> >>status = intel_dp_detect_dpcd(intel_dp);
> >> - else
> >> + if (status == connector_status_connected)
> >> + drm_dp_cec_configure_adapter(&intel_dp->aux,
> >> + intel_dp->aux.name, dev->dev);
> >
> > This is cluttering up the code a bit. Maybe do this call somewhere
> > around the intel_dp_configure_mst() call instead since that seems to be
> > the place where we start to do changes to externally visible state.
> >
> > Actually, do we want to register cec adapters for MST devices?
> >
> > And shouldn't we call this regardless of the connector state so that
> > the cec adapter gets unregistered when the device is disconnected?
>
> This hasn't (AFAIK) anything to do with MST. This is in a branch device (i.e.
> a DP to HDMI adapter).
You are now potentiall registering the CEC adapter to the immediately
upstream MST device (ie. the one that we talk to over the normal AUX stuff),
but kms will consider that paticular connector as disconnected, and
instead only sinks downstream of that device may have connected connectors
associated with them. Presumably the CEC towards that device goes
nowhere, and instead we'd have to talk to the remote branch devices
somewhere downstream.
Thus my question whether we want to potentially register the CEC adapter
to the immediately upstream MST device or not. I would imagine not, and
thus the call should perhaps be moved past the 'is_mst? -> disconnected'
checks.
>
> The CEC adapter should ideally be associate
Re: [PATCHv4 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
On Sat, Sep 16, 2017 at 04:17:26PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Implement support for this DisplayPort feature.
>
> The cec device is created whenever it detects an adapter that
> has this feature. It is only removed when a new adapter is connected
> that does not support this. If a new adapter is connected that has
> different properties than the previous one, then the old cec device is
> unregistered and a new one is registered to replace the old one.
>
> Signed-off-by: Hans Verkuil
> Tested-by: Carlos Santa
> ---
> drivers/gpu/drm/i915/intel_dp.c | 18 ++
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 64fa774c855b..fdb853d2c458 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -32,6 +32,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -1449,6 +1450,7 @@ static void intel_aux_reg_init(struct intel_dp
> *intel_dp)
> static void
> intel_dp_aux_fini(struct intel_dp *intel_dp)
> {
> + cec_unregister_adapter(intel_dp->aux.cec_adap);
> kfree(intel_dp->aux.name);
> }
>
> @@ -4587,6 +4589,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
> intel_connector->detect_edid = edid;
>
> intel_dp->has_audio = drm_detect_monitor_audio(edid);
> + cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
> }
>
> static void
> @@ -4596,6 +4599,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>
> kfree(intel_connector->detect_edid);
> intel_connector->detect_edid = NULL;
> + cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
>
> intel_dp->has_audio = false;
> }
> @@ -4616,13 +4620,17 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
> intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
>
> /* Can't disconnect eDP, but you can close the lid... */
> - if (is_edp(intel_dp))
> + if (is_edp(intel_dp)) {
> status = edp_detect(intel_dp);
> - else if (intel_digital_port_connected(to_i915(dev),
> - dp_to_dig_port(intel_dp)))
> + } else if (intel_digital_port_connected(to_i915(dev),
> + dp_to_dig_port(intel_dp))) {
> status = intel_dp_detect_dpcd(intel_dp);
> - else
> + if (status == connector_status_connected)
> + drm_dp_cec_configure_adapter(&intel_dp->aux,
> + intel_dp->aux.name, dev->dev);
This is cluttering up the code a bit. Maybe do this call somewhere
around the intel_dp_configure_mst() call instead since that seems to be
the place where we start to do changes to externally visible state.
Actually, do we want to register cec adapters for MST devices?
And shouldn't we call this regardless of the connector state so that
the cec adapter gets unregistered when the device is disconnected?
> + } else {
> status = connector_status_disconnected;
> + }
>
> if (status == connector_status_disconnected) {
> memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
> @@ -5011,6 +5019,8 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
>
> intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>
> + drm_dp_cec_irq(&intel_dp->aux);
> +
> if (intel_dp->is_mst) {
> if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
> /*
> --
> 2.14.1
--
Ville Syrjälä
Intel OTC
Re: [PATCHv4 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX
uess what we want to do is defer this registration until that
time, unless the connector is already registered. drm_dp_aux_register()
might be the right spot for this call, or maybe we want to put it next
to that call in driver code? Not sure.
Hmm. And I guess that's going to be somewhat racy unless we protect
the direct registration with connector->mutex. I hope that can't
deadlock...
> + if (err) {
> + cec_delete_adapter(aux->cec_adap);
> + aux->cec_adap = NULL;
> + }
> + return err;
> +}
> +EXPORT_SYMBOL(drm_dp_cec_configure_adapter);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index b17476a6909c..0e236dd40b42 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -952,6 +952,8 @@ struct drm_dp_aux_msg {
> size_t size;
> };
>
> +struct cec_adapter;
> +
> /**
> * struct drm_dp_aux - DisplayPort AUX channel
> * @name: user-visible name of this AUX channel and the I2C-over-AUX adapter
> @@ -1010,6 +1012,10 @@ struct drm_dp_aux {
>* @i2c_defer_count: Counts I2C DEFERs, used for DP validation.
>*/
> unsigned i2c_defer_count;
> + /**
> + * @cec_adap: the CEC adapter for CEC-Tunneling-over-AUX support.
> + */
> + struct cec_adapter *cec_adap;
> };
>
> ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
> @@ -1132,4 +1138,22 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum
> drm_dp_quirk quirk)
> return desc->quirks & BIT(quirk);
> }
>
> +#ifdef CONFIG_DRM_DP_CEC
> +bool drm_dp_cec_irq(struct drm_dp_aux *aux);
> +int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char *name,
> + struct device *parent);
> +#else
> +static inline bool drm_dp_cec_irq(struct drm_dp_aux *aux)
> +{
> + return false;
> +}
> +
> +static inline int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux,
> +const char *name,
> +struct device *parent)
> +{
> + return -ENODEV;
> +}
> +#endif
> +
> #endif /* _DRM_DP_HELPER_H_ */
> --
> 2.14.1
--
Ville Syrjälä
Intel OTC
Re: [PATCHv3 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX
On Wed, Sep 13, 2017 at 10:51:02AM +0200, Hans Verkuil wrote: > On 09/12/2017 07:39 PM, Ville Syrjälä wrote: > > On Mon, Sep 11, 2017 at 01:25:45PM +0200, Hans Verkuil wrote: > >> From: Hans Verkuil > >> > >> This adds support for the DisplayPort CEC-Tunneling-over-AUX > >> feature that is part of the DisplayPort 1.3 standard. > >> > >> Unfortunately, not all DisplayPort/USB-C to HDMI adapters with a > >> chip that has this capability actually hook up the CEC pin, so > >> even though a CEC device is created, it may not actually work. > >> > >> Signed-off-by: Hans Verkuil > >> Tested-by: Carlos Santa > >> --- > >> drivers/gpu/drm/Kconfig | 10 ++ > >> drivers/gpu/drm/Makefile | 1 + > >> drivers/gpu/drm/drm_dp_cec.c | 301 > >> +++ > >> include/drm/drm_dp_helper.h | 24 > >> 4 files changed, 336 insertions(+) > >> create mode 100644 drivers/gpu/drm/drm_dp_cec.c > >> > >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > >> index 83cb2a88c204..1f2708df5c4e 100644 > >> --- a/drivers/gpu/drm/Kconfig > >> +++ b/drivers/gpu/drm/Kconfig > >> @@ -120,6 +120,16 @@ config DRM_LOAD_EDID_FIRMWARE > >> default case is N. Details and instructions how to build your own > >> EDID data are given in Documentation/EDID/HOWTO.txt. > >> > >> +config DRM_DP_CEC > >> + bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support" > >> + select CEC_CORE > >> + help > >> +Choose this option if you want to enable HDMI CEC support for > >> +DisplayPort/USB-C to HDMI adapters. > >> + > >> +Note: not all adapters support this feature, and even for those > >> +that do support this they often do not hook up the CEC pin. > >> + > >> config DRM_TTM > >>tristate > >>depends on DRM && MMU > >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > >> index 24a066e1841c..c6552c62049e 100644 > >> --- a/drivers/gpu/drm/Makefile > >> +++ b/drivers/gpu/drm/Makefile > >> @@ -40,6 +40,7 @@ drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += > >> drm_edid_load.o > >> drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o > >> drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o > >> drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o > >> +drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o > >> > >> obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o > >> obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/ > >> diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c > >> new file mode 100644 > >> index ..b831bb72c932 > >> --- /dev/null > >> +++ b/drivers/gpu/drm/drm_dp_cec.c > >> @@ -0,0 +1,301 @@ > >> +/* > >> + * DisplayPort CEC-Tunneling-over-AUX support > >> + * > >> + * Copyright 2017 Cisco Systems, Inc. and/or its affiliates. All rights > >> reserved. > >> + * > >> + * This program is free software; you may redistribute it and/or modify > >> + * it under the terms of the GNU General Public License as published by > >> + * the Free Software Foundation; version 2 of the License. > >> + * > >> + * 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 > >> +#include > >> +#include > >> +#include > >> +#include > >> + > >> +/* > >> + * Unfortunately it turns out that we have a chicken-and-egg situation > >> + * here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters > >> + * have a converter chip that supports CEC-Tunneling-over-AUX (usually the > >> + * Parade PS176), but they do not wire up the CEC pin, thus making CEC > >> + * useless. > >> + * > >> + * Sadly there is
Re: [PATCHv3 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX
NG_CAPABLE)) {
> + cec_unregister_adapter(aux->cec_adap);
> + aux->cec_adap = NULL;
> + return -ENODEV;
> + }
> +
> + if (cap & DP_CEC_SNOOPING_CAPABLE)
> + cec_caps |= CEC_CAP_MONITOR_ALL;
> + if (cap & DP_CEC_MULTIPLE_LA_CAPABLE)
> + num_las = CEC_MAX_LOG_ADDRS;
> +
> + if (aux->cec_adap) {
> + if (aux->cec_adap->capabilities == cec_caps &&
> + aux->cec_adap->available_log_addrs == num_las)
> + return 0;
> + cec_unregister_adapter(aux->cec_adap);
> + }
> +
> + aux->cec_adap = cec_allocate_adapter(&drm_dp_cec_adap_ops,
> + aux, name, cec_caps, num_las);
> + if (IS_ERR(aux->cec_adap)) {
> + err = PTR_ERR(aux->cec_adap);
> + aux->cec_adap = NULL;
> + return err;
> + }
> + err = cec_register_adapter(aux->cec_adap, parent);
> + if (err) {
> + cec_delete_adapter(aux->cec_adap);
> + aux->cec_adap = NULL;
> + }
> + return err;
> +}
> +EXPORT_SYMBOL(drm_dp_cec_configure_adapter);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index b17476a6909c..0e236dd40b42 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -952,6 +952,8 @@ struct drm_dp_aux_msg {
> size_t size;
> };
>
> +struct cec_adapter;
> +
> /**
> * struct drm_dp_aux - DisplayPort AUX channel
> * @name: user-visible name of this AUX channel and the I2C-over-AUX adapter
> @@ -1010,6 +1012,10 @@ struct drm_dp_aux {
>* @i2c_defer_count: Counts I2C DEFERs, used for DP validation.
>*/
> unsigned i2c_defer_count;
> + /**
> + * @cec_adap: the CEC adapter for CEC-Tunneling-over-AUX support.
> + */
> + struct cec_adapter *cec_adap;
> };
>
> ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
> @@ -1132,4 +1138,22 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum
> drm_dp_quirk quirk)
> return desc->quirks & BIT(quirk);
> }
>
> +#ifdef CONFIG_DRM_DP_CEC
> +bool drm_dp_cec_irq(struct drm_dp_aux *aux);
> +int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char *name,
> + struct device *parent);
> +#else
> +static inline bool drm_dp_cec_irq(struct drm_dp_aux *aux)
> +{
> + return false;
> +}
> +
> +static inline int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux,
> +const char *name,
> +struct device *parent)
> +{
> + return -ENODEV;
> +}
> +#endif
> +
> #endif /* _DRM_DP_HELPER_H_ */
> --
> 2.14.1
--
Ville Syrjälä
Intel OTC
Re: DRM Atomic property for color-space conversion
On Fri, Mar 17, 2017 at 10:33:15AM +, Brian Starkey wrote: > Hi Ville, > > On Thu, Mar 16, 2017 at 07:36:56PM +0200, Ville Syrjälä wrote: > >On Thu, Mar 16, 2017 at 07:05:12PM +0200, Sharma, Shashank wrote: > >> On 3/16/2017 5:55 PM, Brian Starkey wrote: > >> > On Thu, Mar 16, 2017 at 05:14:07PM +0200, Sharma Shashank wrote: > >> >> On 3/16/2017 4:37 PM, Local user for Liviu Dudau wrote: > >> >>> On Thu, Mar 16, 2017 at 04:30:59PM +0200, Ville Syrjälä wrote: > >> >>>> On Thu, Mar 16, 2017 at 04:20:29PM +0200, Sharma, Shashank wrote: > >> >>>>> On 3/16/2017 4:07 PM, Ville Syrjälä wrote: > > [snip] > > >> >>>>>> > >> >>>>>> So what we might need is something like: > >> >>>>>> enum YCBCR_TO_RGB_CSC > >> >>>>>> * YCbCr BT.601 limited to RGB BT.709 full > >> >>>>>> * YCbCr BT.709 limited to RGB BT.709 full >> >>>>>> likely default value IMO> > >> >>>>>> * YCbCr BT.601 limited to RGB BT.2020 full > >> >>>>>> * YCbCr BT.709 limited to RGB BT.2020 full > >> >>>>>> * YCbCr BT.2020 limited to RGB BT.2020 full > >> >>>>>> > >> >>>>>> And thanks to BT.2020 we'll need a RGB->RGB CSC property as well. > >> >>>>>> Eg: > >> >>>>>> enum RGB_TO_RGB_CSC > >> >>>>>> * bypass (or separate 709->709, 2020->2020?) >> >>>>>> default> > >> >>>>>> * RGB BT.709 full to RGB BT.2020 full > >> > > >> > I like this approach, from a point of view of being explicit and > >> > discoverable by userspace. It also happens to map quite nicely to our > >> > hardware... we have a matrix before degamma, so we could do a > >> > CSC + Gamut conversion there in one go, which is apparently not 100% > >> > mathematically correct, but in general is good enough. > >> > > >> > ... however having talked this over a bit with someone who understands > >> > the detail a lot better than me, it sounds like the "correct" thing to > >> > do as per the spec is: > >> > > >> > CSC -> DEGAMMA -> GAMUT > >> > > >> > e.g. > >> > > >> > YCbCr bt.601 limited to RGB bt.601 full -> degamma -> > >> > RGB bt.601 full to RGB bt.709 full > >> > > >> > So that sounds like what we need to support in the API, and also > >> > sounds more like the "separate properties" approach. > >> I agree. > >> > > >> >>>>>> > >> >>>>>> Alternatives would involve two properties to define the input and > >> >>>>>> output > >> >>>>>> from the CSC separately, but then you lose the capability to see > >> >>>>>> which > >> >>>>>> combinations are actually supoorted. > >> >>>>> I was thinking about this too, or would it make more sense to > >> >>>>> create two > >> >>>>> properties: > >> >>>>> - one for gamut mapping (cases like RGB709->RGB2020) > >> >>>>> - other one for Color space conversion (cases lile YUV 709 -> RGB > >> >>>>> 709) > >> >>>>> > >> >>>>> Gamut mapping can represent any of the fix function mapping, > >> >>>>> wereas CSC > >> >>>>> can bring up any programmable matrix > >> >>>>> > >> >>>>> Internally these properties can use the same HW unit or even same > >> >>>>> function. > >> >>>>> Does it sound any good ? > >> > > >> > It seems to me that actually the two approaches can be combined into > >> > the same thing: > >> > * We definitely need a YCbCr-to-RGB conversion before degamma > >> >(for converting YUV data to RGB, in some flavour) > >> > * We definitely need an RGB-to-RGB conversion after gamma to handle > >> >709 layers blended with Rec.2020. > >> > The exact conversion each of those properties represents (CSC + gamut, > >> > CSC only, gamut only) can be implicit in the enum name. > >>
Re: DRM Atomic property for color-space conversion
On Thu, Mar 16, 2017 at 07:05:12PM +0200, Sharma, Shashank wrote: > Regards > > Shashank > > > On 3/16/2017 5:55 PM, Brian Starkey wrote: > > Hi, > > > > On Thu, Mar 16, 2017 at 05:14:07PM +0200, Sharma Shashank wrote: > >> Regards > >> > >> Shashank > >> > >> > >> On 3/16/2017 4:37 PM, Local user for Liviu Dudau wrote: > >>> On Thu, Mar 16, 2017 at 04:30:59PM +0200, Ville Syrjälä wrote: > >>>> On Thu, Mar 16, 2017 at 04:20:29PM +0200, Sharma, Shashank wrote: > >>>>> Regards > >>>>> > >>>>> Shashank > >>>>> > >>>>> > >>>>> On 3/16/2017 4:07 PM, Ville Syrjälä wrote: > >>>>>> On Tue, Jan 31, 2017 at 03:55:41PM +, Brian Starkey wrote: > >>>>>>> On Tue, Jan 31, 2017 at 05:15:46PM +0200, Ville Syrjälä wrote: > >>>>>>>> On Tue, Jan 31, 2017 at 12:33:29PM +, Brian Starkey wrote: > >>>>>>>>> Hi, > >>>>>>>>> > >>>>>>>>> On Mon, Jan 30, 2017 at 03:35:13PM +0200, Ville Syrjälä wrote: > >>>>>>>>>> On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: > >>>>>>>>>>> Hi, > >>>>>>>>>>> > >>>>>>>>>>> We're looking to enable the per-plane color management > >>>>>>>>>>> hardware in > >>>>>>>>>>> Mali-DP with atomic properties, which has sparked some > >>>>>>>>>>> conversation > >>>>>>>>>>> around how to handle YCbCr formats. > >>>>>>>>>>> > >>>>>>>>>>> As it stands today, it's assumed that a driver will > >>>>>>>>>>> implicitly "do the > >>>>>>>>>>> right thing" to display a YCbCr buffer. > >>>>>>>>>>> > >>>>>>>>>>> YCbCr data often uses different gamma curves and signal > >>>>>>>>>>> ranges (e.g. > >>>>>>>>>>> BT.609, BT.701, BT.2020, studio range, full-range), so its > >>>>>>>>>>> desirable > >>>>>>>>>>> to be able to explicitly control the YCbCr to RGB conversion > >>>>>>>>>>> process > >>>>>>>>>>> from userspace. > >>>>>>>>>>> > >>>>>>>>>>> We're proposing adding a "CSC" (color-space conversion) > >>>>>>>>>>> property to > >>>>>>>>>>> control this - primarily per-plane for framebuffer->pipeline > >>>>>>>>>>> CSC, but > >>>>>>>>>>> perhaps one per CRTC too for devices which have an RGB > >>>>>>>>>>> pipeline and > >>>>>>>>>>> want to output in YUV to the display: > >>>>>>>>>>> > >>>>>>>>>>> Name: "CSC" > >>>>>>>>>>> Type: ENUM | ATOMIC; > >>>>>>>>>>> Enum values (representative): > >>>>>>>>>>> "default": > >>>>>>>>>>> Same behaviour as now. "Some kind" of YCbCr->RGB conversion > >>>>>>>>>>> for YCbCr buffers, bypass for RGB buffers > >>>>>>>>>>> "disable": > >>>>>>>>>>> Explicitly disable all colorspace conversion (i.e. use an > >>>>>>>>>>> identity matrix). > >>>>>>>>>>> "YCbCr to RGB: BT.709": > >>>>>>>>>>> Only valid for YCbCr formats. CSC in accordance with BT.709 > >>>>>>>>>>> using [16..235] for (8-bit) luma values, and [16..240] for > >>>>>>>>>>> 8-bit chroma values. For 10-bit formats, the range > >>>>>>>>>>> limits are > >>>>>>>>>>> multiplied by 4. > >>>>>>>>>>> "YCbCr to RG
Re: DRM Atomic property for color-space conversion
On Thu, Mar 16, 2017 at 04:20:29PM +0200, Sharma, Shashank wrote: > Regards > > Shashank > > > On 3/16/2017 4:07 PM, Ville Syrjälä wrote: > > On Tue, Jan 31, 2017 at 03:55:41PM +, Brian Starkey wrote: > >> On Tue, Jan 31, 2017 at 05:15:46PM +0200, Ville Syrjälä wrote: > >>> On Tue, Jan 31, 2017 at 12:33:29PM +, Brian Starkey wrote: > >>>> Hi, > >>>> > >>>> On Mon, Jan 30, 2017 at 03:35:13PM +0200, Ville Syrjälä wrote: > >>>>> On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: > >>>>>> Hi, > >>>>>> > >>>>>> We're looking to enable the per-plane color management hardware in > >>>>>> Mali-DP with atomic properties, which has sparked some conversation > >>>>>> around how to handle YCbCr formats. > >>>>>> > >>>>>> As it stands today, it's assumed that a driver will implicitly "do the > >>>>>> right thing" to display a YCbCr buffer. > >>>>>> > >>>>>> YCbCr data often uses different gamma curves and signal ranges (e.g. > >>>>>> BT.609, BT.701, BT.2020, studio range, full-range), so its desirable > >>>>>> to be able to explicitly control the YCbCr to RGB conversion process > >>>>>> from userspace. > >>>>>> > >>>>>> We're proposing adding a "CSC" (color-space conversion) property to > >>>>>> control this - primarily per-plane for framebuffer->pipeline CSC, but > >>>>>> perhaps one per CRTC too for devices which have an RGB pipeline and > >>>>>> want to output in YUV to the display: > >>>>>> > >>>>>> Name: "CSC" > >>>>>> Type: ENUM | ATOMIC; > >>>>>> Enum values (representative): > >>>>>> "default": > >>>>>>Same behaviour as now. "Some kind" of YCbCr->RGB conversion > >>>>>>for YCbCr buffers, bypass for RGB buffers > >>>>>> "disable": > >>>>>>Explicitly disable all colorspace conversion (i.e. use an > >>>>>>identity matrix). > >>>>>> "YCbCr to RGB: BT.709": > >>>>>>Only valid for YCbCr formats. CSC in accordance with BT.709 > >>>>>>using [16..235] for (8-bit) luma values, and [16..240] for > >>>>>>8-bit chroma values. For 10-bit formats, the range limits are > >>>>>>multiplied by 4. > >>>>>> "YCbCr to RGB: BT.709 full-swing": > >>>>>>Only valid for YCbCr formats. CSC in accordance with BT.709, > >>>>>>but using the full range of each channel. > >>>>>> "YCbCr to RGB: Use CTM":* > >>>>>>Only valid for YCbCr formats. Use the matrix applied via the > >>>>>>plane's CTM property > >>>>>> "RGB to RGB: Use CTM":* > >>>>>>Only valid for RGB formats. Use the matrix applied via the > >>>>>>plane's CTM property > >>>>>> "Use CTM":* > >>>>>>Valid for any format. Use the matrix applied via the plane's > >>>>>>CTM property > >>>>>> ... any other values for BT.601, BT.2020, RGB to YCbCr etc. etc. as > >>>>>> they are required. > >>>>> Having some RGB2RGB and YCBCR2RGB things in the same property seems > >>>>> weird. I would just go with something very simple like: > >>>>> > >>>>> YCBCR_TO_RGB_CSC: > >>>>> * BT.601 > >>>>> * BT.709 > >>>>> * custom matrix > >>>>> > >>>> I think we've agreed in #dri-devel that this CSC property > >>>> can't/shouldn't be mapped on-to the existing (hardware implementing > >>>> the) CTM property - even in the case of per-plane color management - > >>>> because CSC needs to be done before DEGAMMA. > >>>> > >>>> So, I'm in favour of going with what you suggested in the first place: > >>>> > >>>> A new YCBCR_TO_RGB_CSC property, enum type, with a list of fixed
Re: DRM Atomic property for color-space conversion
On Tue, Jan 31, 2017 at 03:55:41PM +, Brian Starkey wrote: > On Tue, Jan 31, 2017 at 05:15:46PM +0200, Ville Syrjälä wrote: > >On Tue, Jan 31, 2017 at 12:33:29PM +, Brian Starkey wrote: > >> Hi, > >> > >> On Mon, Jan 30, 2017 at 03:35:13PM +0200, Ville Syrjälä wrote: > >> >On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: > >> >> Hi, > >> >> > >> >> We're looking to enable the per-plane color management hardware in > >> >> Mali-DP with atomic properties, which has sparked some conversation > >> >> around how to handle YCbCr formats. > >> >> > >> >> As it stands today, it's assumed that a driver will implicitly "do the > >> >> right thing" to display a YCbCr buffer. > >> >> > >> >> YCbCr data often uses different gamma curves and signal ranges (e.g. > >> >> BT.609, BT.701, BT.2020, studio range, full-range), so its desirable > >> >> to be able to explicitly control the YCbCr to RGB conversion process > >> >> from userspace. > >> >> > >> >> We're proposing adding a "CSC" (color-space conversion) property to > >> >> control this - primarily per-plane for framebuffer->pipeline CSC, but > >> >> perhaps one per CRTC too for devices which have an RGB pipeline and > >> >> want to output in YUV to the display: > >> >> > >> >> Name: "CSC" > >> >> Type: ENUM | ATOMIC; > >> >> Enum values (representative): > >> >> "default": > >> >> Same behaviour as now. "Some kind" of YCbCr->RGB conversion > >> >> for YCbCr buffers, bypass for RGB buffers > >> >> "disable": > >> >> Explicitly disable all colorspace conversion (i.e. use an > >> >> identity matrix). > >> >> "YCbCr to RGB: BT.709": > >> >> Only valid for YCbCr formats. CSC in accordance with BT.709 > >> >> using [16..235] for (8-bit) luma values, and [16..240] for > >> >> 8-bit chroma values. For 10-bit formats, the range limits are > >> >> multiplied by 4. > >> >> "YCbCr to RGB: BT.709 full-swing": > >> >> Only valid for YCbCr formats. CSC in accordance with BT.709, > >> >> but using the full range of each channel. > >> >> "YCbCr to RGB: Use CTM":* > >> >> Only valid for YCbCr formats. Use the matrix applied via the > >> >> plane's CTM property > >> >> "RGB to RGB: Use CTM":* > >> >> Only valid for RGB formats. Use the matrix applied via the > >> >> plane's CTM property > >> >> "Use CTM":* > >> >> Valid for any format. Use the matrix applied via the plane's > >> >> CTM property > >> >> ... any other values for BT.601, BT.2020, RGB to YCbCr etc. etc. as > >> >> they are required. > >> > > >> >Having some RGB2RGB and YCBCR2RGB things in the same property seems > >> >weird. I would just go with something very simple like: > >> > > >> >YCBCR_TO_RGB_CSC: > >> >* BT.601 > >> >* BT.709 > >> >* custom matrix > >> > > >> > >> I think we've agreed in #dri-devel that this CSC property > >> can't/shouldn't be mapped on-to the existing (hardware implementing > >> the) CTM property - even in the case of per-plane color management - > >> because CSC needs to be done before DEGAMMA. > >> > >> So, I'm in favour of going with what you suggested in the first place: > >> > >> A new YCBCR_TO_RGB_CSC property, enum type, with a list of fixed > >> conversions. I'd drop the custom matrix for now, as we'd need to add > >> another property to attach the custom matrix blob too. > >> > >> I still think we need a way to specify whether the source data range > >> is broadcast/full-range, so perhaps the enum list should be expanded > >> to all combinations of BT.601/BT.709 + broadcast/full-range. > > > >Sounds reasonable. Not that much full range YCbCr stuff out there > >perhaps. Well, apart from jpegs I suppose. But no harm in being able > >to deal with it. > > > &g
Re: [PATCH v6 1/3] drm_fourcc: Add new P010, P016 video format
On Tue, Mar 07, 2017 at 01:58:23AM +0800, Ayaka wrote:
>
>
> 從我的 iPad 傳送
>
> > Ville Syrjälä 於 2017年3月6日 下午9:06 寫道：
> >
> >> On Sun, Mar 05, 2017 at 06:00:31PM +0800, Randy Li wrote:
> >> P010 is a planar 4:2:0 YUV with interleaved UV plane, 10 bits
> >> per channel video format.
> >>
> >> P016 is a planar 4:2:0 YUV with interleaved UV plane, 16 bits
> >> per channel video format.
> >>
> >> V3: Added P012 and fixed cpp for P010
> >> V4: format definition refined per review
> >> V5: Format comment block for each new pixel format
> >> V6: reversed Cb/Cr order in comments
> >> v7: reversed Cb/Cr order in comments of header files, remove
> >> the wrong part of commit message.
> >
> > What? Why? You just undid what Clint did in v6.
> He missed a file also keeping the wrong description of rockchip.
I don't follow. Who missed what exactly?
> >
> >>
> >> Cc: Daniel Stone
> >> Cc: Ville Syrjälä
> >>
> >> Signed-off-by: Randy Li
> >> Signed-off-by: Clint Taylor
> >> ---
> >> drivers/gpu/drm/drm_fourcc.c | 3 +++
> >> include/uapi/drm/drm_fourcc.h | 21 +
> >> 2 files changed, 24 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> >> index 90d2cc8..3e0fd58 100644
> >> --- a/drivers/gpu/drm/drm_fourcc.c
> >> +++ b/drivers/gpu/drm/drm_fourcc.c
> >> @@ -165,6 +165,9 @@ const struct drm_format_info *__drm_format_info(u32
> >> format)
> >>{ .format = DRM_FORMAT_UYVY,.depth = 0, .num_planes = 1,
> >> .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> >>{ .format = DRM_FORMAT_VYUY,.depth = 0, .num_planes = 1,
> >> .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> >>{ .format = DRM_FORMAT_AYUV,.depth = 0, .num_planes = 1,
> >> .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> >> +{ .format = DRM_FORMAT_P010,.depth = 0, .num_planes = 2,
> >> .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 },
> >> +{ .format = DRM_FORMAT_P012,.depth = 0, .num_planes = 2,
> >> .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 },
> >> +{ .format = DRM_FORMAT_P016,.depth = 0, .num_planes = 2,
> >> .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 },
> >>};
> >>
> >>unsigned int i;
> >> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> >> index ef20abb..306f979 100644
> >> --- a/include/uapi/drm/drm_fourcc.h
> >> +++ b/include/uapi/drm/drm_fourcc.h
> >> @@ -128,6 +128,27 @@ extern "C" {
> >> #define DRM_FORMAT_NV42fourcc_code('N', 'V', '4', '2') /*
> >> non-subsampled Cb:Cr plane */
> >>
> >> /*
> >> + * 2 plane YCbCr MSB aligned
> >> + * index 0 = Y plane, [15:0] Y:x [10:6] little endian
> >> + * index 1 = Cb:Cr plane, [31:0] Cb:x:Cr:x [10:6:10:6] little endian
> >> + */
> >> +#define DRM_FORMAT_P010fourcc_code('P', '0', '1', '0') /* 2x2
> >> subsampled Cb:Cr plane 10 bits per channel */
> >> +
> >> +/*
> >> + * 2 plane YCbCr MSB aligned
> >> + * index 0 = Y plane, [15:0] Y:x [12:4] little endian
> >> + * index 1 = Cb:Cr plane, [31:0] Cb:x:Cr:x [12:4:12:4] little endian
> >> + */
> >> +#define DRM_FORMAT_P012fourcc_code('P', '0', '1', '2') /* 2x2
> >> subsampled Cb:Cr plane 12 bits per channel */
> >> +
> >> +/*
> >> + * 2 plane YCbCr MSB aligned
> >> + * index 0 = Y plane, [15:0] Y little endian
> >> + * index 1 = Cb:Cr plane, [31:0] Cb:Cr [16:16] little endian
> >> + */
> >> +#define DRM_FORMAT_P016fourcc_code('P', '0', '1', '6') /* 2x2
> >> subsampled Cb:Cr plane 16 bits per channel */
> >> +
> >> +/*
> >> * 3 plane YCbCr
> >> * index 0: Y plane, [7:0] Y
> >> * index 1: Cb plane, [7:0] Cb
> >> --
> >> 2.7.4
> >
> > --
> > Ville Syrjälä
> > Intel OTC
--
Ville Syrjälä
Intel OTC
Re: [PATCH v6 1/3] drm_fourcc: Add new P010, P016 video format
On Sun, Mar 05, 2017 at 06:00:31PM +0800, Randy Li wrote:
> P010 is a planar 4:2:0 YUV with interleaved UV plane, 10 bits
> per channel video format.
>
> P016 is a planar 4:2:0 YUV with interleaved UV plane, 16 bits
> per channel video format.
>
> V3: Added P012 and fixed cpp for P010
> V4: format definition refined per review
> V5: Format comment block for each new pixel format
> V6: reversed Cb/Cr order in comments
> v7: reversed Cb/Cr order in comments of header files, remove
> the wrong part of commit message.
What? Why? You just undid what Clint did in v6.
>
> Cc: Daniel Stone
> Cc: Ville Syrjälä
>
> Signed-off-by: Randy Li
> Signed-off-by: Clint Taylor
> ---
> drivers/gpu/drm/drm_fourcc.c | 3 +++
> include/uapi/drm/drm_fourcc.h | 21 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 90d2cc8..3e0fd58 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -165,6 +165,9 @@ const struct drm_format_info *__drm_format_info(u32
> format)
> { .format = DRM_FORMAT_UYVY,.depth = 0,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_VYUY,.depth = 0,
> .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
> { .format = DRM_FORMAT_AYUV,.depth = 0,
> .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
> + { .format = DRM_FORMAT_P010,.depth = 0,
> .num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 },
> + { .format = DRM_FORMAT_P012,.depth = 0,
> .num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 },
> + { .format = DRM_FORMAT_P016,.depth = 0,
> .num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 },
> };
>
> unsigned int i;
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index ef20abb..306f979 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -128,6 +128,27 @@ extern "C" {
> #define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /*
> non-subsampled Cb:Cr plane */
>
> /*
> + * 2 plane YCbCr MSB aligned
> + * index 0 = Y plane, [15:0] Y:x [10:6] little endian
> + * index 1 = Cb:Cr plane, [31:0] Cb:x:Cr:x [10:6:10:6] little endian
> + */
> +#define DRM_FORMAT_P010 fourcc_code('P', '0', '1', '0') /* 2x2
> subsampled Cb:Cr plane 10 bits per channel */
> +
> +/*
> + * 2 plane YCbCr MSB aligned
> + * index 0 = Y plane, [15:0] Y:x [12:4] little endian
> + * index 1 = Cb:Cr plane, [31:0] Cb:x:Cr:x [12:4:12:4] little endian
> + */
> +#define DRM_FORMAT_P012 fourcc_code('P', '0', '1', '2') /* 2x2
> subsampled Cb:Cr plane 12 bits per channel */
> +
> +/*
> + * 2 plane YCbCr MSB aligned
> + * index 0 = Y plane, [15:0] Y little endian
> + * index 1 = Cb:Cr plane, [31:0] Cb:Cr [16:16] little endian
> + */
> +#define DRM_FORMAT_P016 fourcc_code('P', '0', '1', '6') /* 2x2
> subsampled Cb:Cr plane 16 bits per channel */
> +
> +/*
> * 3 plane YCbCr
> * index 0: Y plane, [7:0] Y
> * index 1: Cb plane, [7:0] Cb
> --
> 2.7.4
--
Ville Syrjälä
Intel OTC
Re: DRM Atomic property for color-space conversion
On Tue, Jan 31, 2017 at 12:33:29PM +, Brian Starkey wrote: > Hi, > > On Mon, Jan 30, 2017 at 03:35:13PM +0200, Ville Syrjälä wrote: > >On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: > >> Hi, > >> > >> We're looking to enable the per-plane color management hardware in > >> Mali-DP with atomic properties, which has sparked some conversation > >> around how to handle YCbCr formats. > >> > >> As it stands today, it's assumed that a driver will implicitly "do the > >> right thing" to display a YCbCr buffer. > >> > >> YCbCr data often uses different gamma curves and signal ranges (e.g. > >> BT.609, BT.701, BT.2020, studio range, full-range), so its desirable > >> to be able to explicitly control the YCbCr to RGB conversion process > >> from userspace. > >> > >> We're proposing adding a "CSC" (color-space conversion) property to > >> control this - primarily per-plane for framebuffer->pipeline CSC, but > >> perhaps one per CRTC too for devices which have an RGB pipeline and > >> want to output in YUV to the display: > >> > >> Name: "CSC" > >> Type: ENUM | ATOMIC; > >> Enum values (representative): > >> "default": > >>Same behaviour as now. "Some kind" of YCbCr->RGB conversion > >>for YCbCr buffers, bypass for RGB buffers > >> "disable": > >>Explicitly disable all colorspace conversion (i.e. use an > >>identity matrix). > >> "YCbCr to RGB: BT.709": > >>Only valid for YCbCr formats. CSC in accordance with BT.709 > >>using [16..235] for (8-bit) luma values, and [16..240] for > >>8-bit chroma values. For 10-bit formats, the range limits are > >>multiplied by 4. > >> "YCbCr to RGB: BT.709 full-swing": > >>Only valid for YCbCr formats. CSC in accordance with BT.709, > >>but using the full range of each channel. > >> "YCbCr to RGB: Use CTM":* > >>Only valid for YCbCr formats. Use the matrix applied via the > >>plane's CTM property > >> "RGB to RGB: Use CTM":* > >>Only valid for RGB formats. Use the matrix applied via the > >>plane's CTM property > >> "Use CTM":* > >>Valid for any format. Use the matrix applied via the plane's > >>CTM property > >> ... any other values for BT.601, BT.2020, RGB to YCbCr etc. etc. as > >> they are required. > > > >Having some RGB2RGB and YCBCR2RGB things in the same property seems > >weird. I would just go with something very simple like: > > > >YCBCR_TO_RGB_CSC: > >* BT.601 > >* BT.709 > >* custom matrix > > > > I think we've agreed in #dri-devel that this CSC property > can't/shouldn't be mapped on-to the existing (hardware implementing > the) CTM property - even in the case of per-plane color management - > because CSC needs to be done before DEGAMMA. > > So, I'm in favour of going with what you suggested in the first place: > > A new YCBCR_TO_RGB_CSC property, enum type, with a list of fixed > conversions. I'd drop the custom matrix for now, as we'd need to add > another property to attach the custom matrix blob too. > > I still think we need a way to specify whether the source data range > is broadcast/full-range, so perhaps the enum list should be expanded > to all combinations of BT.601/BT.709 + broadcast/full-range. Sounds reasonable. Not that much full range YCbCr stuff out there perhaps. Well, apart from jpegs I suppose. But no harm in being able to deal with it. > > (I'm not sure what the canonical naming for broadcast/full-range is, > we call them narrow and wide) We tend to call them full vs. limited range. That's how our "Broadcast RGB" property is defined as well. > > >And trying to use the same thing for the crtc stuff is probably not > >going to end well. Like Daniel said we already have the > >'Broadcast RGB' property muddying the waters there, and that stuff > >also ties in with what colorspace we signal to the sink via > >infoframes/whatever the DP thing was called. So my gut feeling is > >that trying to use the same property everywhere will just end up > >messy. > > Yeah, agreed. If/when someone wants to add CSC on the output of a CRTC > (after GAMMA), we can add a new property. > > That makes me wonder about calling this one SOURCE_YCBCR_TO_RGB_CSC to > be explicit that i
Re: DRM Atomic property for color-space conversion
On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: > Hi, > > We're looking to enable the per-plane color management hardware in > Mali-DP with atomic properties, which has sparked some conversation > around how to handle YCbCr formats. > > As it stands today, it's assumed that a driver will implicitly "do the > right thing" to display a YCbCr buffer. > > YCbCr data often uses different gamma curves and signal ranges (e.g. > BT.609, BT.701, BT.2020, studio range, full-range), so its desirable > to be able to explicitly control the YCbCr to RGB conversion process > from userspace. > > We're proposing adding a "CSC" (color-space conversion) property to > control this - primarily per-plane for framebuffer->pipeline CSC, but > perhaps one per CRTC too for devices which have an RGB pipeline and > want to output in YUV to the display: > > Name: "CSC" > Type: ENUM | ATOMIC; > Enum values (representative): > "default": > Same behaviour as now. "Some kind" of YCbCr->RGB conversion > for YCbCr buffers, bypass for RGB buffers > "disable": > Explicitly disable all colorspace conversion (i.e. use an > identity matrix). > "YCbCr to RGB: BT.709": > Only valid for YCbCr formats. CSC in accordance with BT.709 > using [16..235] for (8-bit) luma values, and [16..240] for > 8-bit chroma values. For 10-bit formats, the range limits are > multiplied by 4. > "YCbCr to RGB: BT.709 full-swing": > Only valid for YCbCr formats. CSC in accordance with BT.709, > but using the full range of each channel. > "YCbCr to RGB: Use CTM":* > Only valid for YCbCr formats. Use the matrix applied via the > plane's CTM property > "RGB to RGB: Use CTM":* > Only valid for RGB formats. Use the matrix applied via the > plane's CTM property > "Use CTM":* > Valid for any format. Use the matrix applied via the plane's > CTM property > ... any other values for BT.601, BT.2020, RGB to YCbCr etc. etc. as > they are required. Having some RGB2RGB and YCBCR2RGB things in the same property seems weird. I would just go with something very simple like: YCBCR_TO_RGB_CSC: * BT.601 * BT.709 * custom matrix And trying to use the same thing for the crtc stuff is probably not going to end well. Like Daniel said we already have the 'Broadcast RGB' property muddying the waters there, and that stuff also ties in with what colorspace we signal to the sink via infoframes/whatever the DP thing was called. So my gut feeling is that trying to use the same property everywhere will just end up messy. -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] drm_fourcc: Add new P010 video format
On Thu, Jan 05, 2017 at 12:31:27AM +0800, ayaka wrote:
>
>
> On 01/04/2017 11:56 PM, Ville Syrjälä wrote:
> > On Mon, Jan 02, 2017 at 04:50:03PM +0800, Randy Li wrote:
> >> P010 is a planar 4:2:0 YUV with interleaved UV plane, 10 bits
> >> per channel video format. Rockchip's vop support this
> >> video format(little endian only) as the input video format.
> >>
> >> Signed-off-by: Randy Li
> >> ---
> >> include/uapi/drm/drm_fourcc.h | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> >> index 9e1bb7f..d2721da 100644
> >> --- a/include/uapi/drm/drm_fourcc.h
> >> +++ b/include/uapi/drm/drm_fourcc.h
> >> @@ -119,6 +119,7 @@ extern "C" {
> >> #define DRM_FORMAT_NV61 fourcc_code('N', 'V', '6', '1') /* 2x1
> >> subsampled Cb:Cr plane */
> >> #define DRM_FORMAT_NV24 fourcc_code('N', 'V', '2', '4') /*
> >> non-subsampled Cr:Cb plane */
> >> #define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /*
> >> non-subsampled Cb:Cr plane */
> >> +#define DRM_FORMAT_P010 fourcc_code('P', '0', '1', '0') /* 2x2
> >> subsampled Cr:Cb plane 10 bits per channel */
> > We could use a better description of the format here. IIRC there is
> > 10bits of actual data contained in each 16bits. So there should be a
> > proper comment explaning in which way the bits are stored.
> It is a little hard to describe P010,
/*
* 2 plane YCbCr
* index 0 = Y plane, [15:0] Y:X 10:6 little-endian
* index 1 = Cr:Cb plane, [31:0] Cr:X:Cb:X 10:6:10:6 little-endian
*/
/*
* 2 plane YCbCr
* index 0 = Y plane, [15:0] Y 16 little-endian
* index 1 = Cr:Cb plane, [31:0] Cr:Cb 16:16 little-endian
*/
or something like that (not 100% sure I got the order of bits and
whatnot correct).
--
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] drm_fourcc: Add new P010 video format
On Mon, Jan 02, 2017 at 04:50:03PM +0800, Randy Li wrote:
> P010 is a planar 4:2:0 YUV with interleaved UV plane, 10 bits
> per channel video format. Rockchip's vop support this
> video format(little endian only) as the input video format.
>
> Signed-off-by: Randy Li
> ---
> include/uapi/drm/drm_fourcc.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 9e1bb7f..d2721da 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -119,6 +119,7 @@ extern "C" {
> #define DRM_FORMAT_NV61 fourcc_code('N', 'V', '6', '1') /* 2x1
> subsampled Cb:Cr plane */
> #define DRM_FORMAT_NV24 fourcc_code('N', 'V', '2', '4') /*
> non-subsampled Cr:Cb plane */
> #define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /*
> non-subsampled Cb:Cr plane */
> +#define DRM_FORMAT_P010 fourcc_code('P', '0', '1', '0') /* 2x2
> subsampled Cr:Cb plane 10 bits per channel */
We could use a better description of the format here. IIRC there is
10bits of actual data contained in each 16bits. So there should be a
proper comment explaning in which way the bits are stored.
>
> /*
> * 3 plane YCbCr
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 00/11] Introduce writeback connectors
connector_state. > > Ville touched on scaling support previously, suggesting adding a > fixed_mode property (for all types of connectors) - on writeback this > would represent scaling the framebuffer, and on normal connectors it > could control output scaling (like panel-fitting). We got some patches [1] posted for i915 recently that added a bunch of new properties to control post-blending scaling, but I'm not sure I like the approach since it seems harder to reconcile with the current way we deal with scaling for eDP/LVDS/DSI/etc. So I'm still somewhat partial to the fixed mode idea. Just FYI. [1] https://lists.freedesktop.org/archives/intel-gfx/2016-August/105557.html > > Certainly destination coords, color-space converstion etc. are things > that are worth adding, but IMO I'd rather keep this initial > implementation small so we can enable the basic case right away. For > the most part, the additional things are "just properties" which > should be easily added later without impacting the overall interface. > > Cheers, > Brian > > > >Thanks, > >Archit > > > >> > >>Thanks, > >> > >>-Brian > >> > >>--- > >> > >>Brian Starkey (10): > >> drm: add writeback connector type > >> drm/fb-helper: skip writeback connectors > >> drm: extract CRTC/plane disable from drm_framebuffer_remove > >> drm: add __drm_framebuffer_remove_atomic > >> drm: add fb to connector state > >> drm: expose fb_id property for writeback connectors > >> drm: add writeback-connector pixel format properties > >> drm: mali-dp: rename malidp_input_format > >> drm: mali-dp: add RGB writeback formats for DP550/DP650 > >> drm: mali-dp: add writeback connector > >> > >>Liviu Dudau (1): > >> drm: mali-dp: Add support for writeback on DP550/DP650 > >> > >> drivers/gpu/drm/arm/Makefile|1 + > >> drivers/gpu/drm/arm/malidp_crtc.c | 10 ++ > >> drivers/gpu/drm/arm/malidp_drv.c| 25 +++- > >> drivers/gpu/drm/arm/malidp_drv.h|5 + > >> drivers/gpu/drm/arm/malidp_hw.c | 104 ++ > >> drivers/gpu/drm/arm/malidp_hw.h | 27 +++- > >> drivers/gpu/drm/arm/malidp_mw.c | 268 > >> +++ > >> drivers/gpu/drm/arm/malidp_planes.c |8 +- > >> drivers/gpu/drm/arm/malidp_regs.h | 15 ++ > >> drivers/gpu/drm/drm_atomic.c| 40 ++ > >> drivers/gpu/drm/drm_atomic_helper.c |4 + > >> drivers/gpu/drm/drm_connector.c | 79 ++- > >> drivers/gpu/drm/drm_crtc.c | 14 +- > >> drivers/gpu/drm/drm_fb_helper.c |4 + > >> drivers/gpu/drm/drm_framebuffer.c | 249 > >> drivers/gpu/drm/drm_ioctl.c |7 + > >> include/drm/drmP.h |2 + > >> include/drm/drm_atomic.h|3 + > >> include/drm/drm_connector.h | 15 ++ > >> include/drm/drm_crtc.h | 12 ++ > >> include/uapi/drm/drm.h | 10 ++ > >> include/uapi/drm/drm_mode.h |1 + > >> 22 files changed, 830 insertions(+), 73 deletions(-) > >> create mode 100644 drivers/gpu/drm/arm/malidp_mw.c > >> > > > >-- > >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-media" in > >the body of a message to majord...@vger.kernel.org > >More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 00/11] Introduce writeback connectors
8 +- > drivers/gpu/drm/arm/malidp_regs.h | 15 ++ > drivers/gpu/drm/drm_atomic.c| 40 ++ > drivers/gpu/drm/drm_atomic_helper.c |4 + > drivers/gpu/drm/drm_connector.c | 79 ++- > drivers/gpu/drm/drm_crtc.c | 14 +- > drivers/gpu/drm/drm_fb_helper.c |4 + > drivers/gpu/drm/drm_framebuffer.c | 249 > drivers/gpu/drm/drm_ioctl.c |7 + > include/drm/drmP.h |2 + > include/drm/drm_atomic.h|3 + > include/drm/drm_connector.h | 15 ++ > include/drm/drm_crtc.h | 12 ++ > include/uapi/drm/drm.h | 10 ++ > include/uapi/drm/drm_mode.h |1 + > 22 files changed, 830 insertions(+), 73 deletions(-) > create mode 100644 drivers/gpu/drm/arm/malidp_mw.c > > -- > 1.7.9.5 -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: DRM device memory writeback (Mali-DP)
the scaler. > >> I don't know if/how the DRM device should communicate to userspace > >> that the scaler is or isn't available for use. > >> > >> Any thoughts on this approach? > >> Is it acceptable to both V4L2 and DRM folks? > > > >For streaming a V4L2 capture device seems like the right interface. But if > >you want to use writeback in your compositor you must know which atomic > >kms update results in which frame, since if you don't you can't use that > >composited buffer for the next frame reliable. > > > >For that case I think a drm-only solution would be better, to make sure > >you can do an atomic update and writeback in one step. v4l seems to grow > >an atomic api of its own, but I don't think we'll have one spanning > >subsystems anytime soon. > > > > I've been thinking about this from the point of view of a HWComposer > implementation and I think the hybrid DRM-V4L2 device would work OK. > However it depends on the behaviour I mentioned above: > > >> if the screen is actively displaying a > >> static scene and the V4L2 client queues up a buffer, it won't get > >> filled until the DRM scene changes. > > V4L2 buffer queues are FIFO, so as long as the compositor queues only > one V4L2 buffer per atomic update, there's no ambiguity. > In the most simplistic case the compositor would alternate between: > - Queue V4L2 buffer > - DRM atomic update > ... and dequeue either in the same thread or a different one. As long > as the compositor keeps track of how many buffers it has queued and > how many atomic updates it's made, it doesn't really matter. > > We'd probably be looking to add in V4L2 asynchronous dequeue using > fences for synchronisation, but that's a separate issue. > > >For the kms-only interface the idea was to add a property on the crtc > >where you can attach a writeback drm_framebuffer. Extending that idea to > >the drm->v4l case we could create special drm_framebuffer objects > >representing a v4l sink, and attach them to the same property. That would > >also solve the problem of getting some agreement on buffer metadata > >between v4l and drm side. > > > > I think a drm_framebuffer on its own wouldn't be enough to handle our > scaling case - at that point it starts to look more like a plane. > However, if "special" CRTC sinks became a thing it could allow us to > chain our writeback output to another CRTC's input (via memory) > without a trip through userspace, which would be nice. My thinking has been that we could represent the writeback sink as a connector. Combine that with my other idea of exposing a fixed mode property for each connector (to make output scaling user configurable), and we should end up with a way to control the scaling of the writeback path. Also it would mean there's always a connector attached to the crtc, even for the pure writeback only case, which I think should result in less problems in general as I'm sure there are a lot of assumptions in the code that there must be at least one connector for an active crtc. > >Laurent had some poc patches a while ago for this, he's definitely the one > >to ping. > > I've had a little look at: https://patchwork.kernel.org/patch/6026611/ > It looks pretty similar to what I was hoping to do. > > We have the advantage of being able to update the display scene and > writeback buffer together atomically in hardware, and to write-back in > a one-shot mode. This lets us make the DRM update queue and V4L2 > buffer queues advance in lock-step. > > >-Daniel > > Thanks, > -Brian > > > >> > >> Thanks for your time, > >> > >> -Brian > >> > >> [1] > >> http://malideveloper.arm.com/resources/drivers/open-source-mali-dp-adf-kernel-device-drivers/ > >> [2] > >> https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&date=2016-05-04 > >> ___ > >> 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 > > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
On Mon, Nov 25, 2013 at 09:57:23AM +0100, Daniel Vetter wrote: > On Fri, Nov 22, 2013 at 02:12:13PM -0800, Kristian Høgsberg wrote: > > I don't know what else you'd propose? Pass an X visual in the ioctl? > > An EGL config? This is our name space, we can add stuff as we need > > (as Keith is doing here). include/uapi/drm/drm_fourcc.h is the > > canonical source for these values and we should add > > DRM_FORMAT_SARGB there to make sure we don't clash. > > Well that's kinda the problem. If you don't expect the kernel to clash > with whatever mesa is using internally then we should add it to the > kernel, first. That's kinda what Dave's recent rant has all been about. > > The other issue was that originally the idea behind fourcc was to have one > formate namespace shared between drm, v4l and whomever else cares. If > people are happy to drop that idea on the floor I won't shed a single > tear. I broke that idea alredy when I cooked up the current drm fourccs. I didn't cross check them with any other fourcc source, so I'm 100% sure most of them don't match anything else. -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
On Fri, Nov 22, 2013 at 03:43:13PM -0800, Keith Packard wrote: > Ville Syrjälä writes: > > > What is this format anyway? -ENODOCS > > Same as MESA_FORMAT_SARGB8 and __DRI_IMAGE_FORMAT_SARGB8 :-) > > > If its just an srgb version of ARGB, then I wouldn't really want it > > in drm_fourcc.h. I expect colorspacy stuff will be handled by various > > crtc/plane properties in the kernel so we don't need to encode that > > stuff into the fb format. > > It's not any different from splitting YUV codes from RGB codes; Not really. Saying something is YUV (or rather Y'CbCr) doesn't actually tell you the color space. It just tells you whether the information is encoded as R+G+B or Y+Cb+Cr. How you convert between them is another matter. You need to know the gamma, color primaries, chroma siting for sub-sampled YCbCr formats, etc. -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-gfx] [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
On Fri, Nov 22, 2013 at 02:12:13PM -0800, Kristian Høgsberg wrote: > On Fri, Nov 22, 2013 at 05:17:37PM +0100, Daniel Vetter wrote: > > On Fri, Nov 22, 2013 at 12:01 PM, Keith Packard wrote: > > > Daniel Vetter writes: > > > > > >> Hm, where do we have the canonical source for all these fourcc codes? I'm > > >> asking since we have our own copy in the kernel as drm_fourcc.h, and that > > >> one is part of the userspace ABI since we use it to pass around > > >> framebuffer formats and format lists. > > > > > > I think it's the kernel? I really don't know, as the whole notion of > > > fourcc codes seems crazy to me... > > > > > > Feel free to steal this code and stick it in the kernel if you like. > > > > Well, I wasn't ever in favour of using fourcc codes since they're just > > not standardized at all, highly redundant in some cases and also miss > > lots of stuff we actually need (like all the rgb formats). > > These drm codes are not fourcc codes in any other way than that > they're defined by creating a 32 bit value by picking four characters. > I don't know what PTSD triggers people have from hearing "fourcc", but > it seems severe. Forget all that, these codes are DRM specific > defines that are not inteded to match anything anybody else does. It > doesn't matter if these match of conflict with v4l, fourcc.org, > wikipedia.org or what the amiga did. They're just tokens that let us > define succintly what the pixel format of a kms framebuffer is and > tell the kernel. > > I don't know what else you'd propose? Pass an X visual in the ioctl? > An EGL config? This is our name space, we can add stuff as we need > (as Keith is doing here). include/uapi/drm/drm_fourcc.h is the > canonical source for these values and we should add > DRM_FORMAT_SARGB there to make sure we don't clash. What is this format anyway? -ENODOCS If its just an srgb version of ARGB, then I wouldn't really want it in drm_fourcc.h. I expect colorspacy stuff will be handled by various crtc/plane properties in the kernel so we don't need to encode that stuff into the fb format. -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Mesa-dev] [PATCH] dri3, i915, i965: Add __DRI_IMAGE_FOURCC_SARGB8888
On Fri, Nov 22, 2013 at 05:17:37PM +0100, Daniel Vetter wrote: > On Fri, Nov 22, 2013 at 12:01 PM, Keith Packard wrote: > > Daniel Vetter writes: > > > >> Hm, where do we have the canonical source for all these fourcc codes? I'm > >> asking since we have our own copy in the kernel as drm_fourcc.h, and that > >> one is part of the userspace ABI since we use it to pass around > >> framebuffer formats and format lists. > > > > I think it's the kernel? I really don't know, as the whole notion of > > fourcc codes seems crazy to me... > > > > Feel free to steal this code and stick it in the kernel if you like. > > Well, I wasn't ever in favour of using fourcc codes since they're just > not standardized at all, highly redundant in some cases and also miss > lots of stuff we actually need (like all the rgb formats). I also argued against them, but some people wanted them for whatever reason. And since I didn't want to argue for several years about the subject, I just gave in and made the drm pixel formats fourcss. But given that I just pulled the fourccs out of my ass, we can't really cross use them between different subsystems anyway. So if we just consider all the different fourcc namespaces totally distinct, we're not going to have any problems. Personally I can promise that I will _not_ be checking Mesa/whatever code for conflicting fourccs when I need to add a new one to drm_fourcc.h. There, now I've given fair warning and if things explode later it won't be my fault. However if someone wants to emulate the drm fourcc style for whatever reason, there is a distinct pattern how I cooked them up. Well, a few different patterns depending whether it's RGB,YUV,packed,planar etc. > > Cc'ing the heck out of this to get kernel people to hopefully notice. > Maybe someone takes charge of this ... Otherwise meh. > > >> Just afraid to create long-term maintainance madness here with the > >> kernel's iron thou-shalt-not-break-userspace-ever rule ... Not likely > >> we'll ever accept srgb for framebuffers though. > > > > Would suck to collide with something we do want though. > > Yeah, it'd suck. But given how fourcc works we probably have that > already, just haven't noticed yet :( > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] mutex: add support for reservation style locks, v2
On Wed, Apr 17, 2013 at 09:08:17PM +0200, Daniel Vetter wrote: > On Wed, Apr 10, 2013 at 12:28 AM, Steven Rostedt wrote: > > On Thu, Apr 04, 2013 at 06:41:02PM +0200, Peter Zijlstra wrote: > >> On Thu, 2013-04-04 at 15:31 +0200, Daniel Vetter wrote: > >> > The thing is now that you're not expected to hold these locks for a > >> > long > >> > time - if you need to synchronously stall while holding a lock > >> > performance > >> > will go down the gutters anyway. And since most current > >> > gpus/co-processors > >> > still can't really preempt fairness isn't that high a priority, > >> > either. > >> > So we didn't think too much about that. > >> > >> Yeah but you're proposing a new synchronization primitive for the core > >> kernel.. all such 'fun' details need to be considered, not only those > >> few that bear on the one usecase. > > > > Which bares the question, what other use cases are there? > > Just stumbled over one I think: If we have a big graph of connected > things (happens really often for video pipelines). And we want > multiple users to use them in parallel. But sometimes a configuration > change could take way too long and so would unduly stall a 2nd thread > with just a global mutex, then per-object ww_mutexes would be a fit: > You'd start with grabbing all the locks for the objects you want to > change anything with, then grab anything in the graph that you also > need to check. Thanks to loop detection and self-recursion this would > all nicely work out, even for cyclic graphs of objects. Indeed, that would make the locking for atomic modeset/page flip very easy to handle, while still allowing the use of suitable fine grained locks. I like the idea. -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma-buf: document fd flags and O_CLOEXEC requirement
On Mon, Mar 19, 2012 at 04:41:55PM +0100, Daniel Vetter wrote: > Otherwise subsystems will get this wrong and end up with a second > export ioctl with the flag and O_CLOEXEC support added. > > v2: Fixup the function name and caution exporters to limit the flags > to only O_CLOEXEC. Noted by Dave Airlie. > > Cc: Dave Airlie > Signed-Off-by: Daniel Vetter > --- > Documentation/dma-buf-sharing.txt |6 ++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/Documentation/dma-buf-sharing.txt > b/Documentation/dma-buf-sharing.txt > index 9f3aeef..a6d4c37 100644 > --- a/Documentation/dma-buf-sharing.txt > +++ b/Documentation/dma-buf-sharing.txt > @@ -319,6 +319,12 @@ Miscellaneous notes > - Any exporters or users of the dma-buf buffer sharing framework must have >a 'select DMA_SHARED_BUFFER' in their respective Kconfigs. > > +- To avoid the wrath of userspace library writers exporting subsystems must > have > + a flag parameter in the ioctl that creates the dma-buf fd which needs to > + support at least the O_CLOEXEC fd flag. This needs to be passed in the flag > + parameter of dma_buf_fd. Without any other reasons applying it is > recommended > + that exporters limit the flags passed to dma_buf_fd to only O_CLOEXEC. Difficult to parse. Needs more punctuation. -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ksummit-2010-discuss] [v2] Remaining BKL users, what to do
On Wed, Oct 20, 2010 at 06:50:58AM +1000, Dave Airlie wrote:
> On Tue, Oct 19, 2010 at 11:26 PM, Arnd Bergmann wrote:
> > On Tuesday 19 October 2010, Arnd Bergmann wrote:
> >> On Tuesday 19 October 2010 06:52:32 Dave Airlie wrote:
> >> > > I might be able to find some hardware still lying around here that
> >> > > uses an
> >> > > i810. Not sure unless I go hunting it. But I get the impression that if
> >> > > the kernel is a single-CPU kernel there is not any problem anyway?
> >> > > Don't
> >> > > distros offer a non-smp kernel as an installation option in case the
> >> > > user
> >> > > needs it? So in reality how big a problem is this?
> >> >
> >> > Not anymore, which is my old point of making a fuss. Nowadays in the
> >> > modern distro world, we supply a single kernel that can at runtime
> >> > decide if its running on SMP or UP and rewrite the text section
> >> > appropriately with locks etc. Its like magic, and something like
> >> > marking drivers as BROKEN_ON_SMP at compile time is really wrong when
> >> > what you want now is a runtime warning if someone tries to hotplug a
> >> > CPU with a known iffy driver loaded or if someone tries to load the
> >> > driver when we are already in SMP mode.
> >>
> >> We could make the driver run-time non-SMP by adding
> >>
> >> if (num_present_cpus() > 1) {
> >> pr_err("i810 no longer supports SMP\n");
> >> return -EINVAL;
> >> }
> >>
> >> to the init function. That would cover the vast majority of the
> >> users of i810 hardware, I guess.
> >
> > Some research showed that Intel never support i810/i815 SMP setups,
> > but there was indeed one company (http://www.acorpusa.com at the time,
> > now owned by a domain squatter) that made i815E based dual Pentium-III
> > boards like this one: http://cgi.ebay.com/280319795096
>
> Also that board has no on-board GPU enabled i815EP (P means no on-board GPU).
A quick search seems to indicate that an i815E variant also existed.
--
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] Input: ati-remote2 - switch to using new keycode interface
On Mon, Sep 13, 2010 at 09:28:07AM -0700, Dmitry Torokhov wrote:
> On Thu, Sep 09, 2010 at 03:40:04PM +0300, Ville Syrjälä wrote:
> > On Wed, Sep 08, 2010 at 12:42:05AM -0700, Dmitry Torokhov wrote:
> > > Switch the code to use new style of getkeycode and setkeycode
> > > methods to allow retrieving and setting keycodes not only by
> > > their scancodes but also by index.
> > >
> > > Signed-off-by: Dmitry Torokhov
> > > ---
> > >
> > > drivers/input/misc/ati_remote2.c | 93
> > > +++---
> > > 1 files changed, 65 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/drivers/input/misc/ati_remote2.c
> > > b/drivers/input/misc/ati_remote2.c
> > > index 2325765..b2e0d82 100644
> > > --- a/drivers/input/misc/ati_remote2.c
> > > +++ b/drivers/input/misc/ati_remote2.c
> > > @@ -483,51 +483,88 @@ static void ati_remote2_complete_key(struct urb
> > > *urb)
> > > }
> > >
> > > static int ati_remote2_getkeycode(struct input_dev *idev,
> > > - unsigned int scancode, unsigned int *keycode)
> > > + struct input_keymap_entry *ke)
> > > {
> > > struct ati_remote2 *ar2 = input_get_drvdata(idev);
> > > unsigned int mode;
> > > - int index;
> > > + int offset;
> > > + unsigned int index;
> > > + unsigned int scancode;
> > > +
> > > + if (ke->flags & INPUT_KEYMAP_BY_INDEX) {
> > > + index = ke->index;
> > > + if (index >= (ATI_REMOTE2_MODES - 1) *
> >
> > That -1 looks wrong. Same in setkeycode().
> >
>
> Yes, indeed. Thanks for noticing.
I fixed this bug locally and gave this a short whirl with my RWII.
I tried both the old and new style keycode ioctls. Everything
worked as expected.
So if you want more tags feel free to add my Acked-by and Tested-by
for this (assuming the off-by-one fix is included) and you can add my
Tested-by for patch 1/6 as well.
--
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] Input: ati-remote2 - switch to using new keycode interface
ndex];
> - ar2->keycode[mode][index] = keycode;
> - __set_bit(keycode, idev->keybit);
> + *old_keycode = ar2->keycode[mode][offset];
> + ar2->keycode[mode][offset] = ke->keycode;
> + __set_bit(ke->keycode, idev->keybit);
>
> for (mode = 0; mode < ATI_REMOTE2_MODES; mode++) {
> for (index = 0; index < ARRAY_SIZE(ati_remote2_key_table);
> index++) {
> - if (ar2->keycode[mode][index] == old_keycode)
> + if (ar2->keycode[mode][index] == *old_keycode)
> return 0;
> }
> }
>
> - __clear_bit(old_keycode, idev->keybit);
> + __clear_bit(*old_keycode, idev->keybit);
>
> return 0;
> }
> @@ -575,8 +612,8 @@ static int ati_remote2_input_init(struct ati_remote2 *ar2)
> idev->open = ati_remote2_open;
> idev->close = ati_remote2_close;
>
> - idev->getkeycode = ati_remote2_getkeycode;
> - idev->setkeycode = ati_remote2_setkeycode;
> + idev->getkeycode_new = ati_remote2_getkeycode;
> + idev->setkeycode_new = ati_remote2_setkeycode;
>
> idev->name = ar2->name;
> idev->phys = ar2->phys;
>
--
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Idea of a v4l -> fb interface driver
On Fri, May 28, 2010 at 03:41:46PM -0400, Alex Deucher wrote: > On Fri, May 28, 2010 at 3:15 PM, Florian Tobias Schandinat > > If he wants different (independent) content on each output, just provide > > multiple /dev/fbX devices. I admit that we could use a controlling interface > > here that decides which user (application) might draw at a time to the > > interface which they currently only do if they are the active VT. > > If you want 2 or more outputs to be merged as one just configure this in the > > driver. > > The only thing that is impossible to do in fbdev is controlling 2 or more > > independent display outputs that access the same buffer. But that's not an > > issue I think. > > The things above only could use a unification of how to set them up on > > module load time (as only limited runtime changes are permited given that we > > must always be able to support a mode that we once entered during runtime). > > > > What about changing outputs on the fly (turn off VGA, turn on DVI, > switch between multi-head and single-head, etc) or encoders shared > between multiple connectors (think a single dac shared between a VGA > and a TV port); how do you expose them easily as separate fbdevs? > Lots of stuff is doable with fbdev, but it's nicer with kms. But actually getting your data onto the screen is a lot easier with fbdev. There's no standard API in drm to actually allocate the framebuffer and manipulate it. You always need a user space driver to go along with the kernel bits. I'm not saying fbdev is better than drm/kms but at least it can be used to write simple applications that work across different hardware. Perhaps that's something that should be addressed in the drm API. -- Ville Syrjälä syrj...@sci.fi http://www.sci.fi/~syrjala/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Infrared Keycode standardization
On Thu, Aug 27, 2009 at 04:57:10AM -0300, Mauro Carvalho Chehab wrote: > After years of analyzing the existing code and receiving/merging patches > related to IR, and taking a looking at the current scenario, it is clear to me > that something need to be done, in order to have some standard way to map and > to give precise key meanings for each used media keycode found on > include/linux/input.h. > > Just as an example, I've parsed the bigger keymap file we have > (linux/media/common/ir-common.c). Most IR's have less than 40 keys, most are > common between several different models. Yet, we've got almost 500 different > mappings there (and I removed from my parser all the "obvious" keys that there > weren't any comment about what is labeled for that key on the IR). > > The same key name is mapped differently, depending only at the wish of the > patch author, as shown at: > > http://linuxtv.org/wiki/index.php/Ir-common.c > > It doesn't come by surprise, but currently, almost all media player > applications don't care to properly map all those keys. > > I've tried to find comments and/or descriptions about each media keys defined > at input.h without success. Just a few keys are commented at the file itself. > (or maybe I've just seek them at the wrong places). > > So, I took the initiative of doing a proposition for standardizing those keys > at: > > http://linuxtv.org/wiki/index.php/Proposal I welcome this effort. It would be nice to have some kind of consistent behaviour between devices. But just limiting the effort to IR devices doesn't make sense. It shouldn't matter how the device is connected. FASTWORWARD,REWIND,FORWARD and BACK aren't very clear. To me it would make most sense if FASTFORWARD and REWIND were paired and FORWARD and BACK were paired. I actually have those two a bit confused in ati_remote2 too where I used FASTFORWARD and BACK. I suppose it should be REWIND instead. Also I should probably use ZOOM for the maximize/restore button (it's FRONT now), and maybe SETUP instead of ENTER for another. It has a picture of a checkbox, Windows software apparently shows a setup menu when it's pressed. There are also a couple of buttons where no keycode really seems to match. One is the mouse button drag. I suppose I could implement the drag lock feature in the driver but I'm not sure if that's a good idea. It would make that button special and unmappable. Currently I have that mapped to EDIT IIRC. The other oddball button has a picture of a stopwatch (I think, it's not very clear). Currently it uses COFFEE, but maybe TIMER or something like that should be added. The Windows software's manual just say it toggles TV-on-demand, but I have no idea what that actually is. -- Ville Syrjälä syrj...@sci.fi http://www.sci.fi/~syrjala/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
