On Mon, Jan 9, 2017 at 4:03 AM, Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx> wrote:
> On 6 January 2017 at 16:56, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote:
>> On Fri, Jan 6, 2017 at 9:30 AM, Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx> wrote:
>>> This backpointer allows DP helpers to access the connector it's being
>>> used for.
>>>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx>
>>> ---
>>>
>>> include/drm/drm_dp_helper.h | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>>> index 55bbeb0ff594..4fa77b434594 100644
>>> --- a/include/drm/drm_dp_helper.h
>>> +++ b/include/drm/drm_dp_helper.h
>>> @@ -721,6 +721,7 @@ struct drm_dp_aux_msg {
>>> * @name: user-visible name of this AUX channel and the I2C-over-AUX adapter
>>> * @ddc: I2C adapter that can be used for I2C-over-AUX communication
>>> * @dev: pointer to struct device that is the parent for this AUX channel
>>> + * @connector: backpointer to connector that uses this AUX channel
>>> * @hw_mutex: internal mutex used for locking transfers
>>> * @transfer: transfers a message representing a single AUX transaction
>>> *
>>> @@ -757,6 +758,7 @@ struct drm_dp_aux {
>>> const char *name;
>>> struct i2c_adapter ddc;
>>> struct device *dev;
>>> + struct drm_connector *connector;
>>
>> Perhaps I'm misreading the series, but it seems like you could instead
>> add int crc_pipe along with the other crc fields. Then when you start
>> the crc, set the pipe number. If you have the pipe in the crc worker,
>> you can wait vblank on that pipe (no pointers needed).
>>
>> Reasonable?
>
> I think that would work fine, but is it any better? I like that the
> connector isn't going to change during the lifetime of the drm_dp_aux,
Is this actually guaranteed, though? I am having a hard time thinking
about a practical example where it's not implemented this way, but I
don't see anything actually enforcing the lifetime relationship.
> but crc_pipe could be changing between sampling sessions and any bugs
> in keeping that field updated would be quite disconcerting and
> cumbersome to debug.
Couldn't the same could be said for connector->state->crtc?
Sean
>
> crc_pipe's advantage is that we wouldn't need to update all callers of
> drm_dp_aux_register, but I think it's better to have a connector field
> that is mistakenly NULL, than a crc_pipe field with the wrong value.
>
> Regards,
>
> Tomeu
>
>> Sean
>>
>>> struct mutex hw_mutex;
>>> ssize_t (*transfer)(struct drm_dp_aux *aux,
>>> struct drm_dp_aux_msg *msg);
>>> --
>>> 2.9.3
>>>
>>
>>
>>
>> --
>> Sean Paul, Software Engineer, Google / Chromium OS
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
